Skip to content

ext/curl: show curl option name in error message - #22908

Merged
arnaud-lb merged 9 commits into
php:masterfrom
Sjord:curl-error-option-name
Aug 6, 2026
Merged

ext/curl: show curl option name in error message#22908
arnaud-lb merged 9 commits into
php:masterfrom
Sjord:curl-error-option-name

Conversation

@Sjord

@Sjord Sjord commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Retrieve the option name by using curl_easy_option_by_id or by walking zend_constants, depending on whether the curl version used has curl_easy_option_by_id.

Showing the option name is especially useful when using curl_setopt_array. A user may specify many options, and this change makes it clear which option is wrong exactly.

Related to #10097, #22705.

@Sjord

Sjord commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

@GrahamCampbell What do you think about this fallback in case curl_easy_option_by_id is not available?

@Sjord

Sjord commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

php_curl_option_get_name currently does not work correctly for values that have multiple names:

  • CURLOPT_FTP_RESPONSE_TIMEOUT = CURLOPT_SERVER_RESPONSE_TIMEOUT
  • CURLOPT_ENCODING = CURLOPT_ACCEPT_ENCODING
  • CURLOPT_MAIL_RCPT_ALLLOWFAILS = CURLOPT_MAIL_RCPT_ALLOWFAILS

I think this is currently OK and can probably be fixed in the future.

I am also wondering if it makes sense to loop through zend_constants as a fallback when curl_easy_option_by_id returns null, but I can't think of a situation where that would result in anything.

@Sjord
Sjord marked this pull request as ready for review July 29, 2026 09:15
@Sjord
Sjord requested a review from adoy as a code owner July 29, 2026 09:15
Comment thread ext/curl/interface.c
@GrahamCampbell

Copy link
Copy Markdown
Contributor

Should this get a short Curl entry in NEWS? It changes user-visible validation messages, and the recent error-message clean-ups in other extensions have been recorded there. Perhaps:

- Curl:
  . Improved cURL option validation errors to include the option name.
    (Sjoerd Langkemper)
Comment thread ext/curl/tests/bug48207.phpt
@Sjord

Sjord commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@GrahamCampbell Thanks, great feedback.

Comment thread NEWS Outdated
@Sjord Sjord changed the title Show curl option name in error message Jul 30, 2026
@Sjord
Sjord force-pushed the curl-error-option-name branch from 0c92a07 to 37c402b Compare July 30, 2026 12:08
@Sjord
Sjord requested a review from GrahamCampbell August 1, 2026 19:21
Sjord added 8 commits August 4, 2026 08:10
Use curl_easy_option_by_id to retrieve the name of the option, and also
show the name of the option in the case where strings contain a null
byte.

curl_easy_option_by_id simplifies the code, but was introduced in curl
7.73.0, so this also bumps the minimum version of curl. 7.73.0 was
released in 2020, so I think that's acceptable.

Showing the option name is especially useful when using
curl_setopt_array. A user may specify many options, and this change
makes it clear which option is wrong exactly.
- Add line to NEWS
- Test more values for writable file handles
- Fallback to zend_constants if curl_easy_option_by_id fails
@Sjord
Sjord force-pushed the curl-error-option-name branch from 37c402b to 040221c Compare August 4, 2026 08:11
@Sjord

Sjord commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

This is ready.

@Sjord

Sjord commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@arnaud-lb Could you take a look at this, and perhaps merge it?

@arnaud-lb arnaud-lb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me!

@arnaud-lb
arnaud-lb merged commit 6a4ab2e into php:master Aug 6, 2026
18 checks passed
@arnaud-lb

Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants