[PHP 8.4] Add CURL_HTTP_VERSION_3(ONLY) constants

[Ayesh]

If the underlying Curl build supports it, setting the HTTP version should be theoretically possible. The availability of the constants do not guarantee the feature support, so declaring them in the polyfill should be same and effective.

• PR that added constants: php-src#15350
• libcurl doc on CURLOPT_HTTP_VERSION](https://curl.se/libcurl/c/CURLOPT_HTTP_VERSION.html)
• PHP.Watch: HTTP/3 Request with PHP Curl Extension
• Codex for CURL_HTTP_VERSION_3 and CURL_HTTP_VERSION_3ONLY

Fixes GH-488.

[stof]

Defining constants that cannot have effect would make the polyfilled case worse as it breaks feature detection without making the feature works. So -1 from me.

[nicolas-grekas]

Defining constants that cannot have effect would make the polyfilled case worse

Actually, passing the raw values does work. Curl accepts them (provided a recent enough version is installed). Only PHP doesn't expose these values as constants.

[Ayesh]

Thank you for the reviews @derrabus @stof.

@derrabus you are right that declaring Curl constants gives a false pretense that the feature is available. However, all of our constant declarations are only dependent on libcurl and PHP version numbers; They do not check the availability of those features. Recent additions include DoH and HSTS support. The constants are declared as long as the Curl extension is compiled with the minimum require version or later, but it does not guarantee those features are available.

For HTTP/3, the version check php-src uses is whether libcurl is >= v7.66. While most setups will have the libcurl version, it will not have HTTP/3 support because as of now, default builds for libcurl do not come with HTTP/3 support (checked on Fedora, Ubuntu and Debian). Even without HTTP/3 support, PHP 8.4 will have these constants declared.

Like @nicolas-grekas said, any PHP version can use HTTP/3 if built with support for it.

curl_setopt($ch, CURLOPT_HTTP_VERSION, 30); // returns false if HTTP/3 is not supported.

What the polyfill does is merely declare this as a proper constant. I think having this in the polyfill will can be helpful to avoid magic numbers.

The technically correct way to check HTTP/3 availability would be:

$hasHttp3 = !empty(curl_version()['feature_list']['HTTP3']); // PHP 8.4+ only
$hasHttp3 = curl_version()['features'] & 33554432); // or use CURL_VERSION_HTTP3 available on PHP 8.2.

That said, we cannot polyfill all Curl constants. We can't polyfill Curl options, but we should be able to polyfill Curl option values where they make sense. For example, we added CURLOPT_PREREQFUNCTION for PHP 8.4 but we cannot technically polyfill it.

[derrabus]

However, all of our constant declarations are only dependent on libcurl and PHP version numbers; They do not check the availability of those features.

Understood. But this PR ignores the libcurl version and the availability of ext-curl completely.

• Do we expect the constants to be declared if PHP is compiled without ext-curl?
• Do we expect the constants to be declared if PHP is compiled against a libcurl older than 7.66?

Let's look at how this is implemented in PHP:

https://github.com/php/php-src/blob/3ed884fab73c5807fe2b3001ee79545c27ad5e70/ext/curl/curl_arginfo.h#L854-L856

If I read that piece of code correctly, the answer to both questions should be no. But this is not what this PR implements.

[Ayesh]

Thank you for the pointers @nicolas-grekas @derrabus. I think you are right, it doesn't make sense to expose the constants on Curl < 7.66. I updated the PR to check for it and a function_exists check.

CURL_HTTP_VERSION_3ONLY constant is Curl 7.88+, so I updated the declaration to check for it too.

In my previous attempt, the bitmask check was to make sure Curl supports HTTP/3 with a true assertion. As @nicolas-grekas pointed out, I changed it to just check the Curl version regardless of the HTTP/3 availability of the underlying system. The check now makes sure the constant is declared and Curl accepted or rejected it with a true or false return value.

[nicolas-grekas]

I'm a bit annoyed by this slow path, how about not doing the check and always declare the const next to the previous one?
Accuracy of the polyfilling on this aspect doesn't look more important than raw perf here to me.

[derrabus]

Is the curl_version() call really that expensive? 😞

[nicolas-grekas]

Things add up: there are many polyfills to load: if we're not careful with each of them, this might make things slow without us realizing. For now, most (all?) checks are resolved at compile-time so they're free.

[Ayesh]

It's not the slowest call, but curl_version() has a lot of information mostly by checking bitmasks for protocol and feature support. We only need the version here, so yeah I agree it makes sense to use something like defined('CURLOPT_UPLOAD_BUFFERSIZE') that also implicitly ensures libcurl >= 7.62

[nicolas-grekas]

Suggested change

	if (curl_version()['version'] >= 0x075800) { // libcurl >= 7.88.0
	if (defined('CURLOPT_SSH_HOST_PUBLIC_KEY_SHA256')) { // libcurl >= 7.80.0 (7.88 would be better but is slow to check)

[derrabus]

LGTM

[nicolas-grekas]

Thank you @Ayesh.

[stof]

Why PHP 8.2.0 as boundary ? the PR you linked was merged in 8.4-dev so we might need the polyfill on 8.2 or 8.3

[nicolas-grekas]

php/php-src#8720 this one is merged in 8.2

[stof]

but it does not add those constants. So we could be on PHP 8.2.12 or 8.3.5 with CURL_VERSION_HTTP3 not being defined and we would never enter this if even if we have the latest libcurl

[derrabus]

@stof: If the CURL_VERSION_HTTP3 constant does not exist although we're on PHP 8.2, we know for sure that we either don't have ext-curl or the linked libcurl is too old, so we don't need to call curl_version() anymore.

[nicolas-grekas]

https://github.com/php/php-src/pull/8720/files#diff-29336a112e08c5b5caa7950afb6601010867f40a4643a33263646504277f7b33R1195

this adds the constant if php is compiled with a recent enough version of curl