From 3f9b4afdfd7dd3ea16e07af98bb324b9f4277696 Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Mon, 11 Jul 2011 23:24:45 +0200 Subject: [PATCH] http error response: stop sending when error is received When libcurl has said to the server that there's a POST or PUT coming (with a content-length and all) it has to either deliver that amount of data or it needs to close the connection before trying a second request. Adds test case 1129, 1130 and 1131 The bug report is about when used with 100-continue, but the change is more generic. Bug: http://curl.haxx.se/mail/lib-2011-06/0191.html Reported by: Steven Parkes --- lib/http.c | 67 +++++++++++++++++++++++------ lib/transfer.c | 6 +++ tests/data/Makefile.am | 7 +-- tests/data/test1129 | 97 ++++++++++++++++++++++++++++++++++++++++++ tests/data/test1130 | 97 ++++++++++++++++++++++++++++++++++++++++++ tests/data/test1131 | 95 +++++++++++++++++++++++++++++++++++++++++ 6 files changed, 354 insertions(+), 15 deletions(-) create mode 100644 tests/data/test1129 create mode 100644 tests/data/test1130 create mode 100644 tests/data/test1131 diff --git a/lib/http.c b/lib/http.c index bb6af2cb4..5c529231f 100644 --- a/lib/http.c +++ b/lib/http.c @@ -404,8 +404,10 @@ static CURLcode http_perhapsrewind(struct connectdata *conn) data left to send, keep on sending. */ /* rewind data when completely done sending! */ - if(!conn->bits.authneg) + if(!conn->bits.authneg) { conn->bits.rewindaftersend = TRUE; + infof(data, "Rewind stream after send\n"); + } return CURLE_OK; } @@ -1683,6 +1685,7 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) if(!data->state.first_host) return CURLE_OUT_OF_MEMORY; } + http->writebytecount = http->readbytecount = 0; if((conn->handler->protocol&(CURLPROTO_HTTP|CURLPROTO_FTP)) && data->set.upload) { @@ -2543,6 +2546,17 @@ CURLcode Curl_http(struct connectdata *conn, bool *done) Curl_pgrsSetUploadCounter(data, http->writebytecount); if(Curl_pgrsUpdate(conn)) result = CURLE_ABORTED_BY_CALLBACK; + + if(http->writebytecount >= postsize) { + /* already sent the entire request body, mark the "upload" as + complete */ + infof(data, "upload completely sent off: %" FORMAT_OFF_T "out of " + "%" FORMAT_OFF_T " bytes\n", + http->writebytecount, postsize); + data->req.upload_done = TRUE; + data->req.keepon &= ~KEEP_SEND; /* we're done writing */ + data->req.exp100 = EXP100_SEND_DATA; /* already sent */ + } } return result; @@ -2814,17 +2828,6 @@ CURLcode Curl_http_readwrite_headers(struct SessionHandle *data, } } - if(417 == k->httpcode) { - /* - * we got: "417 Expectation Failed" this means: - * we have made a HTTP call and our Expect Header - * seems to cause a problem => abort the write operations - * (or prevent them from starting). - */ - k->exp100 = EXP100_FAILED; - k->keepon &= ~KEEP_SEND; - } - /* * When all the headers have been parsed, see if we should give * up and return an error. @@ -2864,6 +2867,46 @@ CURLcode Curl_http_readwrite_headers(struct SessionHandle *data, if(result) return result; + if(k->httpcode >= 300) { + if((!conn->bits.authneg) && !conn->bits.close && + !conn->bits.rewindaftersend) { + /* + * General treatment of errors when about to send data. Including : + * "417 Expectation Failed", while waiting for 100-continue. + * + * The check for close above is done simply because of something + * else has already deemed the connection to get closed then + * something else should've considered the big picture and we + * avoid this check. + * + * rewindaftersend indicates that something has told libcurl to + * continue sending even if it gets discarded + */ + + switch(data->set.httpreq) { + case HTTPREQ_PUT: + case HTTPREQ_POST: + case HTTPREQ_POST_FORM: + /* We got an error response. If this happened before the whole + * request body has been sent we stop sending and mark the + * connection for closure after we've read the entire response. + */ + if(!k->upload_done) { + infof(data, "HTTP error before end of send, stop sending\n"); + conn->bits.close = TRUE; /* close after this */ + k->upload_done = TRUE; + k->keepon &= ~KEEP_SEND; /* don't send */ + if(data->state.expect100header) + k->exp100 = EXP100_FAILED; + } + break; + + default: /* default label present to avoid compiler warnings */ + break; + } + } + } + if(conn->bits.rewindaftersend) { /* We rewind after a complete send, so thus we continue sending now */ diff --git a/lib/transfer.c b/lib/transfer.c index 94cd6d6f9..1c2afc4a2 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -1043,6 +1043,12 @@ CURLcode Curl_readwrite(struct connectdata *conn, if(result || *done) return result; } + else if(k->keepon & KEEP_RECV) { + DEBUGF(infof(data, "additional stuff not fine %s:%d: %d %d\n", + __FILE__, __LINE__, + select_res & CURL_CSELECT_IN, + conn->bits.stream_was_rewound)); + } /* If we still have writing to do, we check if we have a writable socket. */ if((k->keepon & KEEP_SEND) && (select_res & CURL_CSELECT_OUT)) { diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index ccaa8a36b..9b8e94722 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -71,9 +71,10 @@ test1094 test1095 test1096 test1097 test1098 test1099 test1100 test1101 \ test1102 test1103 test1104 test1105 test1106 test1107 test1108 test1109 \ test1110 test1111 test1112 test1113 test1114 test1115 test1116 test1117 \ test1118 test1119 test1120 test1121 test1122 test1123 test1124 test1125 \ -test1126 test1127 test1128 test1200 test1201 test1202 test1203 test1300 \ -test1301 test1302 test1303 test1304 test1305 test1306 test1307 test1308 \ -test1309 test2000 test2001 test2002 test2003 test2004 test2005 +test1126 test1127 test1128 test1129 test1130 test1131 test1200 test1201 \ +test1202 test1203 test1300 test1301 test1302 test1303 test1304 test1305 \ +test1306 test1307 test1308 test1309 test2000 test2001 test2002 test2003 \ +test2004 test2005 EXTRA_DIST = $(TESTCASES) DISABLED diff --git a/tests/data/test1129 b/tests/data/test1129 new file mode 100644 index 000000000..f47141cd3 --- /dev/null +++ b/tests/data/test1129 @@ -0,0 +1,97 @@ + + + +HTTP +HTTP POST +Expect: 100-continue + + + +# +# Server-side + + +HTTP/1.1 404 NOOOOOOOOO +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- + + + +HTTP/1.1 404 NEITHER +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- + + +# we use skip to make the test server never read the full payload off +# the socket and instead return the response at once + +skip: 1025 + + + +# +# Client-side + +# 1025 x 'x' + +XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + + +http + + +HTTP POST expect 100-continue with a 404 + + +-d @log/file1129 http://%HOSTIP:%HTTPPORT/1129 http://%HOSTIP:%HTTPPORT/11290001 + + + +# +# Verify data after the test has been "shot" + + +HTTP/1.1 404 NOOOOOOOOO +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- +HTTP/1.1 404 NEITHER +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- + + +^User-Agent:.* + + +POST /1129 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 1025 +Content-Type: application/x-www-form-urlencoded +Expect: 100-continue + +POST /11290001 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 1025 +Content-Type: application/x-www-form-urlencoded +Expect: 100-continue + + + + diff --git a/tests/data/test1130 b/tests/data/test1130 new file mode 100644 index 000000000..eb1e59f5b --- /dev/null +++ b/tests/data/test1130 @@ -0,0 +1,97 @@ + + + +HTTP +HTTP POST +Expect: 100-continue + + + +# +# Server-side + + +HTTP/1.1 404 NOOOOOOOOO +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- + + + +HTTP/1.1 404 NEITHER +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- + + +# we use skip to make the test server never read the full payload off +# the socket and instead return the response at once + +skip: 100 + + + +# +# Client-side + +# 100 x 'x' + +XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + + +http + + +HTTP POST forced expect 100-continue with a 404 + + +-d @log/file1130 http://%HOSTIP:%HTTPPORT/1130 http://%HOSTIP:%HTTPPORT/11300001 -H "Expect: 100-continue" + + + +# +# Verify data after the test has been "shot" + + +HTTP/1.1 404 NOOOOOOOOO +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- +HTTP/1.1 404 NEITHER +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 6 +Content-Type: text/html + +-foo- + + +^User-Agent:.* + + +POST /1130 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Expect: 100-continue +Content-Length: 100 +Content-Type: application/x-www-form-urlencoded + +POST /11300001 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Expect: 100-continue +Content-Length: 100 +Content-Type: application/x-www-form-urlencoded + + + + diff --git a/tests/data/test1131 b/tests/data/test1131 new file mode 100644 index 000000000..96843af54 --- /dev/null +++ b/tests/data/test1131 @@ -0,0 +1,95 @@ + + + +HTTP +HTTP PUT +Expect: 100-continue + + + +# +# Server-side + + +HTTP/1.1 400 NOOOOOOOOO +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 9 +Content-Type: text/html + +FAILURE1 + + + +HTTP/1.1 400 NEITHER +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 9 +Content-Type: text/html + +FAILURE2 + + +# we use skip to make the test server never read the full payload off +# the socket and instead return the response at once + +skip: 100 + + + +# +# Client-side + +# 100 x 'x' + +XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX + + +http + + +HTTP PUT expect 100-continue with a 400 + + +-T log/file1131 http://%HOSTIP:%HTTPPORT/1131 -T log/file1131 http://%HOSTIP:%HTTPPORT/11310001 + + + +# +# Verify data after the test has been "shot" + + +HTTP/1.1 400 NOOOOOOOOO +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 9 +Content-Type: text/html + +FAILURE1 +HTTP/1.1 400 NEITHER +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 9 +Content-Type: text/html + +FAILURE2 + + +^User-Agent:.* + + +PUT /1131 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 100 +Expect: 100-continue + +PUT /11310001 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 100 +Expect: 100-continue + + + +