http: make 416 not fail with resume + CURLOPT_FAILONERRROR

When asked to resume a download, libcurl will convert that to HTTP logic
and if then the entire file is already transferred it will result in a
416 response from the HTTP server. With CURLOPT_FAILONERRROR set in that
scenario, it should *not* lead to an error return.

Updated test 1156, added test 1273

Reported-by: Jonathan Watt
Fixes #6740
Closes #6753

5 files changed, 110 insertions, 21 deletions

diff --git a/lib/http.c b/lib/http.c
index 43eb840de..7ec03e9bd 100644
--- a/lib/http.c
+++ b/lib/http.c
@@ -1095,6 +1095,14 @@ static bool http_should_fail(struct Curl_easy *data)
     return FALSE;
 
   /*
+  ** A 416 response to a resume request is presumably because the file is
+  ** already completely downloaded and thus not actually a fail.
+  */
+  if(data->state.resume_from && data->state.httpreq == HTTPREQ_GET &&
+     httpcode == 416)
+    return FALSE;
+
+  /*
   ** Any code >= 400 that's not 401 or 407 is always
   ** a terminal error
   */
diff --git a/tests/data/Makefile.inc b/tests/data/Makefile.inc
index 6306b5980..2c7a0ca89 100644
--- a/tests/data/Makefile.inc
+++ b/tests/data/Makefile.inc
@@ -153,7 +153,8 @@ test1240 test1241 test1242 test1243 test1244 test1245 test1246 test1247 \
 test1248 test1249 test1250 test1251 test1252 test1253 test1254 test1255 \
 test1256 test1257 test1258 test1259 test1260 test1261 test1262 test1263 \
 test1264 test1265 test1266 test1267 test1268 test1269 test1270 test1271 \
-test1272 \
+test1272 test1273 \
+\
 test1280 test1281 test1282 test1283 test1284 test1285 test1286 test1287 \
 test1288 test1289 test1290 test1291 test1292 test1293 test1294 test1295 \
 test1296 test1297 test1298 test1299 test1300 test1301 test1302 test1303 \
diff --git a/tests/data/test1156 b/tests/data/test1156
index f78dc998f..ff57bb0ec 100644
--- a/tests/data/test1156
+++ b/tests/data/test1156
@@ -58,14 +58,15 @@ lib1156
 HTTP resume/range fail range-error content-range combinations
  </name>
  <command>
-http://%HOSTIP:%HTTPPORT/want/1156
+http://%HOSTIP:%HTTPPORT/want/%TESTNUMBER
 </command>
 </client>
 
 # Verify data after the test has been "shot"
 <verify>
-<errorcode>
+<stderr mode="text">
+URL: http://%HOSTIP:%HTTPPORT/want/%TESTNUMBER
 0
-</errorcode>
+</stderr>
 </verify>
 </testcase>
diff --git a/tests/data/test1273 b/tests/data/test1273
new file mode 100644
index 000000000..61d475a2e
--- /dev/null
+++ b/tests/data/test1273
@@ -0,0 +1,78 @@
+<testcase>
+# also verified by 1156 in libcurl API terms
+
+<info>
+<keywords>
+HTTP
+HTTP GET
+Resume
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data>
+HTTP/1.1 416 Invalid range

+Connection: close

+Content-Length: 0

+

+</data>
+
+# The file data that exists at the start of the test must be included in
+# the verification.
+<datacheck>
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+HTTP/1.1 416 Invalid range

+Connection: close

+Content-Length: 0

+

+</datacheck>
+
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+ <name>
+-f and resume transfer of an entirely-downloaded file
+ </name>
+ <command>
+http://%HOSTIP:%HTTPPORT/%TESTNUMBER -C - -f
+</command>
+<file name="log/curl%TESTNUMBER.out">
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+012345678
+</file>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<protocol>
+GET /%TESTNUMBER HTTP/1.1

+Host: %HOSTIP:%HTTPPORT

+Range: bytes=100-

+User-Agent: curl/%VERSION

