From: Stefan Eissing Date: Wed, 28 May 2025 11:27:19 +0000 (+0200) Subject: http: fail early when rewind of input failed when following redirects X-Git-Tag: curl-8_14_1~44 X-Git-Url: http://git.ipfire.org/gitweb/gitweb.cgi?a=commitdiff_plain;h=71bb004c29631117ab03d90a4214d1515da5a895;p=thirdparty%2Fcurl.git 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 8c593f5a73..9f519381f0 100644 --- 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 6e391ec3eb..40b6211af7 100644 --- 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 4f23122ae4..da6373a49c 100644 --- 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 f396268124..f9ab554cea 100644 --- a/tests/http/testenv/httpd.py +++ b/tests/http/testenv/httpd.py @@ -549,6 +549,11 @@ class Httpd: ' SetEnv force-response-1.0 1', ' ', ' 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([