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

diff --git a/lib/http.c b/lib/http.c
index efd367b9e4c9f996ca0def7720d9aa70a121f139..8e370a78207b4886511c08ccff3eb29d909f5646 100644 (file)
--- a/lib/http.c
+++ b/lib/http.c
@@ -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);

diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 447c385932fb6e619dae70c4a9929ddb360325b9..78edff66c48ad50e50aeb509c6f85c379087be64 100644 (file)
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -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 \

diff --git a/tests/data/test1474 b/tests/data/test1474
new file mode 100644 (file)
index 0000000..2fe9320
--- /dev/null
+++ b/tests/data/test1474
@@ -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\r
+Host: %HOSTIP:%HTTPPORT\r
+User-Agent: curl/%VERSION\r
+Accept: */*\r
+Content-Length: 1053701\r
+Expect: 100-continue\r
+\r
+%repeat[132 x S]%%repeat[1021 x xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx%0a]%%repeat[60 x x]%[DISCONNECT]
+PUT /we/want/%TESTNUMBER HTTP/1.1\r
+Host: %HOSTIP:%HTTPPORT\r
+User-Agent: curl/%VERSION\r
+Accept: */*\r
+Content-Length: 1053701\r
+\r
+%repeat[132 x S]%%repeat[16462 x xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx%0a]%
+[DISCONNECT]
+</protocol>
+</verify>
+</testcase>

diff --git a/tests/server/sws.c b/tests/server/sws.c
index c23534210aa95d35a895666e2d4efa1cd093ff3b..e6b7e2de1156bfec5fbbef0999dc40b289e7d708 100644 (file)
--- a/tests/server/sws.c
+++ b/tests/server/sws.c
@@ -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);
       }