Merge pull request #2630 from cesanta/2593-mongoose-closes-the-connec…

…tion-without-responding-to-post-requests-that-dont-have-explicit-lengths HTTP: respond to messages which have no explicit lengths
cesanta · Feb 28, 2024 · 73a3897 · 73a3897
2 parents 100c4c1 + c59bf66
commit 73a3897
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 2 deletions.
diff --git a/mongoose.c b/mongoose.c
@@ -3205,7 +3205,6 @@ static void http_cb(struct mg_connection *c, int ev, void *ev_data) {
   if (ev == MG_EV_READ || ev == MG_EV_CLOSE) {
     struct mg_http_message hm;
     size_t ofs = 0;  // Parsing offset
-
     while (c->is_resp == 0 && ofs < c->recv.len) {
       const char *buf = (char *) c->recv.buf + ofs;
       int n = mg_http_parse(buf, c->recv.len - ofs, &hm);
@@ -3232,6 +3231,27 @@ static void http_cb(struct mg_connection *c, int ev, void *ev_data) {
           mg_error(c, "Invalid Transfer-Encoding");  // See #2460
           return;
         }
+      } else if (mg_http_get_header(&hm, "Content-length") == NULL) {
+        // #2593: HTTP packets must contain either Transfer-Encoding or
+        // Content-length
+        bool is_response = mg_ncasecmp(hm.method.ptr, "HTTP/", 5) == 0;
+        bool require_content_len = false;
+        if (!is_response && (mg_vcasecmp(&hm.method, "POST") == 0 ||
+            mg_vcasecmp(&hm.method, "PUT") == 0)) {
+          // POST and PUT should include an entity body. Therefore, they should
+          // contain a Content-length header. Other requests can also contain a
+          // body, but their content has no defined semantics (RFC 7231)
+          require_content_len = true;
+        } else if (is_response) {
+          // HTTP spec 7.2 Entity body: All other responses must include a body
+          // or Content-Length header field defined with a value of 0.
+          int status = mg_http_status(&hm);
+          require_content_len = status >= 200 && status != 204 && status != 304;
+        }
+        if (require_content_len) {
+          mg_http_reply(c, 411, "", "");
+          MG_ERROR(("%s", "Content length missing from request"));
+        }
       }
 
       if (is_chunked) {

diff --git a/src/http.c b/src/http.c
@@ -989,7 +989,6 @@ static void http_cb(struct mg_connection *c, int ev, void *ev_data) {
   if (ev == MG_EV_READ || ev == MG_EV_CLOSE) {
     struct mg_http_message hm;
     size_t ofs = 0;  // Parsing offset
-
     while (c->is_resp == 0 && ofs < c->recv.len) {
       const char *buf = (char *) c->recv.buf + ofs;
       int n = mg_http_parse(buf, c->recv.len - ofs, &hm);
@@ -1016,6 +1015,27 @@ static void http_cb(struct mg_connection *c, int ev, void *ev_data) {
           mg_error(c, "Invalid Transfer-Encoding");  // See #2460
           return;
         }
+      } else if (mg_http_get_header(&hm, "Content-length") == NULL) {
+        // #2593: HTTP packets must contain either Transfer-Encoding or
+        // Content-length
+        bool is_response = mg_ncasecmp(hm.method.ptr, "HTTP/", 5) == 0;
+        bool require_content_len = false;
+        if (!is_response && (mg_vcasecmp(&hm.method, "POST") == 0 ||
+            mg_vcasecmp(&hm.method, "PUT") == 0)) {
+          // POST and PUT should include an entity body. Therefore, they should
+          // contain a Content-length header. Other requests can also contain a
+          // body, but their content has no defined semantics (RFC 7231)
+          require_content_len = true;
+        } else if (is_response) {
+          // HTTP spec 7.2 Entity body: All other responses must include a body
+          // or Content-Length header field defined with a value of 0.
+          int status = mg_http_status(&hm);
+          require_content_len = status >= 200 && status != 204 && status != 304;
+        }
+        if (require_content_len) {
+          mg_http_reply(c, 411, "", "");
+          MG_ERROR(("%s", "Content length missing from request"));
+        }
       }
 
       if (is_chunked) {

diff --git a/test/unit_test.c b/test/unit_test.c
@@ -1361,11 +1361,23 @@ static void f4c(struct mg_connection *c, int ev, void *ev_data) {
   }
 }
 
+static void f41(struct mg_connection *c, int ev, void *ev_data) {
+  if (ev == MG_EV_HTTP_MSG) {
+    struct mg_http_message *hm = (struct mg_http_message *) ev_data;
+    mg_printf(c, "HTTP/1.0 200 OK\n\n%.*s/%s", (int) hm->uri.len, hm->uri.ptr,
+              "abcdef");
+  }
+}
+
 static void test_http_no_content_length(void) {
   char buf1[10] = {0}, buf2[10] = {0};
+  char buf[100];
   struct mg_mgr mgr;
   const char *url = "http://127.0.0.1:12348";
   int i;
+  const char *post_req = "POST / HTTP/1.1\r\nContent-Type:"
+                  "b/a\r\nContent-Length: 15\r\n\r\n"
+                  "{\"key\": \"value\"}";
   mg_mgr_init(&mgr);
   mg_http_listen(&mgr, url, f4, (void *) buf1);
   mg_http_connect(&mgr, url, f4c, (void *) buf2);
@@ -1374,6 +1386,16 @@ static void test_http_no_content_length(void) {
   ASSERT(strcmp(buf1, "mc") == 0);
   ASSERT(strcmp(buf2, "mc") == 0);
   mg_mgr_free(&mgr);
+
+  mg_mgr_init(&mgr);
+  mg_http_listen(&mgr, url, f41, (void *) NULL);
+  ASSERT(fetch(&mgr, buf, url, "POST / HTTP/1.1\r\n\r\n") == 411);
+  ASSERT(fetch(&mgr, buf, url, "HTTP/1.1 200\r\n\r\n") == 411);
+  ASSERT(fetch(&mgr, buf, url, "HTTP/1.1 100\r\n\r\n") != 411);
+  ASSERT(fetch(&mgr, buf, url, "HTTP/1.1 304\r\n\r\n") != 411);
+  ASSERT(fetch(&mgr, buf, url, "HTTP/1.1 305\r\n\r\n") == 411);
+  ASSERT(fetch(&mgr, buf, url, post_req) != 411);
+  mg_mgr_free(&mgr);
   ASSERT(mgr.conns == NULL);
 }