http: close the connection after a late 417 is received

In this situation, only part of the data has been sent before aborting
so the connection is no longer usable.

Assisted-by: Jay Satiro
Fixes #11678
Closes #11679
This commit is contained in:
Dan Fandrich 2023-08-15 13:43:07 -07:00
parent 0d89c9096d
commit 86179afcbb
4 changed files with 137 additions and 2 deletions

View File

@ -4270,7 +4270,18 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
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");
if(!k->writebytecount &&
k->exp100 == EXP100_AWAITING_CONTINUE) {
infof(data, "Got HTTP failure 417 while waiting for a 100");
}
else {
infof(data, "Got HTTP failure 417 while sending data");
streamclose(conn,
"Stop sending data before everything sent");
result = http_perhapsrewind(data, conn);
if(result)
return result;
}
data->state.disableexpect = TRUE;
DEBUGASSERT(!data->req.newurl);
data->req.newurl = strdup(data->state.url);

View File

@ -185,7 +185,7 @@ test1439 test1440 test1441 test1442 test1443 test1444 test1445 test1446 \
test1447 test1448 test1449 test1450 test1451 test1452 test1453 test1454 \
test1455 test1456 test1457 test1458 test1459 test1460 test1461 test1462 \
test1463 test1464 test1465 test1466 test1467 test1468 test1469 test1470 \
test1471 test1472 test1473 \
test1471 test1472 test1473 test1474 \
\
test1500 test1501 test1502 test1503 test1504 test1505 test1506 test1507 \
test1508 test1509 test1510 test1511 test1512 test1513 test1514 test1515 \

111
tests/data/test1474 Normal file
View File

@ -0,0 +1,111 @@
<testcase>
# This test is quite timing dependent and tricky to set up. The time line of
# test operations looks like this:
#
# 1. curl sends a PUT request with Expect: 100-continue and waits only 1 msec
# for a 100 response.
# 2. The HTTP server accepts the connection but waits 500 msec before acting
# on the request.
# 3. curl doesn't receive the expected 100 response before its timeout expires,
# so it starts sending the body. It is throttled by a --limit-rate, so it
# sends the first 64 KiB then stops for 1000 msec due to this
# throttling.
# 4. The server sends its 417 response while curl is throttled.
# 5. curl responds to this 417 response by closing the connection (because it
# has a half-completed response outstanding) and starting a new one. This
# new request does not have an Expect: header so it is sent without delay.
# It's still throttled, however, so it takes about 16 seconds to finish
# sending.
# 6. The server receives the response and this time acks it with 200.
#
# Because of the timing sensitivity (scheduling delays of 500 msec can cause
# the test to fail), this test is marked flaky to avoid it being run in the CI
# builds which are often run on overloaded servers.
# Increasing the --limit-rate would decrease the test time, but at the cost of
# becoming even more sensitive to delays (going from 500 msec to 250 msec or
# less of accepted delay before failure).
<info>
<keywords>
HTTP
HTTP PUT
Expect
flaky
</keywords>
</info>
# Server-side
<reply>
# 417 means the server didn't like the Expect header
<data>
HTTP/1.1 417 BAD swsbounce
Date: Tue, 09 Nov 2010 14:49:00 GMT
Server: test-server/fake
Content-Length: 0
</data>
<data1>
HTTP/1.1 200 OK
Date: Tue, 09 Nov 2010 14:49:00 GMT
Server: test-server/fake
Content-Length: 10
blablabla
</data1>
<datacheck>
HTTP/1.1 417 BAD swsbounce
Date: Tue, 09 Nov 2010 14:49:00 GMT
Server: test-server/fake
Content-Length: 0
HTTP/1.1 200 OK
Date: Tue, 09 Nov 2010 14:49:00 GMT
Server: test-server/fake
Content-Length: 10
blablabla
</datacheck>
<servercmd>
no-expect
delay: 500
connection-monitor
</servercmd>
</reply>
# Client-side
<client>
<server>
http
</server>
<name>
HTTP PUT with Expect: 100-continue and 417 response during upload
</name>
<command>
http://%HOSTIP:%HTTPPORT/we/want/%TESTNUMBER -T %LOGDIR/test%TESTNUMBER.txt --limit-rate 64K --expect100-timeout 0.001
</command>
# Must be large enough to trigger curl's automatic 100-continue behaviour
<file name="%LOGDIR/test%TESTNUMBER.txt">
%repeat[132 x S]%%repeat[16462 x xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx%0a]%
</file>
</client>
# Verify data after the test has been "shot"
<verify>
<protocol>
PUT /we/want/%TESTNUMBER HTTP/1.1
Host: %HOSTIP:%HTTPPORT
User-Agent: curl/%VERSION
Accept: */*
Content-Length: 1053701
Expect: 100-continue
%repeat[132 x S]%%repeat[1021 x xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx%0a]%%repeat[60 x x]%[DISCONNECT]
PUT /we/want/%TESTNUMBER HTTP/1.1
Host: %HOSTIP:%HTTPPORT
User-Agent: curl/%VERSION
Accept: */*
Content-Length: 1053701
%repeat[132 x S]%%repeat[16462 x xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx%0a]%
[DISCONNECT]
</protocol>
</verify>
</testcase>

View File

@ -2389,6 +2389,19 @@ int main(int argc, char *argv[])
/* Reset the request, unless we're still in the middle of reading */
if(rc && !req->upgrade_request)
/* Note: resetting the HTTP request here can cause problems if:
* 1) req->skipall is TRUE,
* 2) the socket is still open, and
* 3) (stale) data is still available (or about to be available)
* on that socket
* In that case, this loop will run once more and treat that stale
* data (in service_connection()) as the first data received on
* this new HTTP request and report "** Unusual request" (skipall
* would have otherwise caused that data to be ignored). Normally,
* that socket will be closed by the client and there won't be any
* stale data to cause this, but stranger things have happened (see
* issue #11678).
*/
init_httprequest(req);
} while(rc > 0);
}