curl_setopt(_array) should warn on errors

0xearl commented 1 year ago

Description

The following code:

When using curl_setopt_array to set CURLOPT_RETURNTRANSFER it returns the result and bool. I was able to fix this by adding curl_setopt($ch, CURLOPT_RETURNTRANSFER, true);

<?php
$ch = curl_init();
$options = [CURLOPT_URL => 'https://google.com', CURLOPT_TCP_FASTOPEN => true, CURLOPT_TRANSFERTEXT => false, CURLOPT_RETURNTRANSFER => true];

curl_setopt_array($ch, $options);
$res = curl_exec($ch);
curl_close($ch);

var_dump($res);

Resulted in this output:

<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.google.com/">here</A>.
</BODY></HTML>
bool(true)

But I expected this output instead:

<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.google.com/">here</A>.
</BODY></HTML>

PHP Version

8.1.6 & 8.1.12

Operating System

Windows 10

damianwadley commented 1 year ago

curl_setopt_array is equivalent to a foreach loop that calls curl_setopt. There is something else responsible for the problems you're seeing...

0xearl commented 1 year ago

I'm honestly confused myself, that's the whole code.

damianwadley commented 1 year ago

Not sure what all I can tell you, other than this problem doesn't make sense.

What does var_dump($options) show? Are you sure the script (the entire file) is exactly as you've posted, with the output exactly as you've posted, and using exactly PHP 8.1.6 and/or 8.1.12? And it does so when you run php -n file.php, which guarantees there's no custom configuration being loaded like extensions or an auto_prepend_file?

0xearl commented 1 year ago

Running php -n file.php returns Fatal error: Uncaught Error: Call to undefined function curl_init()

var_dump($options) shows

array(4) {
  [10002]=>
  string(18) "https://google.com"
  [244]=>
  bool(true)
  [53]=>
  bool(false)
  [19913]=>
  bool(true)
}

Yes I have tested both 8.1.6 and 8.1.12

damianwadley commented 1 year ago

Ah, right, yeah -n will totally disable ext/curl as well. Ahem.

So what I meant was php -n -d extension=curl file.php.

0xearl commented 1 year ago

running php -n -d extension=curl file.php returns bool(false)

KapitanOczywisty commented 1 year ago

@0xearl Can you run the following code with the configuration from the beginning? We should see what is the actual version of the curl, which features are compiled in, if setopt was even successful and eventual error messages.

<?php

error_reporting(-1);
ini_set('display_errors', '1');

var_dump(curl_version());

echo "\ncurl_setopt_array\n";

$ch = curl_init();
$options = [CURLOPT_URL => 'https://google.com', CURLOPT_TCP_FASTOPEN => true, CURLOPT_TRANSFERTEXT => false, CURLOPT_RETURNTRANSFER => true];

var_dump(curl_setopt_array($ch, $options));
var_dump(curl_error($ch));
$res = curl_exec($ch);
var_dump($res, curl_error($ch));
curl_close($ch);

echo "\ncurl_setopt\n";

$ch = curl_init();
foreach ($options as $key => $value) {
    var_dump(curl_setopt($ch, $key, $value));
}
$res = curl_exec($ch);
var_dump($res, curl_error($ch));
curl_close($ch);

0xearl commented 1 year ago

@KapitanOczywisty running your code returns

