http: fail early when rewind of input failed when following redirects

When inspecting a possible follow HTTP request, the result of a rewind
of the upload data was ignored as it was not clear at that point in time
if the request would become a GET.

This initiated the followup, rewound again, which failed again and
terminated the follow up.

This was confusing to users as it was not clear of the follow up was
done or not.

Fix: fail the early rewind when the request is not converted to GET.

Fixes #17472
Closes #17474
Reported-by: Jeroen Ooms

diff --git a/lib/http.c b/lib/http.c
index 8c593f5a733b2d8c2b525f8e607c7942a738f431..9f519381f03a5c7efba057ab1dada95252dda57a 100644 (file)
--- a/lib/http.c
+++ b/lib/http.c
@@ -1176,6 +1176,8 @@ CURLcode Curl_http_follow(struct Curl_easy *data, const char *newurl,
   bool reachedmax = FALSE;
   char *follow_url = NULL;
   CURLUcode uc;
+  CURLcode rewind_result;
+  bool switch_to_get = FALSE;
 
   DEBUGASSERT(type != FOLLOW_NONE);
 
@@ -1333,7 +1335,7 @@ CURLcode Curl_http_follow(struct Curl_easy *data, const char *newurl,
 
   data->state.url = follow_url;
   data->state.url_alloc = TRUE;
-  Curl_req_soft_reset(&data->req, data);
+  rewind_result = Curl_req_soft_reset(&data->req, data);
   infof(data, "Issue another request to this URL: '%s'", data->state.url);
   if((data->set.http_follow_mode == CURLFOLLOW_FIRSTONLY) &&
      data->set.str[STRING_CUSTOMREQUEST] &&
@@ -1382,8 +1384,10 @@ CURLcode Curl_http_follow(struct Curl_easy *data, const char *newurl,
     if((data->state.httpreq == HTTPREQ_POST
         || data->state.httpreq == HTTPREQ_POST_FORM
         || data->state.httpreq == HTTPREQ_POST_MIME)
-       && !(data->set.keep_post & CURL_REDIR_POST_301))
+       && !(data->set.keep_post & CURL_REDIR_POST_301)) {
       http_switch_to_get(data, 301);
+      switch_to_get = TRUE;
+    }
     break;
   case 302: /* Found */
     /* (quote from RFC7231, section 6.4.3)
@@ -1405,8 +1409,10 @@ CURLcode Curl_http_follow(struct Curl_easy *data, const char *newurl,
     if((data->state.httpreq == HTTPREQ_POST
         || data->state.httpreq == HTTPREQ_POST_FORM
         || data->state.httpreq == HTTPREQ_POST_MIME)
-       && !(data->set.keep_post & CURL_REDIR_POST_302))
+       && !(data->set.keep_post & CURL_REDIR_POST_302)) {
       http_switch_to_get(data, 302);
+      switch_to_get = TRUE;
+    }
     break;
 
   case 303: /* See Other */
@@ -1419,8 +1425,10 @@ CURLcode Curl_http_follow(struct Curl_easy *data, const char *newurl,
        ((data->state.httpreq != HTTPREQ_POST &&
          data->state.httpreq != HTTPREQ_POST_FORM &&
          data->state.httpreq != HTTPREQ_POST_MIME) ||
-        !(data->set.keep_post & CURL_REDIR_POST_303)))
+        !(data->set.keep_post & CURL_REDIR_POST_303))) {
       http_switch_to_get(data, 303);
+      switch_to_get = TRUE;
+    }
     break;
   case 304: /* Not Modified */
     /* 304 means we did a conditional request and it was "Not modified".
@@ -1437,6 +1445,12 @@ CURLcode Curl_http_follow(struct Curl_easy *data, const char *newurl,
      */
     break;
   }
+
+  /* When rewind of upload data failed and we are not switching to GET,
+   * we need to fail the follow, as we cannot send the data again. */
+  if(rewind_result && !switch_to_get)
+    return rewind_result;
+
   Curl_pgrsTime(data, TIMER_REDIRECT);
   Curl_pgrsResetTransferSizes(data);
 

