From c67a99ff27036c824be15f01e21b91c3ec08da4a Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Fri, 29 Aug 2008 10:47:59 +0000 Subject: [PATCH] - When libcurl was doing a HTTP POST and the server would respond with "Connection: close" and actually close the connection after the response-body, libcurl could still have outstanding data to send and it would not properly notice this and stop sending. This caused weirdness and sad faces. http://curl.haxx.se/bug/view.cgi?id=2080222 Note that there are still reasons to consider libcurl's behavior when getting a >= 400 response code while sending data, as Craig Perras' note "http upload: how to stop on error" specifies: http://curl.haxx.se/mail/archive-2008-08/0138.html --- CHANGES | 12 +++++++++ RELEASE-NOTES | 1 + TODO-RELEASE | 4 +-- lib/transfer.c | 9 +++++++ tests/FILEFORMAT | 10 ++++--- tests/data/test1070 | 65 +++++++++++++++++++++++++++++++++++++++++++++ tests/server/sws.c | 36 ++++++++++++++++++++----- 7 files changed, 123 insertions(+), 14 deletions(-) create mode 100644 tests/data/test1070 diff --git a/CHANGES b/CHANGES index fd8b8f1cc3..8b7b527073 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,18 @@ Changelog +Daniel Stenberg (29 Aug 2008) +- When libcurl was doing a HTTP POST and the server would respond with + "Connection: close" and actually close the connection after the + response-body, libcurl could still have outstanding data to send and it + would not properly notice this and stop sending. This caused weirdness and + sad faces. http://curl.haxx.se/bug/view.cgi?id=2080222 + + Note that there are still reasons to consider libcurl's behavior when + getting a >= 400 response code while sending data, as Craig Perras' note + "http upload: how to stop on error" specifies: + http://curl.haxx.se/mail/archive-2008-08/0138.html + Daniel Stenberg (28 Aug 2008) - Dengminwen reported that libcurl would lock a (cookie) share twice (without an unlock in between) for a certain case and that in fact works when using diff --git a/RELEASE-NOTES b/RELEASE-NOTES index bee89a0696..fc38993547 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -60,6 +60,7 @@ This release includes the following bugfixes: o proxy closing connect during CONNECT with auth with the multi interface o CURLOPT_UPLOAD sets HTTP method back to GET or HEAD when passed in a 0 o shared cookies could get locked twice + o deal with closed connection while doing POST/PUT This release includes the following known bugs: diff --git a/TODO-RELEASE b/TODO-RELEASE index 19e5120240..662748c9e8 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -15,8 +15,6 @@ To be addressed before 7.19.1 (planned release: October 2008) http://curl.haxx.se/mail/archive-2008-08/0075.html 162 - Craig Perras' note "http upload: how to stop on error" - http://curl.haxx.se/mail/archive-2008-08/0138.html Most possibly this is - also closely related to the bug "race condition while POSTing to - HTTP/1.0 servers" => http://curl.haxx.se/bug/view.cgi?id=2080222 + http://curl.haxx.se/mail/archive-2008-08/0138.html 163 - diff --git a/lib/transfer.c b/lib/transfer.c index 3294ba530d..2fd6d4cf3e 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -656,6 +656,15 @@ static CURLcode readwrite_data(struct SessionHandle *data, } while(data_pending(conn)); + if(((k->keepon & (KEEP_READ|KEEP_WRITE)) == KEEP_WRITE) && + conn->bits.close ) { + /* When we've read the entire thing and the close bit is set, the server may + now close the connection. If there's now any kind of sending going on from + our side, we need to stop that immediately. */ + infof(data, "we are done reading and this is set to close, stop send\n"); + k->keepon &= ~KEEP_WRITE; /* no writing anymore either */ + } + return CURLE_OK; } diff --git a/tests/FILEFORMAT b/tests/FILEFORMAT index 555cee52de..e4be47ab40 100644 --- a/tests/FILEFORMAT +++ b/tests/FILEFORMAT @@ -113,12 +113,14 @@ PASVBADIP - makes PASV send back an illegal IP in its 227 response For HTTP/HTTPS: -auth_required - if this is set and a POST/PUT is made without auth, the +auth_required if this is set and a POST/PUT is made without auth, the server will NOT wait for the full request body to get sent -idle - do nothing after receiving the request, just "sit idle" -stream - continuously send data to the client, never-ending -pipe: [num] - tell the server to expect this many HTTP requests before +idle do nothing after receiving the request, just "sit idle" +stream continuously send data to the client, never-ending +pipe: [num] tell the server to expect this many HTTP requests before sending back anything, to allow pipelining tests +skip: [num] instructs the server to ignore reading this many bytes from a PUT + or POST request diff --git a/tests/data/test1070 b/tests/data/test1070 new file mode 100644 index 0000000000..715e4a08da --- /dev/null +++ b/tests/data/test1070 @@ -0,0 +1,65 @@ + + + +HTTP +HTTP POST + + +# +# Server-side + + +HTTP/1.1 403 Go away and swsclose +Server: test-server/fake +Content-Type: text/html +Content-Length: 55 +Connection: close + +you are not supposed to be allowed to send things here + + +skip: 2300 + + + +# +# Client-side + + +http + + +HTTP POST with server sending error before (all) data is received + + + -d @log/input1070 http://%HOSTIP:%HTTPPORT/1070 + + +This creates the named file with this content before the test case is run, +which is useful if the test case needs a file to act on. We create this file +rather large (larger than your typical TCP packet) so that not all of it can nor +will be sent in one go as that is kind of the point of this test! + +Here's 2000 x 'O': +OOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOOO + + + +# +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +POST /1070 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 2313 +Content-Type: application/x-www-form-urlencoded +Expect: 100-continue + +This creates + + + diff --git a/tests/server/sws.c b/tests/server/sws.c index a114cac432..f658d18639 100644 --- a/tests/server/sws.c +++ b/tests/server/sws.c @@ -99,6 +99,10 @@ struct httprequest { bool ntlm; /* Authorization ntlm header found */ int pipe; /* if non-zero, expect this many requests to do a "piped" request/response */ + int skip; /* if non-zero, the server is instructed to not read this + many bytes from a PUT/POST request. Ie the client sends N + bytes said in Content-Length, but the server only reads N + - skip bytes. */ int rcmd; /* doing a special command, see defines above */ int prot_version; /* HTTP version * 10 */ bool pipelining; /* true if request is pipelined */ @@ -303,6 +307,13 @@ int ProcessRequest(struct httprequest *req) req->pipe = num-1; /* decrease by one since we don't count the first request in this number */ } + else if(1 == sscanf(cmd, "skip: %d", &num)) { + logmsg("instructed to skip this number of bytes %d", num); + req->skip = num; + } + else { + logmsg("funny instruction found: %s", cmd); + } free(cmd); } } @@ -351,7 +362,7 @@ int ProcessRequest(struct httprequest *req) headers, for the pipelining case mostly */ req->checkindex += (end - line) + strlen(END_OF_HEADERS); - /* **** Persistency **** + /* **** Persistence **** * * If the request is a HTTP/1.0 one, we close the connection unconditionally * when we're done. @@ -363,14 +374,17 @@ int ProcessRequest(struct httprequest *req) */ do { - if(!req->cl && curlx_strnequal("Content-Length:", line, 15)) { + if((req->cl<=0) && curlx_strnequal("Content-Length:", line, 15)) { /* If we don't ignore content-length, we read it and we read the whole request including the body before we return. If we've been told to ignore the content-length, we will return as soon as all headers have been received */ - req->cl = strtol(line+15, &line, 10); + size_t cl = strtol(line+15, &line, 10); + req->cl = cl - req->skip; - logmsg("Found Content-Length: %d in the request", req->cl); + logmsg("Found Content-Length: %d in the request", cl); + if(req->skip) + logmsg("... but will abort after %d bytes", req->cl); break; } else if(curlx_strnequal("Transfer-Encoding: chunked", line, @@ -457,7 +471,7 @@ int ProcessRequest(struct httprequest *req) if(req->auth_req && !req->auth) return 1; - if(req->cl) { + if(req->cl > 0) { if(req->cl <= req->offset - (end - req->reqbuf) - strlen(END_OF_HEADERS)) return 1; /* done */ else @@ -552,6 +566,7 @@ static int get_request(curl_socket_t sock, struct httprequest *req) req->digest = FALSE; req->ntlm = FALSE; req->pipe = 0; + req->skip = 0; req->rcmd = RCMD_NORMALREQ; req->prot_version = 0; req->pipelining = FALSE; @@ -564,8 +579,15 @@ static int get_request(curl_socket_t sock, struct httprequest *req) got = pipereq_length; pipereq_length = 0; } - else - got = sread(sock, reqbuf + req->offset, REQBUFSIZ-1 - req->offset); + else { + if(req->skip) + /* we are instructed to not read the entire thing, so we make sure to only + read what we're supposed to and NOT read the enire thing the client + wants to send! */ + got = sread(sock, reqbuf + req->offset, req->cl); + else + got = sread(sock, reqbuf + req->offset, REQBUFSIZ-1 - req->offset); + } if (got <= 0) { if (got < 0) { logmsg("recv() returned error: %d", SOCKERRNO);