From 96972ec1c00a142e3859efc82a06b0b810527da2 Mon Sep 17 00:00:00 2001 From: Patrick Monnerat Date: Fri, 6 Mar 2020 09:46:39 +0100 Subject: [PATCH] mime: latch last read callback status. In case a read callback returns a status (pause, abort, eof, error) instead of a byte count, drain the bytes read so far but remember this status for further processing. Takes care of not losing data when pausing, and properly resume a paused mime structure when requested. New tests 670-673 check unpausing cases, with easy or multi interface and mime or form api. Fixes #4813 Reported-by: MrdUkk on github --- lib/easy.c | 8 ++ lib/formdata.c | 12 +- lib/mime.c | 39 +++++- lib/mime.h | 3 + tests/data/Makefile.inc | 1 + tests/data/test644 | 1 + tests/data/test670 | 72 +++++++++++ tests/data/test671 | 72 +++++++++++ tests/data/test672 | 72 +++++++++++ tests/data/test673 | 72 +++++++++++ tests/libtest/Makefile.inc | 17 +++ tests/libtest/lib643.c | 13 +- tests/libtest/lib670.c | 259 +++++++++++++++++++++++++++++++++++++ 13 files changed, 628 insertions(+), 13 deletions(-) create mode 100644 tests/data/test670 create mode 100644 tests/data/test671 create mode 100644 tests/data/test672 create mode 100644 tests/data/test673 create mode 100644 tests/libtest/lib670.c diff --git a/lib/easy.c b/lib/easy.c index d3a29f48b..b648e80c1 100644 --- a/lib/easy.c +++ b/lib/easy.c @@ -1003,6 +1003,14 @@ CURLcode curl_easy_pause(struct Curl_easy *data, int action) return CURLE_OK; } + /* Unpause parts in active mime tree. */ + if((k->keepon & ~newstate & KEEP_SEND_PAUSE) && + (data->mstate == CURLM_STATE_PERFORM || + data->mstate == CURLM_STATE_TOOFAST) && + data->state.fread_func == (curl_read_callback) Curl_mime_read) { + Curl_mime_unpause(data->state.in); + } + /* put it back in the keepon */ k->keepon = newstate; diff --git a/lib/formdata.c b/lib/formdata.c index 50a37e0e7..57ec6ad25 100644 --- a/lib/formdata.c +++ b/lib/formdata.c @@ -728,14 +728,10 @@ int curl_formget(struct curl_httppost *form, void *arg, if(!nread) break; - switch(nread) { - default: - if(append(arg, buffer, nread) != nread) - result = CURLE_READ_ERROR; - break; - case CURL_READFUNC_ABORT: - case CURL_READFUNC_PAUSE: - break; + if(nread > sizeof(buffer) || append(arg, buffer, nread) != nread) { + result = CURLE_READ_ERROR; + if(nread == CURL_READFUNC_ABORT) + result = CURLE_ABORTED_BY_CALLBACK; } } diff --git a/lib/mime.c b/lib/mime.c index 2571287c6..5f928a171 100644 --- a/lib/mime.c +++ b/lib/mime.c @@ -740,9 +740,19 @@ static size_t read_part_content(curl_mimepart *part, { size_t sz = 0; - if(part->readfunc) - sz = part->readfunc(buffer, 1, bufsize, part->arg); - return sz; + switch(part->lastreadstatus) { + case 0: + case CURL_READFUNC_ABORT: + case CURL_READFUNC_PAUSE: + case READ_ERROR: + break; + default: + if(part->readfunc) + sz = part->readfunc(buffer, 1, bufsize, part->arg); + part->lastreadstatus = sz; + break; + } + return part->lastreadstatus; } /* Read and encode part content. */ @@ -1031,6 +1041,7 @@ static int mime_part_rewind(curl_mimepart *part) if(res == CURL_SEEKFUNC_OK) mimesetstate(&part->state, targetstate, NULL); + part->lastreadstatus = 1; /* Successful read status. */ return res; } @@ -1073,6 +1084,7 @@ static void cleanup_part_content(curl_mimepart *part) part->datasize = (curl_off_t) 0; /* No size yet. */ cleanup_encoder_state(&part->encstate); part->kind = MIMEKIND_NONE; + part->lastreadstatus = 1; /* Successful read status. */ } static void mime_subparts_free(void *ptr) @@ -1238,6 +1250,7 @@ void Curl_mime_initpart(curl_mimepart *part, struct Curl_easy *easy) { memset((char *) part, 0, sizeof(*part)); part->easy = easy; + part->lastreadstatus = 1; /* Successful read status. */ mimesetstate(&part->state, MIMESTATE_BEGIN, NULL); } @@ -1805,6 +1818,26 @@ CURLcode Curl_mime_prepare_headers(curl_mimepart *part, return ret; } +/* Recursively reset paused status in the given part. */ +void Curl_mime_unpause(curl_mimepart *part) +{ + if(part) { + if(part->lastreadstatus == CURL_READFUNC_PAUSE) + part->lastreadstatus = 1; /* Successful read status. */ + if(part->kind == MIMEKIND_MULTIPART) { + curl_mime *mime = (curl_mime *) part->arg; + + if(mime) { + curl_mimepart *subpart; + + for(subpart = mime->firstpart; subpart; subpart = subpart->nextpart) + Curl_mime_unpause(subpart); + } + } + } +} + + #else /* !CURL_DISABLE_HTTP || !CURL_DISABLE_SMTP || !CURL_DISABLE_IMAP */ /* Mime not compiled in: define stubs for externally-referenced functions. */ diff --git a/lib/mime.h b/lib/mime.h index 431212579..c6d374ec1 100644 --- a/lib/mime.h +++ b/lib/mime.h @@ -125,6 +125,7 @@ struct curl_mimepart_s { mime_state state; /* Current readback state. */ const mime_encoder *encoder; /* Content data encoder. */ mime_encoder_state encstate; /* Data encoder state. */ + size_t lastreadstatus; /* Last read callback returned status. */ }; CURLcode Curl_mime_add_header(struct curl_slist **slp, const char *fmt, ...); @@ -147,6 +148,7 @@ size_t Curl_mime_read(char *buffer, size_t size, size_t nitems, void *instream); CURLcode Curl_mime_rewind(curl_mimepart *part); const char *Curl_mime_contenttype(const char *filename); +void Curl_mime_unpause(curl_mimepart *part); #else /* if disabled */ @@ -158,6 +160,7 @@ const char *Curl_mime_contenttype(const char *filename); #define Curl_mime_size(x) (curl_off_t) -1 #define Curl_mime_read NULL #define Curl_mime_rewind(x) ((void)x, CURLE_NOT_BUILT_IN) +#define Curl_mime_unpause(x) #endif diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index 9afbf7437..99052007f 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -86,6 +86,7 @@ test635 test636 test637 test638 test639 test640 test641 test642 \ test643 test644 test645 test646 test647 test648 test649 test650 test651 \ test652 test653 test654 test655 test656 test658 test659 test660 test661 \ test662 test663 test664 test665 \ +test670 test671 test672 test673 \ \ test700 test701 test702 test703 test704 test705 test706 test707 test708 \ test709 test710 test711 test712 test713 test714 test715 test716 test717 \ diff --git a/tests/data/test644 b/tests/data/test644 index 4c9a501ed..256d3379a 100644 --- a/tests/data/test644 +++ b/tests/data/test644 @@ -50,6 +50,7 @@ Content-Type: multipart/form-data; boundary=---------------------------- ------------------------------ Content-Disposition: form-data; name="sendfile"; filename="postit2.c" + # CURLE_ABORTED_BY_CALLBACK (42) diff --git a/tests/data/test670 b/tests/data/test670 new file mode 100644 index 000000000..19a51a4e0 --- /dev/null +++ b/tests/data/test670 @@ -0,0 +1,72 @@ + + + +HTTP +HTTP POST +MIME + + + +# +# Server-side + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + + +# Client-side + + +http + +# tool is what to use instead of 'curl' + +lib670 + + + +Request pause from mime read callback: multi + + +http://%HOSTIP:%HTTPPORT/670 + + + +# +# Verify data after the test has been "shot" + + +s/^--------------------------[a-z0-9]*/------------------------------/ +s/boundary=------------------------[a-z0-9]*/boundary=----------------------------/ + + +POST /670 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 142 +Content-Type: multipart/form-data; boundary=---------------------------- + +------------------------------ +Content-Disposition: form-data; name="field" + +AB +-------------------------------- + + + diff --git a/tests/data/test671 b/tests/data/test671 new file mode 100644 index 000000000..eada50a6e --- /dev/null +++ b/tests/data/test671 @@ -0,0 +1,72 @@ + + + +HTTP +HTTP POST +MIME + + + +# +# Server-side + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + + +# Client-side + + +http + +# tool is what to use instead of 'curl' + +lib671 + + + +Request pause from mime read callback: easy + + +http://%HOSTIP:%HTTPPORT/671 + + + +# +# Verify data after the test has been "shot" + + +s/^--------------------------[a-z0-9]*/------------------------------/ +s/boundary=------------------------[a-z0-9]*/boundary=----------------------------/ + + +POST /671 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 142 +Content-Type: multipart/form-data; boundary=---------------------------- + +------------------------------ +Content-Disposition: form-data; name="field" + +AB +-------------------------------- + + + diff --git a/tests/data/test672 b/tests/data/test672 new file mode 100644 index 000000000..9c5f24556 --- /dev/null +++ b/tests/data/test672 @@ -0,0 +1,72 @@ + + + +HTTP +HTTP POST +FORM + + + +# +# Server-side + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + + +# Client-side + + +http + +# tool is what to use instead of 'curl' + +lib672 + + + +Request pause from form read callback: multi + + +http://%HOSTIP:%HTTPPORT/672 + + + +# +# Verify data after the test has been "shot" + + +s/^--------------------------[a-z0-9]*/------------------------------/ +s/boundary=------------------------[a-z0-9]*/boundary=----------------------------/ + + +POST /672 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 142 +Content-Type: multipart/form-data; boundary=---------------------------- + +------------------------------ +Content-Disposition: form-data; name="field" + +AB +-------------------------------- + + + diff --git a/tests/data/test673 b/tests/data/test673 new file mode 100644 index 000000000..efed2727b --- /dev/null +++ b/tests/data/test673 @@ -0,0 +1,72 @@ + + + +HTTP +HTTP POST +FORM + + + +# +# Server-side + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake swsclose +Connection: close +Content-Type: text/html + +hello + + + +# Client-side + + +http + +# tool is what to use instead of 'curl' + +lib673 + + + +Request pause from form read callback: easy + + +http://%HOSTIP:%HTTPPORT/673 + + + +# +# Verify data after the test has been "shot" + + +s/^--------------------------[a-z0-9]*/------------------------------/ +s/boundary=------------------------[a-z0-9]*/boundary=----------------------------/ + + +POST /673 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 142 +Content-Type: multipart/form-data; boundary=---------------------------- + +------------------------------ +Content-Disposition: form-data; name="field" + +AB +-------------------------------- + + + diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index 95ef8309b..9652f03fe 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -23,6 +23,7 @@ noinst_PROGRAMS = chkhostname libauthretry libntlmconnect \ lib583 lib585 lib586 lib587 lib589 lib590 lib591 lib597 lib598 lib599 \ lib643 lib644 lib645 lib650 lib651 lib652 lib653 lib654 lib655 lib658 \ lib659 lib661 \ + lib670 lib671 lib672 lib673 \ lib1156 \ lib1500 lib1501 lib1502 lib1503 lib1504 lib1505 lib1506 lib1507 lib1508 \ lib1509 lib1510 lib1511 lib1512 lib1513 lib1514 lib1515 lib1517 \ @@ -348,6 +349,22 @@ lib659_CPPFLAGS = $(AM_CPPFLAGS) lib661_SOURCES = lib661.c $(SUPPORTFILES) lib661_CPPFLAGS = $(AM_CPPFLAGS) +lib670_SOURCES = lib670.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib670_LDADD = $(TESTUTIL_LIBS) +lib670_CPPFLAGS = $(AM_CPPFLAGS) -DLIB670 + +lib671_SOURCES = lib670.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib671_LDADD = $(TESTUTIL_LIBS) +lib671_CPPFLAGS = $(AM_CPPFLAGS) -DLIB671 + +lib672_SOURCES = lib670.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib672_LDADD = $(TESTUTIL_LIBS) +lib672_CPPFLAGS = $(AM_CPPFLAGS) -DLIB672 + +lib673_SOURCES = lib670.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib673_LDADD = $(TESTUTIL_LIBS) +lib673_CPPFLAGS = $(AM_CPPFLAGS) -DLIB673 + lib1500_SOURCES = lib1500.c $(SUPPORTFILES) $(TESTUTIL) lib1500_LDADD = $(TESTUTIL_LIBS) lib1500_CPPFLAGS = $(AM_CPPFLAGS) diff --git a/tests/libtest/lib643.c b/tests/libtest/lib643.c index 7432dfce8..08c0f2e80 100644 --- a/tests/libtest/lib643.c +++ b/tests/libtest/lib643.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2017, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -41,11 +41,20 @@ struct WriteThis { static size_t read_callback(char *ptr, size_t size, size_t nmemb, void *userp) { #ifdef LIB644 + static int count = 0; (void)ptr; (void)size; (void)nmemb; (void)userp; - return CURL_READFUNC_ABORT; + switch(count++) { + case 0: /* Return a single byte. */ + *ptr = '\n'; + return 1; + case 1: /* Request abort. */ + return CURL_READFUNC_ABORT; + } + printf("Wrongly called >2 times\n"); + exit(1); /* trigger major failure */ #else struct WriteThis *pooh = (struct WriteThis *)userp; diff --git a/tests/libtest/lib670.c b/tests/libtest/lib670.c new file mode 100644 index 000000000..2e6040714 --- /dev/null +++ b/tests/libtest/lib670.c @@ -0,0 +1,259 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) 1998 - 2020, Daniel Stenberg, , et al. + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at https://curl.haxx.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + ***************************************************************************/ + +#include + +#include "test.h" + +#include "memdebug.h" + +#define PAUSE_TIME 2 + + +static const char name[] = "field"; + +struct ReadThis { + CURL *easy; + time_t origin; + int count; +}; + + +static size_t read_callback(char *ptr, size_t size, size_t nmemb, void *userp) +{ + struct ReadThis *pooh = (struct ReadThis *) userp; + time_t delta; + + if(size * nmemb < 1) + return 0; + + switch(pooh->count++) { + case 0: + *ptr = '\x41'; /* ASCII A. */ + return 1; + case 1: + pooh->origin = time(NULL); + return CURL_READFUNC_PAUSE; + case 2: + delta = time(NULL) - pooh->origin; + *ptr = delta >= PAUSE_TIME? '\x42': '\x41'; /* ASCII A or B. */ + return 1; + case 3: + return 0; + } + fprintf(stderr, "Read callback called after EOF\n"); + exit(1); +} + +#if !defined(LIB670) && !defined(LIB672) +static int xferinfo(void *clientp, curl_off_t dltotal, curl_off_t dlnow, + curl_off_t ultotal, curl_off_t ulnow) +{ + struct ReadThis *pooh = (struct ReadThis *) clientp; + + (void) dltotal; + (void) dlnow; + (void) ultotal; + (void) ulnow; + + if(pooh->origin) { + time_t delta = time(NULL) - pooh->origin; + + if(delta >= 4 * PAUSE_TIME) { + fprintf(stderr, "unpausing failed: drain problem?\n"); + return CURLE_ABORTED_BY_CALLBACK; + } + + if(delta >= PAUSE_TIME) + curl_easy_pause(pooh->easy, CURLPAUSE_CONT); + } + + return 0; +} +#endif + +int test(char *URL) +{ +#if defined(LIB670) || defined(LIB671) + curl_mime *mime = NULL; + curl_mimepart *part; +#else + CURLFORMcode formrc; + struct curl_httppost *formpost = NULL; + struct curl_httppost *lastptr = NULL; +#endif +#if defined(LIB670) || defined(LIB672) + CURLM *multi = NULL; + CURLMcode mres; + CURLMsg *msg; + int msgs_left; + int still_running = 0; +#endif + + struct ReadThis pooh; + CURLcode result; + int res = TEST_ERR_FAILURE; + + /* + * Check proper pausing/unpausing from a mime or form read callback. + */ + + if(curl_global_init(CURL_GLOBAL_ALL) != CURLE_OK) { + fprintf(stderr, "curl_global_init() failed\n"); + return TEST_ERR_MAJOR_BAD; + } + + pooh.origin = (time_t) 0; + pooh.count = 0; + pooh.easy = curl_easy_init(); + + /* First set the URL that is about to receive our POST. */ + test_setopt(pooh.easy, CURLOPT_URL, URL); + + /* get verbose debug output please */ + test_setopt(pooh.easy, CURLOPT_VERBOSE, 1L); + + /* include headers in the output */ + test_setopt(pooh.easy, CURLOPT_HEADER, 1L); + +#if defined(LIB670) || defined(LIB671) + /* Build the mime tree. */ + mime = curl_mime_init(pooh.easy); + part = curl_mime_addpart(mime); + result = curl_mime_name(part, name); + if(!result) + res = curl_mime_data_cb(part, (curl_off_t) 2, read_callback, + NULL, NULL, &pooh); + + if(result) { + fprintf(stderr, + "Something went wrong when building the mime structure: %d\n", + (int) result); + goto test_cleanup; + } + + /* Bind mime data to its easy handle. */ + if(!res) + test_setopt(pooh.easy, CURLOPT_MIMEPOST, mime); +#else + /* Build the form. */ + formrc = curl_formadd(&formpost, &lastptr, + CURLFORM_COPYNAME, name, + CURLFORM_STREAM, &pooh, + CURLFORM_CONTENTLEN, (curl_off_t) 2, + CURLFORM_END); + if(formrc) { + fprintf(stderr, "curl_formadd() = %d\n", (int) formrc); + goto test_cleanup; + } + + /* We want to use our own read function. */ + test_setopt(pooh.easy, CURLOPT_READFUNCTION, read_callback); + + /* Send a multi-part formpost. */ + test_setopt(pooh.easy, CURLOPT_HTTPPOST, formpost); +#endif + +#if defined(LIB670) || defined(LIB672) + /* Use the multi interface. */ + multi = curl_multi_init(); + mres = curl_multi_add_handle(multi, pooh.easy); + while(!mres) { + struct timeval timeout; + int rc = 0; + fd_set fdread; + fd_set fdwrite; + fd_set fdexcept; + int maxfd = -1; + + mres = curl_multi_perform(multi, &still_running); + if(!still_running || mres != CURLM_OK) + break; + + if(pooh.origin) { + time_t delta = time(NULL) - pooh.origin; + + if(delta >= 4 * PAUSE_TIME) { + fprintf(stderr, "unpausing failed: drain problem?\n"); + res = CURLE_OPERATION_TIMEDOUT; + break; + } + + if(delta >= PAUSE_TIME) + curl_easy_pause(pooh.easy, CURLPAUSE_CONT); + } + + FD_ZERO(&fdread); + FD_ZERO(&fdwrite); + FD_ZERO(&fdexcept); + timeout.tv_sec = 0; + timeout.tv_usec = 1000000 * PAUSE_TIME / 10; + mres = curl_multi_fdset(multi, &fdread, &fdwrite, &fdexcept, &maxfd); + if(mres) + break; +#ifdef _WIN32 + if(maxfd == -1) + Sleep(100); + else +#endif + rc = select(maxfd + 1, &fdread, &fdwrite, &fdexcept, &timeout); + if(rc == -1) { + fprintf(stderr, "Select error\n"); + break; + } + } + + if(mres != CURLM_OK) + for(;;) { + msg = curl_multi_info_read(multi, &msgs_left); + if(!msg) + break; + if(msg->msg == CURLMSG_DONE) { + result = msg->data.result; + res = (int) result; + } + } + + curl_multi_remove_handle(multi, pooh.easy); + curl_multi_cleanup(multi); + +#else + /* Use the easy interface. */ + test_setopt(pooh.easy, CURLOPT_XFERINFODATA, &pooh); + test_setopt(pooh.easy, CURLOPT_XFERINFOFUNCTION, xferinfo); + test_setopt(pooh.easy, CURLOPT_NOPROGRESS, 0L); + result = curl_easy_perform(pooh.easy); + res = (int) result; +#endif + + +test_cleanup: + curl_easy_cleanup(pooh.easy); +#if defined(LIB670) || defined(LIB671) + curl_mime_free(mime); +#else + curl_formfree(formpost); +#endif + + curl_global_cleanup(); + return res; +}