diff --git a/tests/http/clients/hx-upload.c b/tests/http/clients/hx-upload.c
index 6e391ec3eb3b819a1308bc8edd9b8dc6f40d27a6..40b6211af73289b75ec44a028d321170a58a6067 100644 (file)
--- a/tests/http/clients/hx-upload.c
+++ b/tests/http/clients/hx-upload.c
@@ -261,6 +261,7 @@ static int setup(CURL *hnd, const char *url, struct transfer *t,
   curl_easy_setopt(hnd, CURLOPT_SSL_VERIFYPEER, 0L);
   curl_easy_setopt(hnd, CURLOPT_SSL_VERIFYHOST, 0L);
   curl_easy_setopt(hnd, CURLOPT_BUFFERSIZE, (long)(128 * 1024));
+  curl_easy_setopt(hnd, CURLOPT_FOLLOWLOCATION, CURLFOLLOW_OBEYCODE);
   curl_easy_setopt(hnd, CURLOPT_WRITEFUNCTION, my_write_cb);
   curl_easy_setopt(hnd, CURLOPT_WRITEDATA, t);
   if(use_earlydata)
@@ -328,7 +329,6 @@ int main(int argc, char *argv[])
 {
 #ifndef _MSC_VER
   CURLM *multi_handle;
-  struct CURLMsg *m;
   CURLSH *share;
   const char *url;
   const char *method = "PUT";
@@ -505,6 +505,7 @@ int main(int argc, char *argv[])
     do {
       int still_running; /* keep number of running handles */
       CURLMcode mc = curl_multi_perform(multi_handle, &still_running);
+      struct CURLMsg *m;
 
       if(still_running) {
         /* wait for activity, timeout or "nothing" */
@@ -523,8 +524,11 @@ int main(int argc, char *argv[])
           curl_multi_remove_handle(multi_handle, e);
           t = get_transfer_for_easy(e);
           if(t) {
+            long res_status;
+            curl_easy_getinfo(e, CURLINFO_RESPONSE_CODE, &res_status);
             t->done = 1;
-            fprintf(stderr, "[t-%d] FINISHED\n", t->idx);
+            fprintf(stderr, "[t-%d] FINISHED, result=%d, response=%ld\n",
+                    t->idx, m->data.result, res_status);
             if(use_earlydata) {
               curl_off_t sent;
               curl_easy_getinfo(e, CURLINFO_EARLYDATA_SENT_T, &sent);

diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py
index 4f23122ae4cb1568c496fe6dbed02bdf859cba82..da6373a49c47b776fb43033e92b7ef408e220d3a 100644 (file)
--- a/tests/http/test_07_upload.py
+++ b/tests/http/test_07_upload.py
@@ -606,6 +606,30 @@ class TestUpload:
                           extra_args=['--trace-config', 'all'])
         r.check_stats(count=count, http_status=413, exitcode=0)
 
+    @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
+    @pytest.mark.parametrize("httpcode", [301, 302, 307, 308])
+    def test_07_44_put_redir(self, env: Env, httpd, nghttpx, proto, httpcode):
+        if proto == 'h3' and not env.have_h3():
+            pytest.skip("h3 not supported")
+        count = 1
+        upload_size = 128*1024
+        url = f'https://localhost:{env.https_port}/curltest/put-redir-{httpcode}'
+        client = LocalClient(name='hx-upload', env=env)
+        if not client.exists():
+            pytest.skip(f'example client not built: {client.name}')
+        r = client.run(args=[
+             '-n', f'{count}', '-l', '-S', f'{upload_size}', '-V', proto, url
+        ])
+        r.check_exit_code(0)
+        results = [int(m.group(1)) for line in r.trace_lines
+                     if (m := re.match(r'.* FINISHED, result=(\d+), response=(\d+)', line))]
+        httpcodes = [int(m.group(2)) for line in r.trace_lines
+                     if (m := re.match(r'.* FINISHED, result=(\d+), response=(\d+)', line))]
+        if httpcode == 308:
+            assert results[0] == 65, f'{r}'  # could not rewind input
+        else:
+            assert httpcodes[0] == httpcode, f'{r}'
+
     # speed limited on put handler
     @pytest.mark.parametrize("proto", ['http/1.1', 'h2', 'h3'])
     def test_07_50_put_speed_limit(self, env: Env, httpd, nghttpx, proto):

diff --git a/tests/http/testenv/httpd.py b/tests/http/testenv/httpd.py
index f396268124f71f5ae1c6b8279e1ba9bf4f472279..f9ab554ceaf155fef2fe62cf9dd702732e568256 100644 (file)
--- a/tests/http/testenv/httpd.py
+++ b/tests/http/testenv/httpd.py
@@ -549,6 +549,11 @@ class Httpd:
                 '      SetEnv force-response-1.0 1',
                 '    </Location>',
                 '    SetEnvIf Request_URI "/shutdown_unclean" ssl-unclean=1',
+                '    RewriteEngine on',
+                '    RewriteRule    "^/curltest/put-redir-301$"  "/curltest/put"  [R=301]',
+                '    RewriteRule    "^/curltest/put-redir-302$"  "/curltest/put"  [R=302]',
+                '    RewriteRule    "^/curltest/put-redir-307$"  "/curltest/put"  [R=307]',
+                '    RewriteRule    "^/curltest/put-redir-308$"  "/curltest/put"  [R=308]',
             ])
         if self._auth_digest:
             lines.extend([