From e5b191be35c8145e1c184aa9b915a3abe748d085 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Mon, 18 Jan 2021 14:20:37 +0000 Subject: [PATCH] h2: post buflist: track rx_content_length On h2 server POST, there's a race to see if the POST body is going to be received coalesced with the headers. The problem is on h2, we can't action the stream http request or body until the stream is writeable, since we may start issuing the response right away; there's already DEFERRING_ACTION state to manage this. And indeed, the coalesced, not-immediately-actionable POST body is buflisted properly. However when we come to action the POST using buflisted data, we don't follow the same pattern as dealing with the incoming data immediately. This patch aligns the pattern dumping the buflist content to track expected rx_content_length and handle BODY_COMPLETION if we got to the end of it, along with removal from the pt list of wsi with pending buflists if we used it up. --- lib/core-net/output.c | 18 ++-- lib/roles/h2/http2.c | 19 ++-- lib/roles/h2/ops-h2.c | 157 ++++++++++++++++++++----------- lib/tls/mbedtls/mbedtls-server.c | 2 +- 4 files changed, 118 insertions(+), 78 deletions(-) diff --git a/lib/core-net/output.c b/lib/core-net/output.c index 0c46f2c43..71e59122f 100644 --- a/lib/core-net/output.c +++ b/lib/core-net/output.c @@ -293,11 +293,7 @@ lws_write(struct lws *wsi, unsigned char *buf, size_t len, int lws_ssl_capable_read_no_ssl(struct lws *wsi, unsigned char *buf, size_t len) { - struct lws_context *context = wsi->a.context; - struct lws_context_per_thread *pt = &context->pt[(int)wsi->tsi]; - int n = 0; - - lws_stats_bump(pt, LWSSTATS_C_API_READ, 1); + int n = 0, en; errno = 0; #if defined(LWS_WITH_UDP) @@ -317,7 +313,7 @@ lws_ssl_capable_read_no_ssl(struct lws *wsi, unsigned char *buf, size_t len) (int) #endif len, 0); - + en = LWS_ERRNO; if (n >= 0) { if (!n && wsi->unix_skt) @@ -334,17 +330,17 @@ lws_ssl_capable_read_no_ssl(struct lws *wsi, unsigned char *buf, size_t len) if (wsi->a.vhost) wsi->a.vhost->conn_stats.rx = (unsigned long long)(wsi->a.vhost->conn_stats.rx + (unsigned long long)(long long)n); #endif - lws_stats_bump(pt, LWSSTATS_B_READ, (unsigned int)n); return n; } - if (LWS_ERRNO == LWS_EAGAIN || - LWS_ERRNO == LWS_EWOULDBLOCK || - LWS_ERRNO == LWS_EINTR) + if (en == LWS_EAGAIN || + en == LWS_EWOULDBLOCK || + en == LWS_EINTR) return LWS_SSL_CAPABLE_MORE_SERVICE; - lwsl_info("error on reading from skt : %d\n", LWS_ERRNO); + lwsl_info("error on reading from skt : %d\n", en); + return LWS_SSL_CAPABLE_ERROR; } diff --git a/lib/roles/h2/http2.c b/lib/roles/h2/http2.c index 9710f3a11..6eba0c77a 100644 --- a/lib/roles/h2/http2.c +++ b/lib/roles/h2/http2.c @@ -1583,6 +1583,7 @@ lws_h2_parse_end_of_frame(struct lws *wsi) h2n->swsi->http.rx_content_length = (unsigned long long)atoll(simp); h2n->swsi->http.rx_content_remain = h2n->swsi->http.rx_content_length; + h2n->swsi->http.content_length_given = 1; lwsl_info("setting rx_content_length %lld\n", (long long)h2n->swsi->http.rx_content_length); } @@ -2143,16 +2144,16 @@ lws_h2_parser(struct lws *wsi, unsigned char *in, lws_filepos_t _inlen, &h2n->swsi->buflist, in - 1, (unsigned int)n); if (m < 0) return -1; - if (m) { - struct lws_context_per_thread *pt; - pt = &wsi->a.context->pt[(int)wsi->tsi]; - lwsl_debug("%s: added %s to rxflow list\n", - __func__, lws_wsi_tag(wsi)); - lws_dll2_add_head( - &h2n->swsi->dll_buflist, - &pt->dll_buflist_owner); - } + /* + * Since we're in an open-ended + * DEFERRING_ACTION, don't add this swsi + * to the pt list of wsi holding buflist + * content yet, we are not in a position + * to consume it until we get out of + * DEFERRING_ACTION. + */ + in += n - 1; h2n->inside += (unsigned int)n; h2n->count += (unsigned int)n - 1; diff --git a/lib/roles/h2/ops-h2.c b/lib/roles/h2/ops-h2.c index 8712dfed1..e0ea4b446 100644 --- a/lib/roles/h2/ops-h2.c +++ b/lib/roles/h2/ops-h2.c @@ -90,7 +90,8 @@ const struct http2_settings lws_h2_stock_settings = { { }}; /* - * The wsi at this level is the network wsi + * The wsi at this level is normally the network wsi... we can get called on + * another path via lws_service_do_ripe_rxflow() on mux children too tho... */ static int @@ -190,6 +191,14 @@ read: lwsl_info("draining buflist (len %d)\n", ebuf.len); buffered = 1; goto drain; + } else { + + if (wsi->mux_substream) { + lwsl_warn("%s: uh... %s mux child with nothing to drain\n", __func__, lws_wsi_tag(wsi)); + // assert(0); + lws_dll2_remove(&wsi->dll_buflist); + return LWS_HPI_RET_HANDLED; + } } if (!lws_ssl_pending(wsi) && @@ -767,74 +776,97 @@ rops_callback_on_writable_h2(struct lws *wsi) static int lws_h2_bind_for_post_before_action(struct lws *wsi) { + const struct lws_http_mount *hit; + char *uri_ptr = NULL; uint8_t *buffered; + int uri_len = 0; const char *p; size_t blen; p = lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_METHOD); - if (p && !strcmp(p, "POST")) { - const struct lws_http_mount *hit; + if (!p || strcmp(p, "POST")) + return 0; - if (!lws_hdr_total_length(wsi, WSI_TOKEN_HTTP_COLON_PATH) || - !lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_PATH)) - /* - * There must be a path. Actually this is checked at - * http2.c along with the other required header - * presence before we can get here. - * - * But Coverity insists to see us check it. - */ - return 1; - - hit = lws_find_mount(wsi, - lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_PATH), - lws_hdr_total_length(wsi, WSI_TOKEN_HTTP_COLON_PATH)); - - lwsl_debug("%s: %s: hit %p: %s\n", __func__, - lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_PATH), - hit, hit ? hit->origin : "null"); - if (hit) { - const struct lws_protocols *pp; - const char *name = hit->origin; - - if (hit->protocol) - name = hit->protocol; - - pp = lws_vhost_name_to_protocol(wsi->a.vhost, name); - if (!pp) { - lwsl_info("Unable to find protocol '%s'\n", name); - return 1; - } - - if (lws_bind_protocol(wsi, pp, __func__)) - return 1; - } - - { - int uri_len = 0; - char *uri_ptr = NULL; - - if (lws_http_get_uri_and_method(wsi, &uri_ptr, &uri_len) >= 0) - wsi->a.protocol->callback(wsi, LWS_CALLBACK_HTTP, - wsi->user_space, uri_ptr, (size_t)uri_len); - } - - lwsl_info("%s: setting LRS_BODY from 0x%x (%s)\n", __func__, - (int)wsi->wsistate, wsi->a.protocol->name); - - lwsi_set_state(wsi, LRS_BODY); + if (!lws_hdr_total_length(wsi, WSI_TOKEN_HTTP_COLON_PATH) || + !lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_PATH)) /* - * Dump any stashed body + * There must be a path. Actually this is checked at + * http2.c along with the other required header + * presence before we can get here. + * + * But Coverity insists to see us check it. */ + return 1; - while ((blen = lws_buflist_next_segment_len(&wsi->buflist, &buffered))) { - if (wsi->a.protocol->callback(wsi, LWS_CALLBACK_HTTP_BODY, - wsi->user_space, buffered, blen)) - return 1; - lws_buflist_use_segment(&wsi->buflist, blen); + hit = lws_find_mount(wsi, + lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_PATH), + lws_hdr_total_length(wsi, WSI_TOKEN_HTTP_COLON_PATH)); + + lwsl_debug("%s: %s: hit %p: %s\n", __func__, + lws_hdr_simple_ptr(wsi, WSI_TOKEN_HTTP_COLON_PATH), + hit, hit ? hit->origin : "null"); + if (hit) { + const struct lws_protocols *pp; + const char *name = hit->origin; + + if (hit->protocol) + name = hit->protocol; + + pp = lws_vhost_name_to_protocol(wsi->a.vhost, name); + if (!pp) { + lwsl_info("Unable to find protocol '%s'\n", name); + return 1; } + + if (lws_bind_protocol(wsi, pp, __func__)) + return 1; } + if (lws_http_get_uri_and_method(wsi, &uri_ptr, &uri_len) >= 0) + wsi->a.protocol->callback(wsi, LWS_CALLBACK_HTTP, + wsi->user_space, uri_ptr, (size_t)uri_len); + + lwsl_notice("%s: setting LRS_BODY from 0x%x (%s)\n", __func__, + (int)wsi->wsistate, wsi->a.protocol->name); + + lwsi_set_state(wsi, LRS_BODY); + + if (wsi->http.content_length_explicitly_zero) + return 0; + + /* + * Dump any stashed body + */ + + while (((!wsi->http.content_length_given) || + wsi->http.rx_content_length) && + (blen = lws_buflist_next_segment_len(&wsi->buflist, &buffered))) { + + if ((size_t)wsi->http.rx_content_length < blen) + blen = (size_t)wsi->http.rx_content_length; + + if (wsi->a.protocol->callback(wsi, LWS_CALLBACK_HTTP_BODY, + wsi->user_space, buffered, blen)) + return 1; + lws_buflist_use_segment(&wsi->buflist, blen); + + wsi->http.rx_content_length -= blen; + } + + if (!wsi->buflist) + /* Take us off the pt's "wsi holding input buflist" list */ + lws_dll2_remove(&wsi->dll_buflist); + + if (wsi->http.content_length_given && wsi->http.rx_content_length) + /* still a-ways to go */ + return 0; + + if (!wsi->http.content_length_given && !wsi->h2.END_STREAM) + return 0; + + if (wsi->a.protocol->callback(wsi, LWS_CALLBACK_HTTP_BODY_COMPLETION, + wsi->user_space, NULL, 0)) + return 1; return 0; } @@ -988,6 +1020,17 @@ rops_perform_user_POLLOUT_h2(struct lws *wsi) lwsi_set_state(w, LRS_ESTABLISHED); + if (w->buflist) { + struct lws_context_per_thread *pt; + + pt = &w->a.context->pt[(int)w->tsi]; + lwsl_debug("%s: added %s to rxflow list\n", + __func__, lws_wsi_tag(w)); + lws_dll2_add_head( + &w->dll_buflist, + &pt->dll_buflist_owner); + } + lws_h2_bind_for_post_before_action(w); /* diff --git a/lib/tls/mbedtls/mbedtls-server.c b/lib/tls/mbedtls/mbedtls-server.c index 1b9d6f546..13d93148e 100644 --- a/lib/tls/mbedtls/mbedtls-server.c +++ b/lib/tls/mbedtls/mbedtls-server.c @@ -315,7 +315,7 @@ lws_tls_server_accept(struct lws *wsi) } m = SSL_get_error(wsi->tls.ssl, n); - lwsl_notice("%s: %s: accept SSL_get_error %d errno %d\n", __func__, + lwsl_debug("%s: %s: accept SSL_get_error %d errno %d\n", __func__, lws_wsi_tag(wsi), m, errno); // mbedtls wrapper only