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

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;

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

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

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 \

tests/data/test357

@@ -0,0 +1,97 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP PUT
+Expect
+</keywords>
+</info>
+# Server-side
+<reply>
+# 417 means the server didn't like the Expect header
+<data>
+HTTP/1.1 417 OK swsbounce
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 0
+
+</data>
+<data1>
+HTTP/1.1 200 OK
+Date: Thu, 09 Nov 2010 14:49:00 GMT
+Server: test-server/fake
+Content-Length: 10
+
+blablabla
+</data1>
+<datacheck>
+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
+</datacheck>
+<servercmd>
+no-expect
+</servercmd>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+HTTP PUT with Expect: 100-continue and 417 response
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/we/want/357 -T log/test357.txt
+</command>
+<file name="log/test357.txt">
+Weird
+     file
+         to
+   upload
+for
+   testing
+the
+   PUT
+      feature
+</file>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<strip>
+^User-Agent:.*
+</strip>
+<protocol>
+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
+</protocol>
+</verify>
+</testcase>

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;