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

Redirect overwrites `CURLOPT_URL` option value when `CURLOPT_FOLLOWLOCATION` used. #1631

Closed
rpv-tomsk opened this Issue Jul 2, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@rpv-tomsk

rpv-tomsk commented Jul 2, 2017

Hi.

In Collectd project (http://collectd.org, https://github.com/collectd/collectd/) there are several plugins which uses libcurl.

Libcurl is used by the following scheme:

  1. Libcurl object initialized and configured once, by calling curl_easy_init() and curl_easy_setopt().
  2. Requests will be done later by calling curl_easy_perform() on the same libcurl object many times.

The code uses CURLOPT_FOLLOWLOCATION option set to 1 at initialization.
The CURLOPT_URL option set at the same time (at initialization).

After some request was finished by redirect, subsequent requests are done to new location, not to location set by CURLOPT_URL.

That is unexpected and undocumented behaviour.

It is expected that subsequent calls to curl_easy_perform() will be done with same options set, and to the location, set by CURLOPT_URL.

IMHO, something required to be changed - code or documentation.

What do you think?

curl/libcurl version

7.38.0 (yes, it is quite old, but I found no issue-related changes in current 7.54.1 version too)

operating system

Debian jessie

Related Collectd issue: collectd/collectd#2328

@rpv-tomsk

This comment has been minimized.

rpv-tomsk commented Jul 2, 2017

The curl-7.38.0/lib/url.c has a code which sets value for CURLOPT_URL option:

  case CURLOPT_URL:
    /*
     * The URL to fetch.
     */
    if(data->change.url_alloc) {
      /* the already set URL is allocated, free it first! */
      Curl_safefree(data->change.url);
      data->change.url_alloc = FALSE;
    }
    result = setstropt(&data->set.str[STRING_SET_URL],
                       va_arg(param, char *));
    data->change.url = data->set.str[STRING_SET_URL];
    break;

The data->set.str[STRING_SET_URL] is not used nothere in libcurl code.
All the work is done using data->change.url value. (Which represents CURLINFO_EFFECTIVE_URL option of getinfo_char(), see curl-7.38.0/lib/getinfo.c)

But in Curl_follow() of curl-7.38.0/lib/transfer.c there is an assignment to data->change.url, which overwrites the value set previously.
There is also some other assignments exists too.

At the https://curl.haxx.se/libcurl/c/curl_easy_perform.html page there is the following note:

Just note that you will have to use curl_easy_setopt between the invokes to set options for the following curl_easy_perform.

But it is unclear if it is general requirement or only related to 'more than one file transfer', which requires to change file URL.

@bagder bagder added the URL label Jul 2, 2017

@bagder

This comment has been minimized.

Member

bagder commented Jul 2, 2017

This is definitely a bug, still present in 7.54.1... Setting the URL in the handle should make it remain set like that for subsequent transfers done on the same handle.

bagder added a commit that referenced this issue Jul 2, 2017

url: make the original string get used on subsequent transfers
... since CURLOPT_URL should follow the same rules as other options:
they remain set until changed or cleared.

Added test 1551 to verify.

Fixes #1631
Reported-by: Pavel Rochnyak
@bagder

This comment has been minimized.

Member

bagder commented Jul 2, 2017

My proposed fix for this bug and test case to verify it, are in #1632

@bagder bagder closed this in b3786f6 Jul 3, 2017

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

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