Skip to content

ext/curl: No-op CURLOPT_DNS_USE_GLOBAL_CACHE constant #15127

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
Merged
merged 1 commit into from
Sep 4, 2024
Merged

ext/curl: No-op CURLOPT_DNS_USE_GLOBAL_CACHE constant #15127

merged 1 commit into from
Sep 4, 2024

Conversation

Ayesh
Copy link
Member

@Ayesh Ayesh commented Jul 27, 2024 •
edited
Loading

Libcurl CURLOPT_DNS_USE_GLOBAL_CACHE constant is no longer supported since libcurl1 7.62. This feature is also not thread-safe, and we have special handling to emit warnings if it's used in ZTS builds.

Impact: Close to 1,100 hits on GitHub code search2

Libcurl CURLOPT_DNS_USE_GLOBAL_CACHE constant is no longer supported since libcurl1 7.62. This no-ops the constant, but without causing any deprecation notices.

Footnotes

  1. CURLOPT_DNS_USE_GLOBAL_CACHE ↩ ↩2

  2. GitHub code search: CURLOPT_DNS_USE_GLOBAL_CACHE ↩

@Ayesh Ayesh requested review from kocsismate and adoy as code owners July 27, 2024 16:16
@devnexen
Copy link
Member

Makes sense, note that centos 8 (the future centos minimum supported), curl version is 7.61.1 though.

@Ayesh
Copy link
Member Author

Ayesh commented Jul 27, 2024

You are right I was concerned about CentOS/RHEL 8 usage too. Perhaps we can no-op it without a deprecation notice, and then deprecate it in a future version?

@devnexen
Copy link
Member

Maybe you can gather some opinions on the mailing list ?

@Ayesh Ayesh force-pushed the curl-deprecate-CURLOPT_DNS_USE_GLOBAL_CACHE branch from edc432b to dfd1cd0 Compare July 29, 2024 11:36
@Ayesh Ayesh changed the title ext/curl: Deprecate CURLOPT_DNS_USE_GLOBAL_CACHE constant ext/curl: No-op CURLOPT_DNS_USE_GLOBAL_CACHE constant Jul 29, 2024
cmb69
cmb69 reviewed Jul 29, 2024
View reviewed changes
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

ext/curl/curl.stub.php Outdated
Comment on lines 79 to 78
const CURLOPT_DNS_USE_GLOBAL_CACHE = UNKNOWN;
const CURLOPT_DNS_USE_GLOBAL_CACHE = -1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this to suppress a deprecation warning when building ext/curl against recent libcurl versions, or just a convention?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. just seen #5094 (comment), so this is probably a good idea.

@Ayesh Ayesh force-pushed the curl-deprecate-CURLOPT_DNS_USE_GLOBAL_CACHE branch from dfd1cd0 to 694dfe1 Compare July 30, 2024 06:51
Ayesh
Ayesh commented Jul 30, 2024
View reviewed changes
Copy link
Member Author

@Ayesh Ayesh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently, CURLOPT_SAFE_UPLOAD has the value -1, so I reverted the previous change that set the CURLOPT_DNS_USE_GLOBAL_CACHE value to -1.

@cmb69 cmb69 mentioned this pull request Jul 30, 2024
Closed
4 tasks
@cmb69
Copy link
Member

cmb69 commented Jul 30, 2024

Apparently, CURLOPT_SAFE_UPLOAD has the value -1, so I reverted the previous change that set the CURLOPT_DNS_USE_GLOBAL_CACHE value to -1.

Indeed, that appears to be best for now.

cmb69
cmb69 approved these changes Jul 30, 2024
View reviewed changes
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@Ayesh Ayesh force-pushed the curl-deprecate-CURLOPT_DNS_USE_GLOBAL_CACHE branch from 694dfe1 to 40eb288 Compare September 4, 2024 09:46
@Ayesh
Copy link
Member Author

Ayesh commented Sep 4, 2024

Friendly gentle ping to see we can go forward with this :)

I emailed internals last month. It didn't receive a lot of responses, but there were no objections about changing this to no-op the constant. We can perhaps deprecate this in a future PHP version, hopefully as part of a bigger change to brush up all deprecated/obsolete options.

