From c80f4c53139b286515d3ecd8fa1a41456d0865f0 Mon Sep 17 00:00:00 2001
From: Deomid Ryabkov <rojer@cesanta.com>
Date: Fri, 30 Mar 2018 20:09:42 +0100
Subject: [PATCH] Fix an edge case in multipart HTTP upload parsing

Consume buffer as soon as we know there is no boundary there, no need to delay until next chunk arrives.
This prevents stall where buffer fills up in one go and next chunk never arrives.

CL: Fix an edge case in multipart HTTP upload parsing

PUBLISHED_FROM=025f9001d272df2a75ece22b199b1944d5db9840
---
 mongoose.c    | 36 +++++++-----------------------------
 src/mg_http.c | 36 +++++++-----------------------------
 2 files changed, 14 insertions(+), 58 deletions(-)

diff --git a/mongoose.c b/mongoose.c
index 5140a0acb..1630eb367 100644
--- a/mongoose.c
+++ b/mongoose.c
@@ -5510,7 +5510,6 @@ enum mg_http_multipart_stream_state {
   MPS_BEGIN,
   MPS_WAITING_FOR_BOUNDARY,
   MPS_WAITING_FOR_CHUNK,
-  MPS_GOT_CHUNK,
   MPS_GOT_BOUNDARY,
   MPS_FINALIZE,
   MPS_FINISHED
@@ -5522,7 +5521,6 @@ struct mg_http_multipart_stream {
   const char *var_name;
   const char *file_name;
   void *user_data;
-  int prev_io_len;
   enum mg_http_multipart_stream_state state;
   int processing_part;
 };
@@ -6358,19 +6356,6 @@ static void mg_http_multipart_call_handler(struct mg_connection *c, int ev,
   pd->mp_stream.user_data = mp.user_data;
 }
 
-static int mg_http_multipart_got_chunk(struct mg_connection *c) {
-  struct mg_http_proto_data *pd = mg_http_get_proto_data(c);
-  struct mbuf *io = &c->recv_mbuf;
-
-  mg_http_multipart_call_handler(c, MG_EV_HTTP_PART_DATA, io->buf,
-                                 pd->mp_stream.prev_io_len);
-  mbuf_remove(io, pd->mp_stream.prev_io_len);
-  pd->mp_stream.prev_io_len = 0;
-  pd->mp_stream.state = MPS_WAITING_FOR_CHUNK;
-
-  return 0;
-}
-
 static int mg_http_multipart_finalize(struct mg_connection *c) {
   struct mg_http_proto_data *pd = mg_http_get_proto_data(c);
 
@@ -6505,19 +6490,18 @@ static int mg_http_multipart_continue_wait_for_chunk(struct mg_connection *c) {
   }
 
   boundary = c_strnstr(io->buf, pd->mp_stream.boundary, io->len);
-  if (boundary == NULL && pd->mp_stream.prev_io_len == 0) {
-    pd->mp_stream.prev_io_len = io->len;
+  if (boundary == NULL) {
+    int data_size = (io->len - (pd->mp_stream.boundary_len + 6));
+    if (data_size > 0) {
+      mg_http_multipart_call_handler(c, MG_EV_HTTP_PART_DATA, io->buf,
+                                     data_size);
+      mbuf_remove(io, data_size);
+    }
     return 0;
-  } else if (boundary == NULL &&
-             (int) io->len >
-                 pd->mp_stream.prev_io_len + pd->mp_stream.boundary_len + 4) {
-    pd->mp_stream.state = MPS_GOT_CHUNK;
-    return 1;
   } else if (boundary != NULL) {
     int data_size = (boundary - io->buf - 4);
     mg_http_multipart_call_handler(c, MG_EV_HTTP_PART_DATA, io->buf, data_size);
     mbuf_remove(io, (boundary - io->buf));
-    pd->mp_stream.prev_io_len = 0;
     pd->mp_stream.state = MPS_WAITING_FOR_BOUNDARY;
     return 1;
   } else {
@@ -6551,12 +6535,6 @@ static void mg_http_multipart_continue(struct mg_connection *c) {
         }
         break;
       }
-      case MPS_GOT_CHUNK: {
-        if (mg_http_multipart_got_chunk(c) == 0) {
-          return;
-        }
-        break;
-      }
       case MPS_FINALIZE: {
         if (mg_http_multipart_finalize(c) == 0) {
           return;
diff --git a/src/mg_http.c b/src/mg_http.c
index 402b8d3b5..1fd27d2ac 100644
--- a/src/mg_http.c
+++ b/src/mg_http.c
@@ -127,7 +127,6 @@ enum mg_http_multipart_stream_state {
   MPS_BEGIN,
   MPS_WAITING_FOR_BOUNDARY,
   MPS_WAITING_FOR_CHUNK,
-  MPS_GOT_CHUNK,
   MPS_GOT_BOUNDARY,
   MPS_FINALIZE,
   MPS_FINISHED
@@ -139,7 +138,6 @@ struct mg_http_multipart_stream {
   const char *var_name;
   const char *file_name;
   void *user_data;
-  int prev_io_len;
   enum mg_http_multipart_stream_state state;
   int processing_part;
 };
@@ -975,19 +973,6 @@ static void mg_http_multipart_call_handler(struct mg_connection *c, int ev,
   pd->mp_stream.user_data = mp.user_data;
 }
 
-static int mg_http_multipart_got_chunk(struct mg_connection *c) {
-  struct mg_http_proto_data *pd = mg_http_get_proto_data(c);
-  struct mbuf *io = &c->recv_mbuf;
-
-  mg_http_multipart_call_handler(c, MG_EV_HTTP_PART_DATA, io->buf,
-                                 pd->mp_stream.prev_io_len);
-  mbuf_remove(io, pd->mp_stream.prev_io_len);
-  pd->mp_stream.prev_io_len = 0;
-  pd->mp_stream.state = MPS_WAITING_FOR_CHUNK;
-
-  return 0;
-}
-
 static int mg_http_multipart_finalize(struct mg_connection *c) {
   struct mg_http_proto_data *pd = mg_http_get_proto_data(c);
 
@@ -1122,19 +1107,18 @@ static int mg_http_multipart_continue_wait_for_chunk(struct mg_connection *c) {
   }
 
   boundary = c_strnstr(io->buf, pd->mp_stream.boundary, io->len);
-  if (boundary == NULL && pd->mp_stream.prev_io_len == 0) {
-    pd->mp_stream.prev_io_len = io->len;
+  if (boundary == NULL) {
+    int data_size = (io->len - (pd->mp_stream.boundary_len + 6));
+    if (data_size > 0) {
+      mg_http_multipart_call_handler(c, MG_EV_HTTP_PART_DATA, io->buf,
+                                     data_size);
+      mbuf_remove(io, data_size);
+    }
     return 0;
-  } else if (boundary == NULL &&
-             (int) io->len >
-                 pd->mp_stream.prev_io_len + pd->mp_stream.boundary_len + 4) {
-    pd->mp_stream.state = MPS_GOT_CHUNK;
-    return 1;
   } else if (boundary != NULL) {
     int data_size = (boundary - io->buf - 4);
     mg_http_multipart_call_handler(c, MG_EV_HTTP_PART_DATA, io->buf, data_size);
     mbuf_remove(io, (boundary - io->buf));
-    pd->mp_stream.prev_io_len = 0;
     pd->mp_stream.state = MPS_WAITING_FOR_BOUNDARY;
     return 1;
   } else {
@@ -1168,12 +1152,6 @@ static void mg_http_multipart_continue(struct mg_connection *c) {
         }
         break;
       }
-      case MPS_GOT_CHUNK: {
-        if (mg_http_multipart_got_chunk(c) == 0) {
-          return;
-        }
-        break;
-      }
       case MPS_FINALIZE: {
         if (mg_http_multipart_finalize(c) == 0) {
           return;
-- 
GitLab