From b9a2abc097fd7ead518cdf59be21ad962e92bb67 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Sun, 14 Mar 2021 08:27:06 +0000 Subject: [PATCH] ss: force check all set_metadata returns lws_ss_set_metadata can fail... eg, due to transient OOM situation... if it does, caller must take appropriate action like disconnect and retry. So mark the api as requiring the result checking, and make sure all the examples do it. --- include/libwebsockets/lws-secure-streams.h | 5 ++-- lib/secure-streams/protocols/ss-h1.c | 26 ++++++++++++------- lib/secure-streams/secure-streams.c | 1 + .../system/auth-api.amazon.com/auth.c | 7 ++--- .../minimal-secure-streams.c | 3 ++- .../minimal-secure-streams.c | 4 ++- .../minimal-secure-streams-post.c | 5 ++-- .../minimal-secure-streams-server/ss-server.c | 12 +++++---- .../minimal-secure-streams-testsfail.c | 3 ++- .../minimal-secure-streams.c | 18 +++++++++---- 10 files changed, 54 insertions(+), 30 deletions(-) diff --git a/include/libwebsockets/lws-secure-streams.h b/include/libwebsockets/lws-secure-streams.h index a23f611e3..f52ff1a7d 100644 --- a/include/libwebsockets/lws-secure-streams.h +++ b/include/libwebsockets/lws-secure-streams.h @@ -622,9 +622,10 @@ lws_ss_rideshare(struct lws_ss_handle *h); * name with the value of the metadata on the left. * * Return 0 if OK or nonzero if, eg, metadata name does not exist on the - * streamtype. + * streamtype. You must check the result of this, eg, transient OOM can cause + * these to fail and you should retry later. */ -LWS_VISIBLE LWS_EXTERN int +LWS_VISIBLE LWS_EXTERN int LWS_WARN_UNUSED_RESULT lws_ss_set_metadata(struct lws_ss_handle *h, const char *name, const void *value, size_t len); diff --git a/lib/secure-streams/protocols/ss-h1.c b/lib/secure-streams/protocols/ss-h1.c index 0311dd885..d2dd267ef 100644 --- a/lib/secure-streams/protocols/ss-h1.c +++ b/lib/secure-streams/protocols/ss-h1.c @@ -962,29 +962,35 @@ malformed: #if defined(LWS_ROLE_H2) m = lws_hdr_total_length(wsi, WSI_TOKEN_HTTP_COLON_METHOD); if (m) { - lws_ss_set_metadata(h, "method", + if (lws_ss_set_metadata(h, "method", lws_hdr_simple_ptr(wsi, - WSI_TOKEN_HTTP_COLON_METHOD), (unsigned int)m); + WSI_TOKEN_HTTP_COLON_METHOD), (unsigned int)m)) + return -1; m = lws_hdr_total_length(wsi, WSI_TOKEN_HTTP_COLON_PATH); - lws_ss_set_metadata(h, "path", + if (lws_ss_set_metadata(h, "path", lws_hdr_simple_ptr(wsi, - WSI_TOKEN_HTTP_COLON_PATH), (unsigned int)m); + WSI_TOKEN_HTTP_COLON_PATH), (unsigned int)m)) + return -1; } else #endif { m = lws_hdr_total_length(wsi, WSI_TOKEN_GET_URI); if (m) { - lws_ss_set_metadata(h, "path", + if (lws_ss_set_metadata(h, "path", lws_hdr_simple_ptr(wsi, - WSI_TOKEN_GET_URI), (unsigned int)m); - lws_ss_set_metadata(h, "method", "GET", 3); + WSI_TOKEN_GET_URI), (unsigned int)m)) + return -1; + if (lws_ss_set_metadata(h, "method", "GET", 3)) + return -1; } else { m = lws_hdr_total_length(wsi, WSI_TOKEN_POST_URI); if (m) { - lws_ss_set_metadata(h, "path", + if (lws_ss_set_metadata(h, "path", lws_hdr_simple_ptr(wsi, - WSI_TOKEN_POST_URI), (unsigned int)m); - lws_ss_set_metadata(h, "method", "POST", 4); + WSI_TOKEN_POST_URI), (unsigned int)m)) + return -1; + if (lws_ss_set_metadata(h, "method", "POST", 4)) + return -1; } } } diff --git a/lib/secure-streams/secure-streams.c b/lib/secure-streams/secure-streams.c index c28230495..47736e39d 100644 --- a/lib/secure-streams/secure-streams.c +++ b/lib/secure-streams/secure-streams.c @@ -1027,6 +1027,7 @@ late_bail: lws_pt_lock(pt, __func__); lws_dll2_remove(&h->list); lws_pt_unlock(pt); + __lws_lc_untag(&h->lc); lws_free(h); return 1; diff --git a/lib/secure-streams/system/auth-api.amazon.com/auth.c b/lib/secure-streams/system/auth-api.amazon.com/auth.c index 6caaef591..617823b61 100644 --- a/lib/secure-streams/system/auth-api.amazon.com/auth.c +++ b/lib/secure-streams/system/auth-api.amazon.com/auth.c @@ -54,7 +54,7 @@ static void lws_ss_sys_auth_api_amazon_com_kick(lws_sorted_usec_list_t *sul) { struct lws_context *context = lws_container_of(sul, struct lws_context, - sul_api_amazon_com_kick); + sul_api_amazon_com_kick); lws_state_transition_steps(&context->mgr_system, LWS_SYSTATE_OPERATIONAL); @@ -64,7 +64,7 @@ static void lws_ss_sys_auth_api_amazon_com_renew(lws_sorted_usec_list_t *sul) { struct lws_context *context = lws_container_of(sul, struct lws_context, - sul_api_amazon_com); + sul_api_amazon_com); lws_ss_sys_auth_api_amazon_com(context); } @@ -216,7 +216,8 @@ ss_api_amazon_auth_state(void *userobj, void *sh, lws_ss_constate_t state, switch (state) { case LWSSSCS_CREATING: - lws_ss_set_metadata(m->ss, "ctype", "application/json", 16); + if (lws_ss_set_metadata(m->ss, "ctype", "application/json", 16)) + return LWSSSSRET_DESTROY_ME; /* fallthru */ case LWSSSCS_CONNECTING: s = lws_system_blob_get_size(ab); diff --git a/minimal-examples/secure-streams/minimal-secure-streams-hugeurl/minimal-secure-streams.c b/minimal-examples/secure-streams/minimal-secure-streams-hugeurl/minimal-secure-streams.c index 16e9e56d7..67e4f9178 100644 --- a/minimal-examples/secure-streams/minimal-secure-streams-hugeurl/minimal-secure-streams.c +++ b/minimal-examples/secure-streams/minimal-secure-streams-hugeurl/minimal-secure-streams.c @@ -281,7 +281,8 @@ myss_state(void *userobj, void *sh, lws_ss_constate_t state, lws_hex_random(lws_ss_get_context(m->ss), hugeurl, hugeurl_size + 1); - lws_ss_set_metadata(m->ss, "hugearg", hugeurl, hugeurl_size); + if (lws_ss_set_metadata(m->ss, "hugearg", hugeurl, hugeurl_size)) + return LWSSSSRET_DISCONNECT_ME; return lws_ss_client_connect(m->ss); diff --git a/minimal-examples/secure-streams/minimal-secure-streams-metadata/minimal-secure-streams.c b/minimal-examples/secure-streams/minimal-secure-streams-metadata/minimal-secure-streams.c index 08550890d..4f34ced5a 100644 --- a/minimal-examples/secure-streams/minimal-secure-streams-metadata/minimal-secure-streams.c +++ b/minimal-examples/secure-streams/minimal-secure-streams-metadata/minimal-secure-streams.c @@ -182,7 +182,9 @@ myss_state(void *userobj, void *sh, lws_ss_constate_t state, case LWSSSCS_CREATING: lwsl_notice("%s: CREATING: setting servername metadata to %s\n", __func__, server_name_or_url); - lws_ss_set_metadata(m->ss, "servername", server_name_or_url, strlen(server_name_or_url)); + if (lws_ss_set_metadata(m->ss, "servername", server_name_or_url, + strlen(server_name_or_url))) + return LWSSSSRET_DISCONNECT_ME; return lws_ss_client_connect(m->ss); case LWSSSCS_ALL_RETRIES_FAILED: diff --git a/minimal-examples/secure-streams/minimal-secure-streams-post/minimal-secure-streams-post.c b/minimal-examples/secure-streams/minimal-secure-streams-post/minimal-secure-streams-post.c index a213135a1..39257e3ea 100644 --- a/minimal-examples/secure-streams/minimal-secure-streams-post/minimal-secure-streams-post.c +++ b/minimal-examples/secure-streams/minimal-secure-streams-post/minimal-secure-streams-post.c @@ -345,9 +345,10 @@ myss_state(void *userobj, void *sh, lws_ss_constate_t state, * proxy to create the stream and it has been allowed. */ - lws_ss_set_metadata(m->ss, "ctype", + if (lws_ss_set_metadata(m->ss, "ctype", "multipart/form-data;boundary=\"boundary\"", - 39); + 39)) + return LWSSSSRET_DISCONNECT_ME; /* provide a hint about the payload size */ m->pos = 0; diff --git a/minimal-examples/secure-streams/minimal-secure-streams-server/ss-server.c b/minimal-examples/secure-streams/minimal-secure-streams-server/ss-server.c index 295250149..2a810920e 100644 --- a/minimal-examples/secure-streams/minimal-secure-streams-server/ss-server.c +++ b/minimal-examples/secure-streams/minimal-secure-streams-server/ss-server.c @@ -191,11 +191,13 @@ myss_srv_state(void *userobj, void *sh, lws_ss_constate_t state, /* * ... it's going to be either text/html or multipart ... */ - if (multipart) - lws_ss_set_metadata(m->ss, "mime", - "multipart/form-data; boundary=aBoundaryString", 45); - else - lws_ss_set_metadata(m->ss, "mime", "text/html", 9); + if (multipart) { + if (lws_ss_set_metadata(m->ss, "mime", + "multipart/form-data; boundary=aBoundaryString", 45)) + return LWSSSSRET_DISCONNECT_ME; + } else + if (lws_ss_set_metadata(m->ss, "mime", "text/html", 9)) + return LWSSSSRET_DISCONNECT_ME; /* * ...it's going to be whatever size it is (and request tx) */ diff --git a/minimal-examples/secure-streams/minimal-secure-streams-testsfail/minimal-secure-streams-testsfail.c b/minimal-examples/secure-streams/minimal-secure-streams-testsfail/minimal-secure-streams-testsfail.c index ba5b1a6b8..eee55046e 100644 --- a/minimal-examples/secure-streams/minimal-secure-streams-testsfail/minimal-secure-streams-testsfail.c +++ b/minimal-examples/secure-streams/minimal-secure-streams-testsfail/minimal-secure-streams-testsfail.c @@ -660,7 +660,8 @@ fail: if (curr_test->eom_pass) { sl = (size_t)lws_snprintf(buf, sizeof(buf), "%u", (unsigned int)curr_test->eom_pass); - lws_ss_set_metadata(m->ss, "amount", buf, sl); + if (lws_ss_set_metadata(m->ss, "amount", buf, sl)) + return LWSSSSRET_DISCONNECT_ME; } return lws_ss_client_connect(m->ss); diff --git a/minimal-examples/secure-streams/minimal-secure-streams/minimal-secure-streams.c b/minimal-examples/secure-streams/minimal-secure-streams/minimal-secure-streams.c index 700062942..36704fc6e 100644 --- a/minimal-examples/secure-streams/minimal-secure-streams/minimal-secure-streams.c +++ b/minimal-examples/secure-streams/minimal-secure-streams/minimal-secure-streams.c @@ -207,7 +207,7 @@ myss_rx(void *userobj, const uint8_t *buf, size_t len, int flags) interrupted = 1; } - return 0; + return LWSSSSRET_OK; } static lws_ss_state_return_t @@ -232,11 +232,19 @@ myss_state(void *userobj, void *sh, lws_ss_constate_t state, switch (state) { case LWSSSCS_CREATING: - lws_ss_start_timeout(m->ss, timeout_ms); - lws_ss_set_metadata(m->ss, "uptag", "myuptag123", 10); - lws_ss_set_metadata(m->ss, "ctype", "myctype", 7); return lws_ss_client_connect(m->ss); + case LWSSSCS_CONNECTING: + lws_ss_start_timeout(m->ss, timeout_ms); + if (lws_ss_set_metadata(m->ss, "uptag", "myuptag123", 10)) + /* can fail, eg due to OOM, retry later if so */ + return LWSSSSRET_DISCONNECT_ME; + + if (lws_ss_set_metadata(m->ss, "ctype", "myctype", 7)) + /* can fail, eg due to OOM, retry later if so */ + return LWSSSSRET_DISCONNECT_ME; + break; + case LWSSSCS_ALL_RETRIES_FAILED: /* if we're out of retries, we want to close the app and FAIL */ interrupted = 1; @@ -257,7 +265,7 @@ myss_state(void *userobj, void *sh, lws_ss_constate_t state, break; } - return 0; + return LWSSSSRET_OK; } static int