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

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:

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 */
+  /* CURLOPT_CURLU overrides CURLOPT_URL and the contents of the CURLU handle
-  if(data->state.url_alloc) {
+     is allowed to be changed by the user between transfers */
-    /* the already set URL is allocated, free it first! */
+  if(data->set.uh) {
-    Curl_safefree(data->state.url);
-    data->state.url_alloc = FALSE;
-  }
-
-  if(!data->state.url && 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)

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 \
 \

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
+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: */*
+
+</protocol>
+</verify>
+</testcase>

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)
 

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;
+}