From 6375b205a9d3ebe9583871178db258e6a1f4dfff Mon Sep 17 00:00:00 2001 From: Daniel Stenberg Date: Wed, 26 Feb 2020 22:48:09 +0100 Subject: http: added 417 response treatment When doing a request with a body + Expect: 100-continue and the server responds with a 417, the same request will be retried immediately without the Expect: header. Added test 357 to verify. Also added a control instruction to tell the sws test server to not read the request body if Expect: is present, which the new test 357 uses. Reported-by: bramus on github Fixes #4949 Closes #4964 --- lib/http.c | 13 ++++++- lib/urldata.h | 2 + tests/FILEFORMAT | 7 ++-- tests/data/Makefile.inc | 2 +- tests/data/test357 | 97 +++++++++++++++++++++++++++++++++++++++++++++++++ tests/server/sws.c | 26 +++++++++++-- 6 files changed, 137 insertions(+), 10 deletions(-) create mode 100644 tests/data/test357 diff --git a/lib/http.c b/lib/http.c index c500ae0d7..39e4d3447 100644 --- a/lib/http.c +++ b/lib/http.c @@ -1689,7 +1689,7 @@ static CURLcode expect100(struct Curl_easy *data, CURLcode result = CURLE_OK; data->state.expect100header = FALSE; /* default to false unless it is set to TRUE below */ - if(use_http_1_1plus(data, conn) && + if(!data->state.disableexpect && use_http_1_1plus(data, conn) && (conn->httpversion < 20)) { /* if not doing HTTP 1.0 or version 2, or disabled explicitly, we add an Expect: 100-continue to the headers which actually speeds up post @@ -3543,7 +3543,16 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data, */ Curl_expire_done(data, EXPIRE_100_TIMEOUT); if(!k->upload_done) { - if(data->set.http_keep_sending_on_error) { + if((k->httpcode == 417) && data->state.expect100header) { + /* 417 Expectation Failed - try again without the Expect + header */ + infof(data, "Got 417 while waiting for a 100\n"); + data->state.disableexpect = TRUE; + DEBUGASSERT(!data->req.newurl); + data->req.newurl = strdup(conn->data->change.url); + Curl_done_sending(conn, k); + } + else if(data->set.http_keep_sending_on_error) { infof(data, "HTTP error before end of send, keep sending\n"); if(k->exp100 > EXP100_SEND_DATA) { k->exp100 = EXP100_SEND_DATA; diff --git a/lib/urldata.h b/lib/urldata.h index 6401f49f2..e1348cf29 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1442,6 +1442,8 @@ struct UrlState { BIT(ftp_trying_alternative); BIT(wildcardmatch); /* enable wildcard matching */ BIT(expect100header); /* TRUE if we added Expect: 100-continue */ + BIT(disableexpect); /* TRUE if Expect: is disabled due to a previous + 417 response */ BIT(use_range); BIT(rangestringalloc); /* the range string is malloc()'ed */ BIT(done); /* set to FALSE when Curl_init_do() is called and set to TRUE diff --git a/tests/FILEFORMAT b/tests/FILEFORMAT index 8a9eb3579..6c6f83dba 100644 --- a/tests/FILEFORMAT +++ b/tests/FILEFORMAT @@ -156,18 +156,17 @@ auth_required if this is set and a POST/PUT is made without auth, the idle do nothing after receiving the request, just "sit idle" stream continuously send data to the client, never-ending writedelay: [secs] delay this amount between reply packets -skip: [num] instructs the server to ignore reading this many bytes from a PUT - or POST request - +skip: [num] instructs the server to ignore reading this many bytes from a + PUT or POST request rtp: part [num] channel [num] size [num] stream a fake RTP packet for the given part on a chosen channel with the given payload size - connection-monitor When used, this will log [DISCONNECT] to the server.input log when the connection is disconnected. upgrade when an HTTP upgrade header is found, the server will upgrade to http2 swsclose instruct server to close connection after response +no-expect don't read the request body if Expect: is present For TFTP: writedelay: [secs] delay this amount between reply packets (each packet being diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc index d07d2885a..dfc74320e 100644 --- a/tests/data/Makefile.inc +++ b/tests/data/Makefile.inc @@ -59,7 +59,7 @@ test316 test317 test318 test319 test320 test321 test322 test323 test324 \ test325 test326 test327 test328 test329 test330 test331 test332 test333 \ test334 test335 test336 test337 test338 test339 test340 test341 test342 \ test343 \ -test350 test351 test352 test353 test354 test355 test356 \ +test350 test351 test352 test353 test354 test355 test356 test357 \ test393 test394 test395 \ \ test400 test401 test402 test403 test404 test405 test406 test407 test408 \ diff --git a/tests/data/test357 b/tests/data/test357 new file mode 100644 index 000000000..d0437c685 --- /dev/null +++ b/tests/data/test357 @@ -0,0 +1,97 @@ + + + +HTTP +HTTP PUT +Expect + + +# Server-side + +# 417 means the server didn't like the Expect header + +HTTP/1.1 417 OK swsbounce +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 0 + + + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 10 + +blablabla + + +HTTP/1.1 417 OK swsbounce +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 0 + +HTTP/1.1 200 OK +Date: Thu, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Length: 10 + +blablabla + + +no-expect + + + +# Client-side + + +http + + +HTTP PUT with Expect: 100-continue and 417 response + + +http://%HOSTIP:%HTTPPORT/we/want/357 -T log/test357.txt + + +Weird + file + to + upload +for + testing +the + PUT + feature + + + +# Verify data after the test has been "shot" + + +^User-Agent:.* + + +PUT /we/want/357 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 78 +Expect: 100-continue + +PUT /we/want/357 HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* +Content-Length: 78 + +Weird + file + to + upload +for + testing +the + PUT + feature + + + diff --git a/tests/server/sws.c b/tests/server/sws.c index 65f8aa619..eed2d5fd9 100644 --- a/tests/server/sws.c +++ b/tests/server/sws.c @@ -118,6 +118,8 @@ struct httprequest { int rcmd; /* doing a special command, see defines above */ int prot_version; /* HTTP version * 10 */ int callcount; /* times ProcessRequest() gets called */ + bool skipall; /* skip all incoming data */ + bool noexpect; /* refuse Expect: (don't read the body) */ bool connmon; /* monitor the state of the connection, log disconnects */ bool upgrade; /* test case allows upgrade to http2 */ bool upgrade_request; /* upgrade request found and allowed */ @@ -179,6 +181,9 @@ const char *serverlogfile = DEFAULT_LOGFILE; /* close connection */ #define CMD_SWSCLOSE "swsclose" +/* deny Expect: requests */ +#define CMD_NOEXPECT "no-expect" + #define END_OF_HEADERS "\r\n\r\n" enum { @@ -427,6 +432,10 @@ static int parse_servercmd(struct httprequest *req) logmsg("instructed to skip this number of bytes %d", num); req->skip = num; } + else if(!strncmp(CMD_NOEXPECT, cmd, strlen(CMD_NOEXPECT))) { + logmsg("instructed to reject Expect: 100-continue"); + req->noexpect = TRUE; + } else if(1 == sscanf(cmd, "writedelay: %d", &num)) { logmsg("instructed to delay %d secs between packets", num); req->writedelay = num; @@ -735,19 +744,28 @@ static int ProcessRequest(struct httprequest *req) req->open = FALSE; /* closes connection */ return 1; /* done */ } - req->cl = clen - req->skip; + if(req->skipall) + req->cl = 0; + else + req->cl = clen - req->skip; logmsg("Found Content-Length: %lu in the request", clen); if(req->skip) logmsg("... but will abort after %zu bytes", req->cl); - break; } else if(strncasecompare("Transfer-Encoding: chunked", line, strlen("Transfer-Encoding: chunked"))) { /* chunked data coming in */ chunked = TRUE; } - + else if(req->noexpect && + strncasecompare("Expect: 100-continue", line, + strlen("Expect: 100-continue"))) { + if(req->cl) + req->cl = 0; + req->skipall = TRUE; + logmsg("Found Expect: 100-continue, ignore body"); + } if(chunked) { if(strstr(req->reqbuf, "\r\n0\r\n\r\n")) { @@ -939,6 +957,8 @@ static void init_httprequest(struct httprequest *req) req->digest = FALSE; req->ntlm = FALSE; req->skip = 0; + req->skipall = FALSE; + req->noexpect = FALSE; req->writedelay = 0; req->rcmd = RCMD_NORMALREQ; req->prot_version = 0; -- cgit v1.2.1