From cbd58fa4586bf458b0eed104eda02ea6cea65df1 Mon Sep 17 00:00:00 2001 From: Andy Green Date: Fri, 5 Jul 2019 06:07:03 +0100 Subject: [PATCH] http: body: make sure to consume body before transaction complete https://github.com/warmcat/libwebsockets/issues/1625 "dead bodies" that were sent but not processed by lws as server will clog up and destroy transaction tracking if repeated POSTs with keepalive are sent to nonexistant paths. This patch introduces a DISCARD_BODY state that follows BODY except the payload is not signalled to the protocol callback. Calling transaction_completed() with pending body makes lws enter DISCARD_BODY and retry transaction completed only after the pending body is exhausted. --- lib/roles/h1/ops-h1.c | 18 ++++++++++++++++++ lib/roles/h2/ops-h2.c | 3 ++- lib/roles/http/server/server.c | 31 ++++++++++++++++++++++++++++--- lib/roles/private.h | 1 + lib/roles/ws/ops-ws.c | 3 ++- 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/lib/roles/h1/ops-h1.c b/lib/roles/h1/ops-h1.c index fd9401d6d..f73845588 100644 --- a/lib/roles/h1/ops-h1.c +++ b/lib/roles/h1/ops-h1.c @@ -101,6 +101,7 @@ lws_read_h1(struct lws *wsi, unsigned char *buf, lws_filepos_t len) goto read_ok; case LRS_ISSUING_FILE: goto read_ok; + case LRS_DISCARD_BODY: case LRS_BODY: wsi->http.rx_content_remain = wsi->http.rx_content_length; @@ -114,6 +115,7 @@ lws_read_h1(struct lws *wsi, unsigned char *buf, lws_filepos_t len) } break; + case LRS_DISCARD_BODY: case LRS_BODY: http_postbody: lwsl_debug("%s: http post body: remain %d\n", __func__, @@ -149,11 +151,13 @@ http_postbody: goto bail; } else { #endif + if (lwsi_state(wsi) != LRS_DISCARD_BODY) { n = wsi->protocol->callback(wsi, LWS_CALLBACK_HTTP_BODY, wsi->user_space, buf, (size_t)body_chunk_len); if (n) goto bail; + } n = (size_t)body_chunk_len; #ifdef LWS_WITH_CGI } @@ -183,6 +187,19 @@ postbody_completion: if (!wsi->http.cgi) #endif { +#if !defined(LWS_NO_SERVER) + if (lwsi_state(wsi) == LRS_DISCARD_BODY) { + /* + * repeat the transaction completed + * that got us into this state, having + * consumed the pending body now + */ + + if (lws_http_transaction_completed(wsi)) + return -1; + break; + } +#endif lwsl_info("HTTP_BODY_COMPLETION: %p (%s)\n", wsi, wsi->protocol->name); @@ -313,6 +330,7 @@ lws_h1_server_socket_service(struct lws *wsi, struct lws_pollfd *pollfd) if ((lwsi_state(wsi) == LRS_ESTABLISHED || lwsi_state(wsi) == LRS_ISSUING_FILE || lwsi_state(wsi) == LRS_HEADERS || + lwsi_state(wsi) == LRS_DISCARD_BODY || lwsi_state(wsi) == LRS_BODY)) { if (!wsi->http.ah && lws_header_table_attach(wsi, 0)) { diff --git a/lib/roles/h2/ops-h2.c b/lib/roles/h2/ops-h2.c index 8490181d1..183f81c04 100644 --- a/lib/roles/h2/ops-h2.c +++ b/lib/roles/h2/ops-h2.c @@ -256,7 +256,8 @@ drain: if (ebuf.len) { n = 0; - if (lwsi_role_h2(wsi) && lwsi_state(wsi) != LRS_BODY) + if (lwsi_role_h2(wsi) && lwsi_state(wsi) != LRS_BODY && + lwsi_state(wsi) != LRS_DISCARD_BODY) n = lws_read_h2(wsi, ebuf.token, ebuf.len); else n = lws_read_h1(wsi, ebuf.token, ebuf.len); diff --git a/lib/roles/http/server/server.c b/lib/roles/http/server/server.c index 2433149b8..a2465e77d 100644 --- a/lib/roles/http/server/server.c +++ b/lib/roles/http/server/server.c @@ -1268,7 +1268,8 @@ lws_http_action(struct lws *wsi) lws_hdr_copy(wsi, content_length_str, sizeof(content_length_str) - 1, WSI_TOKEN_HTTP_CONTENT_LENGTH) > 0) { - wsi->http.rx_content_length = atoll(content_length_str); + wsi->http.rx_content_remain = wsi->http.rx_content_length = + atoll(content_length_str); if (!wsi->http.rx_content_length) { wsi->http.content_length_explicitly_zero = 1; lwsl_debug("%s: explicit 0 content-length\n", __func__); @@ -1652,9 +1653,11 @@ deal_body: if (wsi->http.rx_content_length > 0) { - lwsi_set_state(wsi, LRS_BODY); - lwsl_info("%s: %p: LRS_BODY state set (0x%x)\n", + if (lwsi_state(wsi) != LRS_DISCARD_BODY) { + lwsi_set_state(wsi, LRS_BODY); + lwsl_info("%s: %p: LRS_BODY state set (0x%x)\n", __func__, wsi, wsi->wsistate); + } wsi->http.rx_content_remain = wsi->http.rx_content_length; @@ -2140,6 +2143,28 @@ lws_http_transaction_completed(struct lws *wsi) return 0; } + /* + * Are we finishing the transaction before we have consumed any body? + * + * For h1 this would kill keepalive pipelining, and for h2, considering + * it can extend over multiple DATA frames, it would kill the network + * connection. + */ + if (wsi->http.rx_content_length && wsi->http.rx_content_remain) { + /* + * are we already in LRS_DISCARD_BODY and didn't clear the + * remaining before trying to complete the transaction again? + */ + if (lwsi_state(wsi) == LRS_DISCARD_BODY) + return -1; + /* + * let's defer transaction completed processing until we + * discarded the remaining body + */ + lwsi_set_state(wsi, LRS_DISCARD_BODY); + + return 0; + } lwsl_info("%s: wsi %p\n", __func__, wsi); diff --git a/lib/roles/private.h b/lib/roles/private.h index d8fcfeac5..55d955004 100644 --- a/lib/roles/private.h +++ b/lib/roles/private.h @@ -121,6 +121,7 @@ enum lwsi_state { LRS_ISSUING_FILE = 20, LRS_HEADERS = 21, LRS_BODY = 22, + LRS_DISCARD_BODY = 31, LRS_ESTABLISHED = LWSIFS_POCB | 23, /* we are established, but we have embarked on serving a single * transaction. Other transaction input may be pending, but we will diff --git a/lib/roles/ws/ops-ws.c b/lib/roles/ws/ops-ws.c index f5c4c567d..4fbeb3691 100644 --- a/lib/roles/ws/ops-ws.c +++ b/lib/roles/ws/ops-ws.c @@ -1199,7 +1199,8 @@ drain: //lws_buflist_describe(&wsi->buflist, wsi); if (ebuf.len) { #if defined(LWS_ROLE_H2) - if (lwsi_role_h2(wsi) && lwsi_state(wsi) != LRS_BODY) + if (lwsi_role_h2(wsi) && lwsi_state(wsi) != LRS_BODY && + lwsi_state(wsi) != LRS_DISCARD_BODY) n = lws_read_h2(wsi, ebuf.token, ebuf.len); else