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.

Sign up for GitHub

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

Jump to bottom

Bug when reusing CURL handle which was used for FTP download before #12367

Closed
ohyeaah opened this issue Nov 20, 2023 · 4 comments
Closed

Bug when reusing CURL handle which was used for FTP download before #12367

ohyeaah opened this issue Nov 20, 2023 · 4 comments
Labels
needs-info

Comments

@ohyeaah
Copy link

ohyeaah commented Nov 20, 2023 •
edited

I did this

In short
If you reuse a CURL handle in PHP the HTTP code (CURLINFO_HTTP_CODE) may come from a previous FTP request.

Long description
I use CURL to download an FTP file in PHP. Later I reuse the handle to download an HTTPS file from a completely different host.

The HTTPS connection successfully downloads the file but the HTTP return code reported by CURL is 221 which comes from the FTP connection of the previous request.

I traced the FTP connection in Wireshark and it looks like this:

226 Transfer completed.
QUIT
221 Goodbye.

So here again what I do and what probably happens behind the scenes:

  1. I use CURL to download a file from an FTP server. CURLINFO_HTTP_CODE is 226 (everything good).
  2. CURL will leave the FTP connection open because it may be needed again later (everything good).
  3. I reuse the CURL handle to download a file from an HTTPS server of a completely different host (everything good).
  4. CURL receives the HTTP header and after that it downloads let's say 50% of the file (everything good).
  5. While the HTTP download is running, CURL decides to close the FTP connection of the previous request (everything good).
  6. The FTP server replies "221 Goodbye." This overwrites the valid HTTP code with an invalid value of a previous request (221) (not good).
  7. The HTTP download finishes but the CURLINFO_HTTP_CODE (221) is invalid and of a previous FTP request (not good).

I am using PHP 8.2.10-2ubuntu1 and curl_version() reports 8.2.1.

I reported this here because I thought it's probably more a CURL bug than a PHP bug.

I expected the following

At the end I expected CURLINFO_HTTP_CODE to report the HTTP code of the last request.

curl/libcurl version

PHP curl plugin 8.2.1

operating system

KUbuntu 23.10
@ohyeaah ohyeaah changed the title Bug when re-using CURL handle which was used for FTP download Bug when reusing CURL handle which was used for FTP download Nov 20, 2023
@ohyeaah ohyeaah changed the title Bug when reusing CURL handle which was used for FTP download Bug when reusing CURL handle which was used for FTP download before Nov 20, 2023
@bagder
Copy link
Member

bagder commented Nov 20, 2023

While the HTTP download is running, CURL decides to close the FTP connection of the previous request

Is this what you guess is happening or have you actually seen any evidence of this? (Because I cannot understand how that would happen) Are you using the easy interface?

@ohyeaah
Copy link
Author

ohyeaah commented Nov 20, 2023

@bagder I am not using the easy interface. I think it doesn't exist in PHP.

Is this what you guess is happening or have you actually seen any evidence of this?

It's what I suppose is happening. Please give me some time I will provide an example code within this week.

@ohyeaah
Copy link
Author

ohyeaah commented Nov 24, 2023 •
edited

So here is the code. I am sorry for the strange URLs I'm using but as soon as I use different URLs everything works fine.

The code performs a download of 5 URLs while reusing the CURL handle.

The URLs are:

https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=355
https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=359
https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=364
https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=365
https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=366

They all redirect (HTTP code 302) to a different URL.

The first URL redirects to ftp://ftp.matrox.com/pub/mga/archive/bios/2001/setup208.exe
The redirect destinations of URL 2 to 4 don't matter (none is FTP).
The last URL redirects to https://ts.hercules.com/download/video/exe/KYROBASED/3DPK-2K-7111a.exe

It should be said that CURLINFO_HTTP_CODE always returns the HTTP code of the redirected destination URL and not 302.

You can download the last URL on the command line using curl --verbose and you will see it's a normal 200 code.