array(16) {
  ["version_number"]=>        
  int(478464)
  ["age"]=>
  int(9)
  ["features"]=>
  int(288165789)
  ["ssl_version_number"]=>    
  int(0)
  ["version"]=>
  string(6) "7.77.0"
  ["host"]=>
  string(15) "x86_64-pc-win32"
  ["ssl_version"]=>
  string(14) "OpenSSL/1.1.1n" 
  ["libz_version"]=>
  string(6) "1.2.12"
  ["protocols"]=>
  array(24) {
    [0]=>
    string(4) "dict"
    [1]=>
    string(4) "file"
    [2]=>
    string(3) "ftp"
    [3]=>
    string(4) "ftps"
    [4]=>
    string(6) "gopher"
    [5]=>
    string(7) "gophers"
    [6]=>
    string(4) "http"
    [7]=>
    string(5) "https"
    [8]=>
    string(4) "imap"
    [9]=>
    string(5) "imaps"
    [10]=>
    string(4) "ldap"
    [11]=>
    string(5) "ldaps"
    [12]=>
    string(4) "mqtt"
    [13]=>
    string(4) "pop3"
    [14]=>
    string(5) "pop3s"
    [15]=>
    string(4) "rtsp"
    [16]=>
    string(3) "scp"
    [17]=>
    string(4) "sftp"
    [18]=>
    string(3) "smb"
    [19]=>
    string(4) "smbs"
    [20]=>
    string(4) "smtp"
    [21]=>
    string(5) "smtps"
    [22]=>
    string(6) "telnet"
    [23]=>
    string(4) "tftp"
  }
  ["ares"]=>
  string(0) ""
  ["ares_num"]=>
  int(0)
  ["libidn"]=>
  string(0) ""
  ["iconv_ver_num"]=>
  int(0)
  ["libssh_version"]=>
  string(14) "libssh2/1.10.0"
  ["brotli_ver_num"]=>
  int(0)
  ["brotli_version"]=>
  string(0) ""
}

curl_setopt_array
bool(false)
string(0) ""
<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.google.com/">here</A>.
</BODY></HTML>
bool(true)
string(0) ""

curl_setopt
bool(true)
bool(false)
bool(true)
bool(true)
string(220) "<HTML><HEAD><meta http-equiv="content-type" content="text/html;charset=utf-8">
<TITLE>301 Moved</TITLE></HEAD><BODY>
<H1>301 Moved</H1>
The document has moved
<A HREF="https://www.google.com/">here</A>.
</BODY></HTML>
"
string(0) ""

reading https://www.php.net/manual/en/function.curl-setopt-array.php it seems like curl_setopt_array is failing to set CURLOPT_TCP_FASTOPEN => true, and is returning false hence the next curl options are not being set. I'm not sure as to why curl_setopt_array is failing to set CURLOPT_TCP_FASTOPEN

KapitanOczywisty commented 1 year ago

Seems like it's not supported on windows: https://curl.se/libcurl/c/CURLOPT_TCP_FASTOPEN.html

0xearl commented 1 year ago

I see, thanks @KapitanOczywisty

KapitanOczywisty commented 1 year ago

I think there should be a warning when curl_setopt_array is failing on the libcurl side. TBH it's too easy to miss such error.

0xearl commented 1 year ago

it didn't even error out @KapitanOczywisty even with var_dump($res, curl_error($ch));

cmb69 commented 1 year ago

it didn't even error out @KapitanOczywisty even with var_dump($res, curl_error($ch));

Indeed. That is because curl_error() is implemented via CURLOPT_ERRORBUFFER and as such does not apply to curl_easy_setopt() (i.e. when setting options). That needs to be clarified in the documentation.

You can, however, use curl_errno() (and curl_strerror()):

<?php

$ch = curl_init();
$options = [
    CURLOPT_URL => 'https://google.com',
    CURLOPT_TCP_FASTOPEN => true,
    CURLOPT_TRANSFERTEXT => false,
    CURLOPT_RETURNTRANSFER => true
];

curl_setopt_array($ch, $options);
var_dump($errno = curl_errno($ch), curl_strerror($errno));

outputs:

int(4)
string(108) "A requested feature, protocol or option was not found built-in in this libcurl due to a build-time decision."

I think there should be a warning when curl_setopt_array is failing on the libcurl side. TBH it's too easy to miss such error.

I agree that this might be a good idea, but given the potential BC break, I don't think that we should see this as bug fix, but rather as a feature.

php / php-src