@Ayesh
ext/curl: No-op CURLOPT_DNS_USE_GLOBAL_CACHE constant
422255d 
Libcurl `CURLOPT_DNS_USE_GLOBAL_CACHE` constant is no longer supported
since libcurl[^1] 7.62. This no-ops the constant, but without causing
any deprecation notices.

[^1]: [CURLOPT_DNS_USE_GLOBAL_CACHE](https://curl.se/libcurl/c/CURLOPT_DNS_USE_GLOBAL_CACHE.html)
@Ayesh Ayesh force-pushed the curl-deprecate-CURLOPT_DNS_USE_GLOBAL_CACHE branch from 40eb288 to 422255d Compare September 4, 2024 09:52
@cmb69 cmb69 merged commit 24d4ae9 into php:master Sep 4, 2024
10 checks passed
@cmb69
Copy link
Member

cmb69 commented Sep 4, 2024

Thank you!

Ayesh added a commit to Ayesh/doc-en that referenced this pull request Nov 14, 2024
@Ayesh
[PHP 8.4] Curl: CURLOPT_DNS_USE_GLOBAL_CACHE no-op
efdcf6e 
Commit: php/php-src#15127
PHP.Watch: [PHP 8.4: Curl: `CURLOPT_DNS_USE_GLOBAL_CACHE` no longer has any effect](https://php.watch/versions/8.4/CURLOPT_DNS_USE_GLOBAL_CACHE-no-op)
Ayesh added a commit to Ayesh/doc-en that referenced this pull request Nov 15, 2024
@Ayesh
[PHP 8.4] Curl: CURLOPT_DNS_USE_GLOBAL_CACHE no-op
1bcdf0d 
Commit: php/php-src#15127
PHP.Watch: [PHP 8.4: Curl: `CURLOPT_DNS_USE_GLOBAL_CACHE` no longer has any effect](https://php.watch/versions/8.4/CURLOPT_DNS_USE_GLOBAL_CACHE-no-op)
Ayesh added a commit to Ayesh/doc-en that referenced this pull request Nov 15, 2024
@Ayesh
[PHP 8.4] Curl: CURLOPT_DNS_USE_GLOBAL_CACHE no-op
6f252d6 
Commit: php/php-src#15127
PHP.Watch: [PHP 8.4: Curl: `CURLOPT_DNS_USE_GLOBAL_CACHE` no longer has any effect](https://php.watch/versions/8.4/CURLOPT_DNS_USE_GLOBAL_CACHE-no-op)
Ayesh added a commit to Ayesh/doc-en that referenced this pull request Nov 16, 2024
@Ayesh
[PHP 8.4] Curl: CURLOPT_DNS_USE_GLOBAL_CACHE no-op
a8e3078 
Commit: php/php-src#15127
PHP.Watch: [PHP 8.4: Curl: `CURLOPT_DNS_USE_GLOBAL_CACHE` no longer has any effect](https://php.watch/versions/8.4/CURLOPT_DNS_USE_GLOBAL_CACHE-no-op)
Girgias pushed a commit to php/doc-en that referenced this pull request Nov 16, 2024
@Ayesh @Girgias
[PHP 8.4] Curl: CURLOPT_DNS_USE_GLOBAL_CACHE no-op
0d3cf0d 
Commit: php/php-src#15127
PHP.Watch: [PHP 8.4: Curl: `CURLOPT_DNS_USE_GLOBAL_CACHE` no longer has any effect](https://php.watch/versions/8.4/CURLOPT_DNS_USE_GLOBAL_CACHE-no-op)
@Ayesh Ayesh deleted the curl-deprecate-CURLOPT_DNS_USE_GLOBAL_CACHE branch November 25, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmb69 cmb69 cmb69 approved these changes

@kocsismate kocsismate Awaiting requested review from kocsismate

@adoy adoy Awaiting requested review from adoy adoy is a code owner

Assignees
No one assigned
Labels
Extension: curl
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ayesh @devnexen @cmb69