From 2419f0276634dccf505967df1ca234bc3a68fb84 Mon Sep 17 00:00:00 2001 From: Sergey Lyubka Date: Sun, 11 Feb 2024 09:03:55 +0000 Subject: [PATCH] Fix #2592: do not close pipelined connection prematurely --- mongoose.c | 12 ++++++++---- src/http.c | 12 ++++++++---- test/unit_test.c | 21 ++++++++++++++++++--- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/mongoose.c b/mongoose.c index b18e632f..e5462af9 100644 --- a/mongoose.c +++ b/mongoose.c @@ -2348,7 +2348,7 @@ int mg_http_get_var(const struct mg_str *buf, const char *name, char *dst, size_t dst_len) { int len; if (dst != NULL && dst_len > 0) { - dst[0] = '\0'; // If destination buffer is valid, always nul-terminate it + dst[0] = '\0'; // If destination buffer is valid, always nul-terminate it } if (dst == NULL || dst_len == 0) { len = -2; // Bad destination @@ -3158,7 +3158,7 @@ long mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, res = -3; } else if ((size_t) offset + hm->body.len > max_size) { mg_http_reply(c, 400, "", "%s: over max size of %lu", path, - (unsigned long) max_size); + (unsigned long) max_size); res = -4; } else { struct mg_fd *fd; @@ -3216,8 +3216,12 @@ static void http_cb(struct mg_connection *c, int ev, void *ev_data) { struct mg_str *te; // Transfer - encoding header bool is_chunked = false; if (n < 0) { - mg_error(c, "HTTP parse, %lu bytes", c->recv.len); - mg_hexdump(c->recv.buf, c->recv.len > 16 ? 16 : c->recv.len); + // We don't use mg_error() here, to avoid closing pipelined requests + // prematurely, see #2592 + MG_ERROR(("HTTP parse, %lu bytes", c->recv.len)); + c->is_draining = 1; + mg_hexdump(buf, c->recv.len - ofs > 16 ? 16 : c->recv.len - ofs); + c->recv.len = 0; return; } if (n == 0) break; // Request is not buffered yet diff --git a/src/http.c b/src/http.c index f3a19cc1..ff76dfbf 100644 --- a/src/http.c +++ b/src/http.c @@ -128,7 +128,7 @@ int mg_http_get_var(const struct mg_str *buf, const char *name, char *dst, size_t dst_len) { int len; if (dst != NULL && dst_len > 0) { - dst[0] = '\0'; // If destination buffer is valid, always nul-terminate it + dst[0] = '\0'; // If destination buffer is valid, always nul-terminate it } if (dst == NULL || dst_len == 0) { len = -2; // Bad destination @@ -938,7 +938,7 @@ long mg_http_upload(struct mg_connection *c, struct mg_http_message *hm, res = -3; } else if ((size_t) offset + hm->body.len > max_size) { mg_http_reply(c, 400, "", "%s: over max size of %lu", path, - (unsigned long) max_size); + (unsigned long) max_size); res = -4; } else { struct mg_fd *fd; @@ -996,8 +996,12 @@ static void http_cb(struct mg_connection *c, int ev, void *ev_data) { struct mg_str *te; // Transfer - encoding header bool is_chunked = false; if (n < 0) { - mg_error(c, "HTTP parse, %lu bytes", c->recv.len); - mg_hexdump(c->recv.buf, c->recv.len > 16 ? 16 : c->recv.len); + // We don't use mg_error() here, to avoid closing pipelined requests + // prematurely, see #2592 + MG_ERROR(("HTTP parse, %lu bytes", c->recv.len)); + c->is_draining = 1; + mg_hexdump(buf, c->recv.len - ofs > 16 ? 16 : c->recv.len - ofs); + c->recv.len = 0; return; } if (n == 0) break; // Request is not buffered yet diff --git a/test/unit_test.c b/test/unit_test.c index 5d365466..f17ba432 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -1376,18 +1376,33 @@ static void f5(struct mg_connection *c, int ev, void *ev_data) { } } +static void f6(struct mg_connection *c, int ev, void *ev_data) { + if (ev == MG_EV_HTTP_MSG) { + (*(int *) c->fn_data)++; + (void) ev_data; + } +} + static void test_http_pipeline(void) { struct mg_mgr mgr; const char *url = "http://127.0.0.1:12377"; struct mg_connection *c; - int i, ok = 0; + int i, ok = 0, ok2 = 0; mg_mgr_init(&mgr); mg_http_listen(&mgr, url, f5, (void *) &ok); - c = mg_http_connect(&mgr, url, NULL, NULL); + c = mg_http_connect(&mgr, url, f6, &ok2); mg_printf(c, "POST / HTTP/1.0\nContent-Length: 5\n\n12345GET / HTTP/1.0\n\n"); for (i = 0; i < 20; i++) mg_mgr_poll(&mgr, 1); - // MG_INFO(("-----> [%d]", ok)); ASSERT(ok == 2); + ASSERT(ok2 == 2); + // Fire a valid, then invalid request, see #2592. First one should serve + ok = ok2 = 0; + c = mg_http_connect(&mgr, url, f6, (void *) &ok2); + mg_printf(c, "GET / HTTP/1.1\n\nInvalid\n\n"); + for (i = 0; i < 20; i++) mg_mgr_poll(&mgr, 1); + ASSERT(ok == 1); + ASSERT(ok2 == 1); + //MG_INFO(("-----> [%d] [%d]", ok, ok2)); mg_mgr_free(&mgr); ASSERT(mgr.conns == NULL); }