Deprecate ext/curl `CURLOPT_DNS_USE_GLOBAL_CACHE`?

php.internals

Ayesh - PHP.Watch

2 years ago
Hello Internals, We recently bumped[^1] the minimum required libcurl version supported by the PHP Curl extension to 7.61.0. This aligned with the recent CentOS/RHEL 7, along with other major Linux distros that have already updated to a more recent version of libcurl. I'm writing to the Internals to get some feedback on deprecating a Curl option (a `CURLOPT` constant) that we support in PHP, but is removed on Curl 7.62 and later. Curl supported an option called `CURLOPT_DNS_USE_GLOBAL_CACHE`[^2] that enabled Curl to use a global cache for DNS queries. It was added in Curl 7.9.3 (2002 Jan), marked as obsolete soon after, deprecated in 7.11.1, and removed in Curl 7.62.0[^3] (2018 Oct). The reason is that this functionality was not thread-safe, and was not functional after a few releases. Users can still share DNS caches by reusing Curl handles or by copying the Curl handles. The PHP Curl extension does support this constant, and if PHP is built with thread-safety enabled, setting this option emits a warning with the text "CURLOPT_DNS_USE_GLOBAL_CACHE cannot be activated when thread safety is enabled". I would like to propose to either deprecate this constant, or no-op it. A few points I'd like to draw attention to are that: 1. We have had the no-op'd Curl options before. For example, the `CURLOPT_BINARYTRANSFER` constant deprecated in PHP 8.4[^4] had no effect since PHP 5.1.2. Before PHP 8.4, setting this option had no effect, and it did not trigger any deprecation notices[^5]. 2. GitHub code search[^6] shows that there are close to 1,200 usages of the constant. 3. In all Curl versions since 7.62 and even up until the latest 8.9.0, the `CURLOPT_DNS_USE_GLOBAL_CACHE` exists, but does not do anything[^7]. 4. Applications[^8] and the Curl extension[^9] have special handling to avoid setting this constant in ZTS. If we were to no-op, we could remove these extra handling and clean-up the code base. I'd like to know your opinions; should we deprecate it at the constant-level, or no-op it for now and deprecate in a later version? Thank you, Ayesh. [^1]: https://github.com/php/php-src/pull/13259 [^2]: https://curl.se/libcurl/c/CURLOPT_DNS_USE_GLOBAL_CACHE.html [^3]: https://curl.se/mail/lib-2018-09/0010.html [^4]: https://php.watch/versions/8.4/CURLOPT_BINARYTRANSFER-deprecated [^5]: https://github.com/php/php-src/pull/13114 [^6]: https://github.com/search?q=CURLOPT_DNS_USE_GLOBAL_CACHE+language%3APHP++NOT+path%3A%2F%5Eext%5C%2F%2F+NOT+repo%3AJetBrains%2Fphpstorm-stubs+NOT+repo%3Afacebook%2Fhhvm+NOT+repo%3Athecodingmachine%2Fsafe&type=code [^7]: https://github.com/curl/curl/blob/curl-8_9_0/lib/setopt.c#L254-L256 [^8]: https://github.com/symfony/symfony/blob/v7.1.3/src/Symfony/Component/HttpClient/CurlHttpClient.php#L168-L170 [^9]: https://github.com/php/php-src/blob/php-8.4.0alpha2/ext/curl/interface.c#L1106-L1108

Christoph Becker

2 years ago
On 28.07.2024 at 19:26, Ayesh Karunaratne wrote:
> We recently bumped[^1] the minimum required libcurl version supported > by the PHP Curl extension to 7.61.0. This aligned with the recent > CentOS/RHEL 7, along with other major Linux distros that have already > updated to a more recent version of libcurl. I'm writing to the > Internals to get some feedback on deprecating a Curl option (a > `CURLOPT` constant) that we support in PHP, but is removed on Curl > 7.62 and later. > > Curl supported an option called `CURLOPT_DNS_USE_GLOBAL_CACHE`[^2] > that enabled Curl to use a global cache for DNS queries. It was added > in Curl 7.9.3 (2002 Jan), marked as obsolete soon after, deprecated in > 7.11.1, and removed in Curl 7.62.0[^3] (2018 Oct). The reason is that > this functionality was not thread-safe, and was not functional after a > few releases. Users can still share DNS caches by reusing Curl handles > or by copying the Curl handles. > > The PHP Curl extension does support this constant, and if PHP is built > with thread-safety enabled, setting this option emits a warning with > the text "CURLOPT_DNS_USE_GLOBAL_CACHE cannot be activated when thread > safety is enabled". > > I would like to propose to either deprecate this constant, or no-op it. > > [^1]: https://github.com/php/php-src/pull/13259 > [^2]: https://curl.se/libcurl/c/CURLOPT_DNS_USE_GLOBAL_CACHE.html > [^3]: https://curl.se/mail/lib-2018-09/0010.html
I'm somewhat torn about this. Generally, as a user I prefer to be notified about a feature which will not work; a deprecation notice would do. On the other hand, this is "only" a caching functionality, so it's probably not much of an issue if it doesn't work. Actually deprecating the constant might require the RFC process, so maybe it's best to make it a no-op for now (and to deprecate for PHP 8.5/9.0 whatever is next). But regardless of deprecation or no-op'ing, we should document the behavior, and tell users about alternatives to accomplish the same goal. Cheers, Christoph

Ayesh - PHP.Watch

2 years ago
> Actually deprecating the constant might require the RFC process, so > maybe it's best to make it a no-op for now (and to deprecate for PHP > 8.5/9.0 whatever is next).
You are right I also think making this no-op is the best way for now. Notably, Symfony HttpClient uses the constant to **disable** this feature. So if we were to deprecate the constant, it will cause deprecation notices unnecessarily.
> > But regardless of deprecation or no-op'ing, we should document the > behavior, and tell users about alternatives to accomplish the same goal.
I will submit PRs to the php/doc-en about the outcome of this change. Thank you for weighing in on this. There is PR 5094[^1] from you that proposed to deprecate several `CURLE_` constants. If we were to no-op this constant, I'd like to propose a deprecation RFC with all those changes combined. Thank you, Ayesh. [^1]: https://github.com/php/php-src/pull/5094

Christoph Becker

2 years ago
On 29.07.2024 at 13:09, Ayesh Karunaratne wrote:
> Thank you for weighing in on this. There is PR 5094[^1] from you that > proposed to deprecate several `CURLE_` constants. If we were to no-op > this constant, I'd like to propose a deprecation RFC with all those > changes combined. > > [^1]: https://github.com/php/php-src/pull/5094
Thank you for the reminder; I had completely forgotten this PR. Having a deprecation RFC for these constants appears to be prudent, although I'm afraid that voters will need way too much time on the actual vote; the Wiki is terrible for mass votes, but that is another story. Cheers, Christoph

Hans Henrik Bergan

2 years ago
>make it a no-op for now (and to deprecate for PHP8.5/9.0 whatever is next)
Sounds perfect. Fwiw CURLOPT_BINARYTRANSFER was deprecated in 8.4.0alpha without an RFC, but it had been a no-op since 5.1.4 in 2004 On Mon, Jul 29, 2024, 12:23 Christoph M. Becker <cmbecker69@gmx.de> wrote:

Ayesh - PHP.Watch

2 years ago
> Sounds perfect. > > Fwiw CURLOPT_BINARYTRANSFER was deprecated in 8.4.0alpha without an RFC, but it had been a no-op since 5.1.4 in 2004
Thank you! I submitted #15127[^1] that no-ops this constant. [^1]: https://github.com/php/php-src/pull/15127