Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curl_easy_getinfo CURLINFO_EFFECTIVE_URL not retrieveing the last used url #15984

Closed
nsanmartin opened this issue Jan 13, 2025 · 6 comments
Closed
Assignees

Comments

@nsanmartin
Copy link

I did this

I am using curl_easy_getinfo to get CURLINFO_EFFECTIVE_URL but I am not getting the last url used. I pass the url using CURLU* and while the data downloaded is the expected one, the effective url is not. Instead I get the previous one used.

Example:

#include <stdio.h>
#include <curl/curl.h>
 
static size_t cb(char *data, size_t size, size_t nmemb, void *clientp)
{
  size_t realsize = size * nmemb;
  return realsize;
}
 
 
int main(void) {
    CURL *curl = curl_easy_init();
    CURLU* cu = curl_url();
    char* url1 =  "https://curl.se";
    char* url2 = "https://libera.chat";
 
    curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, cb);
    curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
 
    curl_url_set(cu, CURLUPART_URL, url1, CURLU_DEFAULT_SCHEME);
    curl_easy_setopt(curl, CURLOPT_CURLU, cu);
    curl_easy_perform(curl);
        
    curl_url_set(cu, CURLUPART_URL, url2, CURLU_DEFAULT_SCHEME);
    curl_easy_setopt(curl, CURLOPT_CURLU, cu);
    curl_easy_perform(curl);
 
    printf("expecting %s\n", url2);
    char *effective = NULL;
    curl_easy_getinfo(curl, CURLINFO_EFFECTIVE_URL, &effective);
    if(effective)
      printf("effective url\t: %s\n", effective);
 
    curl_easy_cleanup(curl);
}

I expected the following

I expected the last effective used to be printed, "https://libera.chat" in the example, but instead it was "https://curl.se".

curl/libcurl version

curl 8.9.1 (x86_64-pc-linux-gnu) libcurl/8.9.1 OpenSSL/3.3.1 zlib/1.3.1 brotli/1.1.0 zstd/1.5.6 libidn2/2.3.7 libpsl/0.21.2 libssh2/1.11.0 nghttp2/1.62.1 librtmp/2.3 OpenLDAP/2.6.8
Release-Date: 2024-07-31, security patched: 8.9.1-2ubuntu2.2
Protocols: dict file ftp ftps gopher gophers http https imap imaps ipfs ipns ldap ldaps mqtt pop3 pop3s rtmp rtsp scp sftp smb smbs smtp smtps telnet tftp ws wss
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM PSL SPNEGO SSL threadsafe TLS-SRP UnixSockets zstd

operating system

Linux 6.11.0-13-generic #14-Ubuntu SMP PREEMPT_DYNAMIC Sat Nov 30 23:51:51 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

@jay
Copy link
Member

jay commented Jan 13, 2025

Yeah it shouldn't do that. The doc says "CURLOPT_CURLU explicitly overrides CURLOPT_URL" but internally data->state.url is not reset when a new CURLU is passed in as it is for URL. Here's the relevant code:

curl/lib/setopt.c

Lines 2198 to 2203 in 75a2079

case CURLOPT_CURLU:
/*
* pass CURLU to set URL
*/
data->set.uh = (CURLU *)ptr;
break;

curl/lib/setopt.c

Lines 2109 to 2120 in 75a2079

case CURLOPT_URL:
/*
* 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;
}
result = Curl_setstropt(&data->set.str[STRING_SET_URL], ptr);
data->state.url = data->set.str[STRING_SET_URL];
break;

curl/lib/transfer.c

Lines 556 to 565 in 75a2079

if(!data->state.url && data->set.uh) {
CURLUcode uc;
free(data->set.str[STRING_SET_URL]);
uc = curl_url_get(data->set.uh,
CURLUPART_URL, &data->set.str[STRING_SET_URL], 0);
if(uc) {
failf(data, "No URL set");
return CURLE_URL_MALFORMAT;
}
}

It can be solved in setopt or transfer, or maybe both? Here's what it would look like for setopt:

diff --git a/lib/setopt.c b/lib/setopt.c
index 7366d4a..e9aed30 100644
--- a/lib/setopt.c
+++ b/lib/setopt.c
@@ -2191,6 +2191,13 @@ static CURLcode setopt_cptr(struct Curl_easy *data, CURLoption option,
     /*
      * pass CURLU to set URL
      */
+    if(data->state.url_alloc) {
+      /* the already set URL is allocated, free it first! */
+      Curl_safefree(data->state.url);
+      data->state.url_alloc = FALSE;
+    }
+    data->state.url = NULL;
+    data->set.str[STRING_SET_URL] = NULL;
     data->set.uh = (CURLU *)ptr;
     break;
   case CURLOPT_SSLCERT:

More change may be needed in transfer I'm not sure yet.

@jay jay added the URL label Jan 13, 2025
@jay jay self-assigned this Jan 13, 2025
@bagder
Copy link
Member

bagder commented Jan 13, 2025

@nsanmartin's recipe here is also a great foundation for creating a test case

@jay
Copy link
Member

jay commented Jan 13, 2025

I agree. I started writing something similar in a new test but it's failing so far. There are other changes I made to transfer.c that may have something to do with it. Also, there is already test658 (lib658) that is supposed to test the CURLOPT_CURLU override of CURLOPT_URL, but does not test the multiple transfer principle. And weirdly it is failing in address sanitizer so I'm looking into that. (Edit: found it had to Curl_safefree(data->set.str[STRING_SET_URL]))

@bagder
Copy link
Member

bagder commented Jan 13, 2025

test 658 tests if CURLOPT_CURLU overrides CURLOPT_CURL proper. I think this issue is about CURLOPT_CURLU set a second time (for a second transfer). Not exactly the same test.

jay added a commit to jay/curl that referenced this issue Jan 13, 2025
- 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 curl#15984
Closes #xxxx
@jay
Copy link
Member

jay commented Jan 13, 2025

Not exactly the same test.

Yes. I made a separate test and didn't touch 658. It failed in address sanitizer because I forgot to free data->set.str[STRING_SET_URL] though I noticed that later and updated my post.

Proposed fix in #15985

@jay jay closed this as completed in 5ffc73c Jan 14, 2025
@jay
Copy link
Member

jay commented Jan 14, 2025

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants