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 dcf54f29cee1acc85b05975101de70d8de35ec50..5b9f8be6a0db9ce6c99de7ea2ace33e69fa5bda3 100644 (file)
--- 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 452e5f413b7290cea96ae62dd9caff35dde0f612..0407613f08bb4fd90f2aad45e75f56fdca0aaeaa 100644 (file)
--- 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 f915a3630c8b35bf38b9be415f17fafacbd6f483..a68d3d9114b97ecf4a5011efc9490e0aa1bd1c0b 100644 (file)
--- 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 (file)
index 0000000..c105c46
--- /dev/null
+++ b/tests/data/test1977
@@ -0,0 +1,62 @@
+<testcase>
+<info>
+<keywords>
+CURLOPT_CURLU
+CURLINFO_EFFECTIVE_URL
+</keywords>
+</info>
+
+# Server-side
+<reply>
+<data nocheck="yes">
+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
+
+</data>
+</reply>
+
+# Client-side
+<client>
+<server>
+http
+</server>
+<name>
+CURLOPT_CURLU and CURLINFO_EFFECTIVE_URL
+</name>
+<tool>
+lib%TESTNUMBER
+</tool>
+
+# The test does three transfers to check how CURLINFO_EFFECTIVE_URL is reported
+# when CURLOPT_CURLU changes between transfers. (Bug #15984)
+<command>
+http://%HOSTIP:%HTTPPORT/%TESTNUMBER
+</command>
+</client>
+
+# Verify data after the test has been "shot"
+<verify>
+<stdout>
+effective URL: http://%HOSTIP:%HTTPPORT/%TESTNUMBER
+effective URL: http://%HOSTIP:%HTTPPORT/%TESTNUMBER?foo
+effective URL: http://%HOSTIP:%HTTPPORT/%TESTNUMBER?foo&bar
+</stdout>
+<protocol>
+GET /%TESTNUMBER HTTP/1.1\r
+Host: %HOSTIP:%HTTPPORT\r
+Accept: */*\r
+\r
+GET /%TESTNUMBER?foo HTTP/1.1\r
+Host: %HOSTIP:%HTTPPORT\r
+Accept: */*\r
+\r
+GET /%TESTNUMBER?foo&bar HTTP/1.1\r
+Host: %HOSTIP:%HTTPPORT\r
+Accept: */*\r
+\r
+</protocol>
+</verify>
+</testcase>

diff --git a/tests/libtest/Makefile.inc b/tests/libtest/Makefile.inc
index b605d4f2194610d33f14b9b9f3f31dbf85ad9f7e..ba0aa9ca95ee959d0a2d3186282b98ad8f4a9a71 100644 (file)
--- 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 (file)
index 0000000..939ac0b
--- /dev/null
+++ b/tests/libtest/lib1977.c
@@ -0,0 +1,96 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) 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
+ * 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;
+}