http: have CURLOPT_FAILONERROR fail after all headers

... so that Retry-After and other meta-content can still be used. Added 1634 to verify. Adjusted test 194 and 281 since --fail now also includes the header-terminating CRLF in the output before it exits. Fixes curl#6408 Closes curl#6409
wasix-org · Jan 6, 2021 · ab525c0 · ab525c0
1 parent 9d7281f
commit ab525c0
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 37 deletions.
diff --git a/lib/http.c b/lib/http.c
@@ -96,7 +96,7 @@
 
 static int http_getsock_do(struct connectdata *conn,
                            curl_socket_t *socks);
-static int http_should_fail(struct connectdata *conn);
+static bool http_should_fail(struct connectdata *conn);
 
 #ifndef CURL_DISABLE_PROXY
 static CURLcode add_haproxy_protocol_header(struct connectdata *conn);
@@ -1046,11 +1046,11 @@ CURLcode Curl_http_input_auth(struct connectdata *conn, bool proxy,
  *
  * @param conn all information about the current connection
  *
- * @retval 0 communications should continue
+ * @retval FALSE communications should continue
  *
- * @retval 1 communications should not continue
+ * @retval TRUE communications should not continue
  */
-static int http_should_fail(struct connectdata *conn)
+static bool http_should_fail(struct connectdata *conn)
 {
   struct Curl_easy *data;
   int httpcode;
@@ -1066,20 +1066,20 @@ static int http_should_fail(struct connectdata *conn)
   ** don't fail.
   */
   if(!data->set.http_fail_on_error)
-    return 0;
+    return FALSE;
 
   /*
   ** Any code < 400 is never terminal.
   */
   if(httpcode < 400)
-    return 0;
+    return FALSE;
 
   /*
   ** Any code >= 400 that's not 401 or 407 is always
   ** a terminal error
   */
   if((httpcode != 401) && (httpcode != 407))
-    return 1;
+    return TRUE;
 
   /*
   ** All we have left to deal with is 401 and 407
@@ -3309,12 +3309,6 @@ checkprotoprefix(struct Curl_easy *data, struct connectdata *conn,
   return checkhttpprefix(data, s, len);
 }
 
-static void print_http_error(struct Curl_easy *data)
-{
-  struct SingleRequest *k = &data->req;
-  failf(data, "The requested URL returned error: %d", k->httpcode);
-}
-
 /*
  * Curl_http_header() parses a single response header.
  */
@@ -3646,16 +3640,6 @@ CURLcode Curl_http_statusline(struct Curl_easy *data,
        pretend this is no error */
     k->ignorebody = TRUE; /* Avoid appending error msg to good data. */
   }
-  else if(data->set.http_fail_on_error && (k->httpcode >= 400) &&
-          ((k->httpcode != 401) || !conn->bits.user_passwd)
-#ifndef CURL_DISABLE_PROXY
-          && ((k->httpcode != 407) || !conn->bits.proxy_user_passwd)
-#endif
-    ) {
-    /* serious error, go home! */
-    print_http_error(data);
-    return CURLE_HTTP_RETURNED_ERROR;
-  }
 
   if(conn->httpversion == 10) {
     /* Default action for HTTP/1.0 must be to close, unless
@@ -3928,15 +3912,6 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
         conn->proxy_negotiate_state = GSS_AUTHSUCC;
       }
 #endif
-      /*
-       * When all the headers have been parsed, see if we should give
-       * up and return an error.
-       */
-      if(http_should_fail(conn)) {
-        failf(data, "The requested URL returned error: %d",
-              k->httpcode);
-        return CURLE_HTTP_RETURNED_ERROR;
-      }
 
       /* now, only output this if the header AND body are requested:
        */
@@ -3954,6 +3929,16 @@ CURLcode Curl_http_readwrite_headers(struct Curl_easy *data,
       data->info.header_size += (long)headerlen;
       data->req.headerbytecount += (long)headerlen;
 
+      /*
+       * When all the headers have been parsed, see if we should give
+       * up and return an error.
+       */
+      if(http_should_fail(conn)) {
+        failf(data, "The requested URL returned error: %d",
+              k->httpcode);
+        return CURLE_HTTP_RETURNED_ERROR;
+      }
+
       data->req.deductheadercount =
         (100 <= k->httpcode && 199 >= k->httpcode)?data->req.headerbytecount:0;
 

diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
@@ -196,7 +196,7 @@ test1608 test1609 test1610 test1611 test1612 \
 \
 test1620 test1621 \
 \
-test1630 test1631 test1632 test1633 \
+test1630 test1631 test1632 test1633 test1634 \
 \
 test1650 test1651 test1652 test1653 test1654 test1655 \
 test1660 \

diff --git a/tests/data/test1634 b/tests/data/test1634
@@ -0,0 +1,70 @@
+<testcase>
+<info>
+<keywords>
+HTTP
+HTTP GET
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data>
+HTTP/1.1 429 too many requests swsbounce
+Retry-After: 1
+Content-Length: 4
+
+moo
+</data>
+<data1>
+HTTP/1.1 200 OK
+Content-Length: 4
+Connection: close
+
+hey
+</data1>
+
+<datacheck>
+HTTP/1.1 429 too many requests swsbounce
+Retry-After: 1
+Content-Length: 4
+
+HTTP/1.1 200 OK
+Content-Length: 4
+Connection: close
+
+hey
+</datacheck>
+</reply>
+
+#
+# Client-side
+<client>
+<server>
+http
+</server>
+<name>
+--retry with a 429 response and Retry-After: and --fail
+</name>
+<command>
+http://%HOSTIP:%HTTPPORT/1634 --retry 1  --fail
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+GET /1634 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+GET /1634 HTTP/1.1
+Host: %HOSTIP:%HTTPPORT
+User-Agent: curl/%VERSION
+Accept: */*
+
+</protocol>
+</verify>
+</testcase>
diff --git a/tests/data/test194 b/tests/data/test194
@@ -36,6 +36,7 @@ Content-Length: 4
 Content-Range: bytes */87
 Content-Type: image/gif
 Connection: close
+
 </datacheck>
 
 </reply>

diff --git a/tests/data/test281 b/tests/data/test281
@@ -14,11 +14,8 @@ HTTP/1.1 401 Bad Auth swsclose
 Date: Thu, 09 Nov 2010 14:49:00 GMT
 WWW-Authenticate: Basic Realm=authenticate
 Server: test-server/fake
-</data>
-<datacheck>
-HTTP/1.1 100 Continue
 
-</datacheck>
+</data>
 
 </reply>