From: Jay Satiro Date: Mon, 13 Jan 2025 08:57:45 +0000 (-0500) Subject: transfer: fix CURLOPT_CURLU override logic X-Git-Tag: curl-8_12_0~125 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=5ffc73c78ec48f6ddf38c68ae7e8ff41f54801e6;p=thirdparty%2Fcurl.git transfer: fix CURLOPT_CURLU override logic - Change setopt and pretransfer to always reset URL related variables for a CURLU handle set CURLOPT_CURLU. This change is to ensure we are in compliance with the doc which says CURLU handles must be able to override a URL set via CURLOPT_URL and that if the contents of the CURLU handle changes between transfers then the updated contents must be used. Prior to this change, although subsequent transfers appear to be performed correctly in those cases, the work URL `data->state.url` was not updated. CURLINFO_EFFECTIVE_URL returns data->state.url to the user so it would return the URL from the initial transfer which was the wrong URL. It's likely there are other cases as well. Ref: https://curl.se/libcurl/c/CURLOPT_CURLU.html Reported-by: Nicolás San Martín Fixes https://github.com/curl/curl/issues/15984 Closes https://github.com/curl/curl/pull/15985 --- diff --git a/lib/setopt.c b/lib/setopt.c index dcf54f29ce..5b9f8be6a0 100644 --- a/lib/setopt.c +++ b/lib/setopt.c @@ -2103,7 +2103,6 @@ static CURLcode setopt_cptr(struct Curl_easy *data, CURLoption option, * The URL to fetch. */ if(data->state.url_alloc) { - /* the already set URL is allocated, free it first! */ Curl_safefree(data->state.url); data->state.url_alloc = FALSE; } @@ -2191,6 +2190,13 @@ static CURLcode setopt_cptr(struct Curl_easy *data, CURLoption option, /* * pass CURLU to set URL */ + if(data->state.url_alloc) { + Curl_safefree(data->state.url); + data->state.url_alloc = FALSE; + } + else + data->state.url = NULL; + Curl_safefree(data->set.str[STRING_SET_URL]); data->set.uh = (CURLU *)ptr; break; case CURLOPT_SSLCERT: diff --git a/lib/transfer.c b/lib/transfer.c index 452e5f413b..0407613f08 100644 --- a/lib/transfer.c +++ b/lib/transfer.c @@ -528,20 +528,15 @@ CURLcode Curl_pretransfer(struct Curl_easy *data) { CURLcode result = CURLE_OK; - if(!data->state.url && !data->set.uh) { + if(!data->set.str[STRING_SET_URL] && !data->set.uh) { /* we cannot do anything without URL */ failf(data, "No URL set"); return CURLE_URL_MALFORMAT; } - /* since the URL may have been redirected in a previous use of this handle */ - if(data->state.url_alloc) { - /* the already set URL is allocated, free it first! */ - Curl_safefree(data->state.url); - data->state.url_alloc = FALSE; - } - - if(!data->state.url && data->set.uh) { + /* CURLOPT_CURLU overrides CURLOPT_URL and the contents of the CURLU handle + is allowed to be changed by the user between transfers */ + if(data->set.uh) { CURLUcode uc; free(data->set.str[STRING_SET_URL]); uc = curl_url_get(data->set.uh, @@ -552,6 +547,14 @@ CURLcode Curl_pretransfer(struct Curl_easy *data) } } + /* since the URL may have been redirected in a previous use of this handle */ + if(data->state.url_alloc) { + Curl_safefree(data->state.url); + data->state.url_alloc = FALSE; + } + + data->state.url = data->set.str[STRING_SET_URL]; + if(data->set.postfields && data->set.set_resume_from) { /* we cannot */ failf(data, "cannot mix POSTFIELDS with RESUME_FROM"); @@ -563,7 +566,6 @@ CURLcode Curl_pretransfer(struct Curl_easy *data) data->state.list_only = data->set.list_only; #endif data->state.httpreq = data->set.method; - data->state.url = data->set.str[STRING_SET_URL]; #ifdef USE_SSL if(!data->state.ssl_scache) diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index f915a3630c..a68d3d9114 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -237,7 +237,7 @@ test1916 test1917 test1918 test1919 \ test1933 test1934 test1935 test1936 test1937 test1938 test1939 test1940 \ test1941 test1942 test1943 test1944 test1945 test1946 test1947 test1948 \ test1955 test1956 test1957 test1958 test1959 test1960 test1964 \ -test1970 test1971 test1972 test1973 test1974 test1975 test1976 \ +test1970 test1971 test1972 test1973 test1974 test1975 test1976 test1977 \ \ test2000 test2001 test2002 test2003 test2004 test2005 \ \ diff --git a/tests/data/test1977 b/tests/data/test1977 new file mode 100644 index 0000000000..c105c46662 --- /dev/null +++ b/tests/data/test1977 @@ -0,0 +1,62 @@ + + + +CURLOPT_CURLU +CURLINFO_EFFECTIVE_URL + + + +# Server-side + + +HTTP/1.1 200 OK +Date: Tue, 09 Nov 2010 14:49:00 GMT +Server: test-server/fake +Content-Type: text/html +Funny-head: yesyes swsclose + + + + +# Client-side + + +http + + +CURLOPT_CURLU and CURLINFO_EFFECTIVE_URL + + +lib%TESTNUMBER + + +# The test does three transfers to check how CURLINFO_EFFECTIVE_URL is reported +# when CURLOPT_CURLU changes between transfers. (Bug #15984) + +http://%HOSTIP:%HTTPPORT/%TESTNUMBER + + + +# Verify data after the test has been "shot" + + +effective URL: http://%HOSTIP:%HTTPPORT/%TESTNUMBER +effective URL: http://%HOSTIP:%HTTPPORT/%TESTNUMBER?foo +effective URL: http://%HOSTIP:%HTTPPORT/%TESTNUMBER?foo&bar + + +GET /%TESTNUMBER HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + +GET /%TESTNUMBER?foo HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + +GET /%TESTNUMBER?foo&bar HTTP/1.1 +Host: %HOSTIP:%HTTPPORT +Accept: */* + + + + diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc index b605d4f219..ba0aa9ca95 100644 --- a/tests/libtest/Makefile.inc +++ b/tests/libtest/Makefile.inc @@ -70,7 +70,7 @@ LIBTESTPROGS = libauthretry libntlmconnect libprereq \ lib1933 lib1934 lib1935 lib1936 lib1937 lib1938 lib1939 lib1940 \ lib1945 lib1946 lib1947 lib1948 lib1955 lib1956 lib1957 lib1958 lib1959 \ lib1960 lib1964 \ - lib1970 lib1971 lib1972 lib1973 lib1974 lib1975 \ + lib1970 lib1971 lib1972 lib1973 lib1974 lib1975 lib1977 \ lib2301 lib2302 lib2304 lib2305 lib2306 lib2308 lib2309 \ lib2402 lib2404 lib2405 \ lib2502 \ @@ -663,6 +663,9 @@ lib1974_LDADD = $(TESTUTIL_LIBS) lib1975_SOURCES = lib1975.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) lib1975_LDADD = $(TESTUTIL_LIBS) +lib1977_SOURCES = lib1977.c $(SUPPORTFILES) $(TESTUTIL) $(WARNLESS) +lib1977_LDADD = $(TESTUTIL_LIBS) + lib2301_SOURCES = lib2301.c $(SUPPORTFILES) lib2301_LDADD = $(TESTUTIL_LIBS) diff --git a/tests/libtest/lib1977.c b/tests/libtest/lib1977.c new file mode 100644 index 0000000000..939ac0b595 --- /dev/null +++ b/tests/libtest/lib1977.c @@ -0,0 +1,96 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) Daniel Stenberg, , et al. + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at https://curl.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + * SPDX-License-Identifier: curl + * + ***************************************************************************/ +#include "test.h" + +#include "testutil.h" +#include "warnless.h" +#include "memdebug.h" + +CURLcode test(char *URL) +{ + CURLcode res = CURLE_OK; + CURLU *curlu = curl_url(); + CURLU *curlu_2 = curl_url(); + CURL *curl; + char *effective = NULL; + + global_init(CURL_GLOBAL_ALL); + easy_init(curl); + + /* first transfer: set just the URL in the first CURLU handle */ + curl_url_set(curlu, CURLUPART_URL, URL, CURLU_DEFAULT_SCHEME); + easy_setopt(curl, CURLOPT_CURLU, curlu); + + res = curl_easy_perform(curl); + if(res) + goto test_cleanup; + + effective = NULL; + res = curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective); + if(res) + goto test_cleanup; + printf("effective URL: %s\n", effective); + + + /* second transfer: set URL + query in the second CURLU handle */ + curl_url_set(curlu_2, CURLUPART_URL, URL, CURLU_DEFAULT_SCHEME); + curl_url_set(curlu_2, CURLUPART_QUERY, "foo", 0); + easy_setopt(curl, CURLOPT_CURLU, curlu_2); + + res = curl_easy_perform(curl); + if(res) + goto test_cleanup; + + effective = NULL; + res = curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective); + if(res) + goto test_cleanup; + printf("effective URL: %s\n", effective); + + + /* third transfer: append extra query in the second CURLU handle, but do not + set CURLOPT_CURLU again. this is to test that the contents of the handle + is allowed to change between transfers and is used without having to set + CURLOPT_CURLU again */ + curl_url_set(curlu_2, CURLUPART_QUERY, "bar", CURLU_APPENDQUERY); + + res = curl_easy_perform(curl); + if(res) + goto test_cleanup; + + effective = NULL; + res = curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective); + if(res) + goto test_cleanup; + printf("effective URL: %s\n", effective); + + +test_cleanup: + curl_easy_cleanup(curl); + curl_url_cleanup(curlu); + curl_url_cleanup(curlu_2); + curl_global_cleanup(); + + return res; +}