However in the PHP script below I get a 221 for the CURLINFO_HTTP_CODE of the last URL. This is impossible because 221 is an FTP code and doesn't exist in HTTP. The line 221 Goodbye. also appears when using CURLOPT_HEADERFUNCTION (also 5. URL). That must be wrong as this is an FTP response. Both the return code and the header must come from the first request.

At the moment I am simply creating a new CURL handle to not experience the bug (which I think it is).
If it's a usage error by me then I am sorry for reporting this as a bug.

Please note it's not neccessary to close the PHP tag. Some people confuse this when they see it the first time.

Here it is:

<?php

function cexec($ch, $dumpHeaders)
{
  echo "get URL ".curl_getinfo($ch, CURLINFO_EFFECTIVE_URL)."\n";
  if ($dumpHeaders)
  {
    curl_setopt($ch, CURLOPT_HEADERFUNCTION,
    function($ch, $line)
    {
        echo "header: $line";
        return strlen($line);
    }
    );
  }
  $ret = curl_exec($ch);
  if ($ret === false)
    echo "curl_exec() returned false\n";
  echo "response code: ".curl_getinfo($ch, CURLINFO_HTTP_CODE)."\n";
}

$ch = curl_init();
curl_setopt($ch, CURLOPT_FOLLOWLOCATION, true);
curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, false);
curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, false);

for ($i = 355; $i <= 366; $i++)
{
    if ($i == 356 || $i == 357 || $i == 358 || $i == 360 || $i == 361 || $i == 362 || $i == 363)
        continue;
    curl_setopt($ch, CURLOPT_URL, "https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=$i");
    cexec($ch, $i == 366);
}

curl_close($ch);

Output on the console:

get URL https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=355
response code: 226
get URL https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=359
response code: 403
get URL https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=364
response code: 404
get URL https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=365
response code: 200
get URL https://www.touslesdrivers.com/php/constructeurs/redirection.php?v_code=366
header: HTTP/2 302 
header: content-type: text/html; charset=UTF-8
header: location: https://ts.hercules.com/download/video/exe/KYROBASED/3DPK-2K-7111a.exe
header: server: HTTP
header: x-frame-options: SAMEORIGIN
header: strict-transport-security: max-age=63072000; includeSubDomains; preload
header: date: Fri, 24 Nov 2023 17:55:58 GMT
header: content-length: 0
header: 
header: HTTP/2 200 
header: date: Fri, 24 Nov 2023 17:55:58 GMT
header: content-type: application/x-msdownload
header: content-length: 6844336
header: last-modified: Tue, 06 Nov 2012 10:28:12 GMT
header: etag: "686fb0-4cdd10f1b7a58"
header: cf-cache-status: HIT
header: age: 14979
header: expires: Fri, 24 Nov 2023 18:55:58 GMT
header: cache-control: public, max-age=3600
header: accept-ranges: bytes
header: server: cloudflare
header: cf-ray: 82b386a0eac0bb55-FRA
header: 
header: 221 Goodbye.
response code: 221

@bagder
Copy link
Member

bagder commented Nov 25, 2023

Thanks for the code. I can now reproduce the problem!

bagder added a commit that referenced this issue Nov 25, 2023
@bagder
conncache: use the closure handle when disconnecting surplus connections
826c862 
Use the closure handle for disconnecting connection cache entries so
that anything that happens during the disconnect is not stored and
associated with the 'data' handle which already just finished a transfer
and it is important that details from the unrelated disconnect does not
taint meta-data in the data handle.

Like storing the response code.

Reported-by: ohyeaah on github
Fixes #12367
@bagder bagder mentioned this issue Nov 25, 2023
Closed
@bagder bagder closed this as completed in dbf4c40 Nov 26, 2023
@heinrich5991 heinrich5991 mentioned this issue Dec 13, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
needs-info
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

conncache: use the closure handle when disconnecting surplus connections curl/curl
2 participants
@bagder @ohyeaah