+Accept: */*

+

+</protocol>
+</verify>
+</testcase>
diff --git a/tests/libtest/lib1156.c b/tests/libtest/lib1156.c
index 9ffe19a79..d8da643c0 100644
--- a/tests/libtest/lib1156.c
+++ b/tests/libtest/lib1156.c
@@ -5,7 +5,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2021, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -64,8 +64,7 @@ static const struct testparams params[] = {
   { F_RESUME |             F_FAIL | F_CONTENTRANGE,                CURLE_OK },
   { F_RESUME | F_HTTP416 |                           F_IGNOREBODY, CURLE_OK },
   { F_RESUME | F_HTTP416 |          F_CONTENTRANGE | F_IGNOREBODY, CURLE_OK },
-  { F_RESUME | F_HTTP416 | F_FAIL |                  F_IGNOREBODY,
-                                                  CURLE_HTTP_RETURNED_ERROR },
+  { F_RESUME | F_HTTP416 | F_FAIL |                  F_IGNOREBODY, CURLE_OK },
   { F_RESUME | F_HTTP416 | F_FAIL | F_CONTENTRANGE | F_IGNOREBODY,
                                                   CURLE_HTTP_RETURNED_ERROR }
 };
@@ -82,7 +81,8 @@ static size_t writedata(char *data, size_t size, size_t nmemb, void *userdata)
   return size * nmemb;
 }
 
-static int onetest(CURL *curl, const char *url, const struct testparams *p)
+static int onetest(CURL *curl, const char *url, const struct testparams *p,
+                   size_t num)
 {
   CURLcode res;
   unsigned int replyselector;
@@ -100,22 +100,22 @@ static int onetest(CURL *curl, const char *url, const struct testparams *p)
   hasbody = 0;
   res = curl_easy_perform(curl);
   if(res != p->result) {
-    fprintf(stderr, "bad error code (%d): resume=%s, fail=%s, http416=%s, "
-                    "content-range=%s, expected=%d\n", res,
-                    (p->flags & F_RESUME)? "yes": "no",
-                    (p->flags & F_FAIL)? "yes": "no",
-                    (p->flags & F_HTTP416)? "yes": "no",
-                    (p->flags & F_CONTENTRANGE)? "yes": "no",
-                    p->result);
+    fprintf(stderr, "%d: bad error code (%d): resume=%s, fail=%s, http416=%s, "
+            "content-range=%s, expected=%d\n", num, res,
+            (p->flags & F_RESUME)? "yes": "no",
+            (p->flags & F_FAIL)? "yes": "no",
+            (p->flags & F_HTTP416)? "yes": "no",
+            (p->flags & F_CONTENTRANGE)? "yes": "no",
+            p->result);
     return 1;
   }
   if(hasbody && (p->flags & F_IGNOREBODY)) {
     fprintf(stderr, "body should be ignored and is not: resume=%s, fail=%s, "
-                    "http416=%s, content-range=%s\n",
-                    (p->flags & F_RESUME)? "yes": "no",
-                    (p->flags & F_FAIL)? "yes": "no",
-                    (p->flags & F_HTTP416)? "yes": "no",
-                    (p->flags & F_CONTENTRANGE)? "yes": "no");
+            "http416=%s, content-range=%s\n",
+            (p->flags & F_RESUME)? "yes": "no",
+            (p->flags & F_FAIL)? "yes": "no",
+            (p->flags & F_HTTP416)? "yes": "no",
+            (p->flags & F_CONTENTRANGE)? "yes": "no");
     return 1;
   }
   return 0;
@@ -147,10 +147,11 @@ int test(char *URL)
   test_setopt(curl, CURLOPT_WRITEFUNCTION, writedata);
 
   for(i = 0; i < sizeof(params) / sizeof(params[0]); i++)
-    status |= onetest(curl, URL, params + i);
+    status |= onetest(curl, URL, params + i, i);
 
   curl_easy_cleanup(curl);
   curl_global_cleanup();
+  fprintf(stderr, "%d\n", status);
   return status;
 
   test_cleanup: