From 339bbee0df8914370bfba201ea637ba9457de5e9 Mon Sep 17 00:00:00 2001 From: Deomid Ryabkov <rojer@cesanta.com> Date: Wed, 20 Jun 2018 13:51:57 +0100 Subject: [PATCH] mg_file_upload_handler: Support multiple files curl -F file1 -F file2 ... Add a unit test and fix a minor memory leak when returning an error. CL: mg_file_upload_handler: Support multiple files PUBLISHED_FROM=5c4bf2be676346fb782e80f50f79df6a6721ac88 --- mongoose.c | 23 +++++++----- src/mg_http.c | 23 +++++++----- test/test.mk | 2 +- test/unit_test.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 122 insertions(+), 20 deletions(-) diff --git a/mongoose.c b/mongoose.c index f19033d86..a5ad3e7ff 100644 --- a/mongoose.c +++ b/mongoose.c @@ -8279,8 +8279,7 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, case MG_EV_HTTP_PART_BEGIN: { struct mg_http_multipart_part *mp = (struct mg_http_multipart_part *) ev_data; - struct file_upload_state *fus = - (struct file_upload_state *) MG_CALLOC(1, sizeof(*fus)); + struct file_upload_state *fus; struct mg_str lfn = local_name_fn(nc, mg_mk_str(mp->file_name)); mp->user_data = NULL; if (lfn.p == NULL || lfn.len == 0) { @@ -8294,6 +8293,11 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, nc->flags |= MG_F_SEND_AND_CLOSE; return; } + fus = (struct file_upload_state *) MG_CALLOC(1, sizeof(*fus)); + if (fus == NULL) { + nc->flags |= MG_F_CLOSE_IMMEDIATELY; + return; + } fus->lfn = (char *) MG_MALLOC(lfn.len + 1); memcpy(fus->lfn, lfn.p, lfn.len); fus->lfn[lfn.len] = '\0'; @@ -8365,12 +8369,6 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, if (mp->status >= 0 && fus->fp != NULL) { LOG(LL_DEBUG, ("%p Uploaded %s (%s), %d bytes", nc, mp->file_name, fus->lfn, (int) fus->num_recd)); - mg_printf(nc, - "HTTP/1.1 200 OK\r\n" - "Content-Type: text/plain\r\n" - "Connection: close\r\n\r\n" - "Ok, %s - %d bytes.\r\n", - mp->file_name, (int) fus->num_recd); } else { LOG(LL_ERROR, ("Failed to store %s (%s)", mp->file_name, fus->lfn)); /* @@ -8382,6 +8380,15 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, MG_FREE(fus->lfn); MG_FREE(fus); mp->user_data = NULL; + /* Don't close the connection yet, there may be more files to come. */ + break; + } + case MG_EV_HTTP_MULTIPART_REQUEST_END: { + mg_printf(nc, + "HTTP/1.1 200 OK\r\n" + "Content-Type: text/plain\r\n" + "Connection: close\r\n\r\n" + "Ok.\r\n"); nc->flags |= MG_F_SEND_AND_CLOSE; break; } diff --git a/src/mg_http.c b/src/mg_http.c index e7151acaf..8c9fd5f17 100644 --- a/src/mg_http.c +++ b/src/mg_http.c @@ -2713,8 +2713,7 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, case MG_EV_HTTP_PART_BEGIN: { struct mg_http_multipart_part *mp = (struct mg_http_multipart_part *) ev_data; - struct file_upload_state *fus = - (struct file_upload_state *) MG_CALLOC(1, sizeof(*fus)); + struct file_upload_state *fus; struct mg_str lfn = local_name_fn(nc, mg_mk_str(mp->file_name)); mp->user_data = NULL; if (lfn.p == NULL || lfn.len == 0) { @@ -2728,6 +2727,11 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, nc->flags |= MG_F_SEND_AND_CLOSE; return; } + fus = (struct file_upload_state *) MG_CALLOC(1, sizeof(*fus)); + if (fus == NULL) { + nc->flags |= MG_F_CLOSE_IMMEDIATELY; + return; + } fus->lfn = (char *) MG_MALLOC(lfn.len + 1); memcpy(fus->lfn, lfn.p, lfn.len); fus->lfn[lfn.len] = '\0'; @@ -2799,12 +2803,6 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, if (mp->status >= 0 && fus->fp != NULL) { LOG(LL_DEBUG, ("%p Uploaded %s (%s), %d bytes", nc, mp->file_name, fus->lfn, (int) fus->num_recd)); - mg_printf(nc, - "HTTP/1.1 200 OK\r\n" - "Content-Type: text/plain\r\n" - "Connection: close\r\n\r\n" - "Ok, %s - %d bytes.\r\n", - mp->file_name, (int) fus->num_recd); } else { LOG(LL_ERROR, ("Failed to store %s (%s)", mp->file_name, fus->lfn)); /* @@ -2816,6 +2814,15 @@ void mg_file_upload_handler(struct mg_connection *nc, int ev, void *ev_data, MG_FREE(fus->lfn); MG_FREE(fus); mp->user_data = NULL; + /* Don't close the connection yet, there may be more files to come. */ + break; + } + case MG_EV_HTTP_MULTIPART_REQUEST_END: { + mg_printf(nc, + "HTTP/1.1 200 OK\r\n" + "Content-Type: text/plain\r\n" + "Connection: close\r\n\r\n" + "Ok.\r\n"); nc->flags |= MG_F_SEND_AND_CLOSE; break; } diff --git a/test/test.mk b/test/test.mk index 23ad747df..36df546e4 100644 --- a/test/test.mk +++ b/test/test.mk @@ -28,7 +28,7 @@ # OSX clang doesn't build ASAN. Use brew: # $ brew tap homebrew/versions # $ brew install llvm36 --with-clang --with-asan -CLANG:=clang-3.6 +CLANG:=clang PEDANTIC=$(shell gcc --version 2>/dev/null | grep -q clang && echo -pedantic) diff --git a/test/unit_test.c b/test/unit_test.c index 485403595..763b683bd 100644 --- a/test/unit_test.c +++ b/test/unit_test.c @@ -4076,14 +4076,19 @@ static void cb_mp_srv(struct mg_connection *nc, int ev, void *p) { } static void cb_mp_send_one_byte(struct mg_connection *nc, int ev, void *p) { - static int i; + static int i = -1; (void) p; - if (ev == MG_EV_POLL) { + if (ev == MG_EV_CONNECT) { + i = 0; + } else if (i >= 0 && ev == MG_EV_POLL) { char ch = ((char *) nc->user_data)[i++]; int l = strlen((char *) nc->user_data); if (ch != '\0') { mg_send(nc, &ch, 1); DBG(("%p sent %d of %d", (void *) nc, i, l)); + } else { + nc->flags |= MG_F_SEND_AND_CLOSE; + i = -1; } } } @@ -4209,7 +4214,6 @@ static const char *test_http_multipart2(void) { if ((r = test_http_multipart_check_res(&mpd.res)) != NULL) return r; - c->flags |= MG_F_CLOSE_IMMEDIATELY; mbuf_free(&mpd.res); memset(&mpd, 0, sizeof(mpd)); mbuf_init(&mpd.res, 0); @@ -4316,6 +4320,89 @@ static const char *test_http_multipart2(void) { return NULL; } + +static struct mg_str upload_lfn_same(struct mg_connection *nc, + struct mg_str fn) { + if (fn.len == 0) { + fn = mg_strdup(mg_mk_str("bar")); + } + (void) nc; + return fn; +} + +static void cb_mp_srv_upload(struct mg_connection *nc, int ev, void *p) { + mg_file_upload_handler(nc, ev, p, upload_lfn_same); + if (ev == MG_EV_CLOSE && nc->listener != NULL) { + *((int *) nc->listener->user_data) = 1; + } + (void) p; +} + +static const char *test_http_multipart_upload(void) { + struct mg_mgr mgr; + const char req_fmt[] = + "%s" + "Content-Disposition: form-data; name=\"a\"; filename=\"foo\"\r\n" + "\r\n" + "%s" + "\r\n--Asrf456BGe4h\r\n" + "Content-Disposition: form-data; name=\"b\"\r\n" + "\r\n" + "%s" + "\r\n--Asrf456BGe4h\r\n" + "Content-Disposition: form-data; name=\"c\"; filename=\"baz\"\r\n" + "\r\n" + "%s" + "\r\n--Asrf456BGe4h--\r\n" + "\r\n"; + + char req[1024 * 5], *data; + struct mg_connection *c, *lc; + size_t size; + int done = 0; + + mg_mgr_init(&mgr, NULL); + lc = mg_bind(&mgr, "8766", cb_mp_srv_upload); + mg_set_protocol_http_websocket(lc); + lc->user_data = &done; + + (void) remove("foo"); + (void) remove("bar"); + (void) remove("baz"); + + snprintf(req, sizeof(req), req_fmt, "", b1, b2, b4); + + ASSERT((c = mg_connect_http(&mgr, cb_mp_send_one_byte, + "http://127.0.0.1:8766/test", + "Content-Type: " + "multipart/form-data;boundary=Asrf456BGe4h", + "\r\n--Asrf456BGe4h\r\n")) != NULL); + c->user_data = req; + + poll_until(&mgr, 5, c_int_eq, &done, (void *) 1); + + data = read_file("foo", &size); + ASSERT_PTRNE(data, NULL); + ASSERT_STREQ_NZ(data, b1); + (void) remove("foo"); + free(data); + + data = read_file("bar", &size); + ASSERT_PTRNE(data, NULL); + ASSERT_STREQ_NZ(data, b2); + (void) remove("bar"); + free(data); + + data = read_file("baz", &size); + ASSERT_PTRNE(data, NULL); + ASSERT_STREQ_NZ(data, b4); + (void) remove("baz"); + free(data); + + mg_mgr_free(&mgr); + return NULL; +} + #endif /* MG_ENABLE_HTTP_STREAMING_MULTIPART */ static const char *test_http_multipart(void) { @@ -5545,6 +5632,7 @@ const char *tests_run(const char *filter) { RUN_TEST(test_http_multipart); #if MG_ENABLE_HTTP_STREAMING_MULTIPART RUN_TEST(test_http_multipart2); + RUN_TEST(test_http_multipart_upload); #endif RUN_TEST(test_parse_date_string); RUN_TEST(test_websocket); -- GitLab