php / php-src

The PHP Interpreter
https://www.php.net
Other
38.08k stars 7.74k forks source link

Support CURLOPT_ECH #16305

Open cracksalad opened 6 days ago

cracksalad commented 6 days ago

Description

Not sure, if this is really a bug. It is more like a question if this is a bug

The following code:

<?php
$curl = curl_init('https://php.net');
curl_exec($curl);

$error = curl_getinfo($curl, CURLINFO_OS_ERRNO);
echo $error.' "'.curl_strerror($error).'"'.PHP_EOL;

curl_close($curl);

Resulted in this output (with cURL >=8.10):

101 "ECH attempted but failed"

But I expected this output instead (with cURL <8.10):

0 "No error"

Seems like a cURL issue and not an issue with the wrapper, but cURL provides the CURLOPT_ECH to configure it, which the PHP wrapper does not. So I am a bit stuck. Do I have to treat errors differently depending on the cURL version I use or is there any possibility to configure the behavior here? The CURLOPT_ECH documentation (see above) says the default setting is off, which does not seem to be true here. Is this an issue with cURL or with the wrapper changing the default somehow?

PHP Version

PHP 8.3.12

Operating System

No response

cmb69 commented 6 days ago

Did you try with cURL 8.10.1? (8.10.0 was a bad release with known issues)

cracksalad commented 6 days ago

Did you try with cURL 8.10.1? (8.10.0 was a bad release with known issues)

Yes, I did. Actually tested with 8.8.0, 8.9.0 and 8.10.1 (did not even try 8.10.0). 8.10.1 is the only one of those three with the 101 error.

cmb69 commented 6 days ago

Hmm, the script doesn't error for me (Windows, cURL 8.10.1, but PHP master branch). That needs further investigation.

cracksalad commented 6 days ago

Hmm, the script doesn't error for me (Windows, cURL 8.10.1, but PHP master branch). That needs further investigation.

That's interesting. I tested on Windows 10 and in a GitLab CI with Alpine 3.20.2 (with cURL 8.9) and Alpine 3.20.3 (with cURL 8.10.1). I believed that would be independent enough to say that it is a platform independent problem. By the way, I noticed that the problem depends on the website you are targeting, e.g. https://github.com works (they might have ECH support already) while most others don't (at least in my experience).

cracksalad commented 2 days ago
Did a little investigation based on PHP's official docker images and the exact same script as in the original post^^: Image Affected by this issue? Comment
8.3.12-alpine affected after apk update && apk upgrade
8.3.12-bookworm not affected even with apt update && apt upgrade installed cURL version is 7.88.1
8.2.24-alpine affected after apk update && apk upgrade
8.4.0RC2-alpine affected comes with cURL v8.10.1 already, no update required
cmb69 commented 1 day ago

The point is not really the cURL version (unless it's older than 8.8.0), but whether ECH is enabled by the build; apparently, CMake builds have an option USE_ECH which is disabled by default, but our Windows builds still use Makefile.vc where apparently no such option exist.

Since ext/curl does not support CURLOPT_ECH at all, you cannot even try to change its value, nor can you find out whether it is supported (e.g. via phpinfo()), except by making an actual request as in the OP above.

It seems to me that we need to add support for CURLOPT_ECH, but given that it is a relative new cURL feature, I would treat that as feature request (not as a bug). Our official Windows builds should catch up, but as I understand it, there is no support in OpenSSL yet, so that probably takes some time.