-
Notifications
You must be signed in to change notification settings - Fork 7.9k
ext/curl: Add CURLOPT_DEBUGFUNCTION
#15674
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.
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
Conversation
This sounds like a reasonable feature to me (and a well written explanation). Thanks! Please fix the merge conflicts, if you have some time. I suggest not to include UPGRADING (or NEWS) entries in PRs; these are often conflicting. |
89b8619
to
75c9351
Compare
Thank you for the review @cmb69. I'll remember your remark wrt UPGRADING/NEWS entries; they indeed tend to conflict, and this time it's actually on myself because that merge conflict was from another PR wrt another Curl option 🤦♂️. |
75c9351
to
70bb7f5
Compare
70bb7f5
to
7d50367
Compare
Seeing if we are past the cutoff for PHP 8.4 feature-freeze? I see some bug fixes made for RC1, but not new features. |
1feaf5b
to
112e4c9
Compare
Well, cut-off is in 5 days, if I'm not mistaken. Asking @php/release-managers-84 if it's okay to merge this for PHP 8.4. |
112e4c9
to
eb7f70b
Compare
Friendly pings to @ericmann @SakiTakamachi for Release Manager blessing to see if we can have this for PHP 8.4 :) |
Sorry for the late reply. Since it's a relatively minor change, I'm in favor of adding this to 8.4. |
eb7f70b
to
2c54ff1
Compare
This adds support for `CURLOPT_DEBUGFUNCTION`[^1] Curl option to set a custom callback that gets called with debug information during the lifetime of a Curl request. The callback gets called with the `CurlHandle` object, an integer containing the type of the debug message, and a string containing the debug message. The callback may get called multiple times with the same message type during a request. PHP already uses `CURLOPT_DEBUGFUNCTION` functionality to internally to expose a Curl option named `CURLINFO_HEADER_OUT`. However,`CURLINFO_HEADER_OUT` is not a "real" Curl option supported by libcurl. Back in 2006, `CURLINFO_HEADER_OUT` was added[^2] as a Curl option by using the debug-callback feature. Git history does not run that back to show why `CURLINFO_HEADER_OUT` was added as a Curl option, and why the other debug types (such as `CURLINFO_HEADER_IN` were not added as Curl options, but this seems to be a historical artifact when we added features without trying to be close to libcurl options. This approach has a few issues: 1. `CURLINFO_HEADER_OUT` is not an actual Curl option supported by upstream libcurl. 2. All of the Curl options have `CURLOPT_` prefix, and `CURLINFO_HEADER_OUT` is the only Curl "option" that uses the `CURLINFO` prefix. This exception is, however, noted[^3] in docs. 3. When `CURLINFO_HEADER_OUT` is set, the `CURLOPT_VERBOSE` is also implicitly set. This was reported[^4] to bugs.php.net, but the bug is marked as wontfix. This commit adds support for `CURLOPT_DEBUGFUNCTION`. It extends the existing `curl_debug` callback to store the header-in information if it encounters a debug message with `CURLINFO_HEADER_OUT`. In all cases, if a callable is set, it gets called. `CURLOPT_DEBUGFUNCTION` intends to replace `CURLINFO_HEADER_OUT` Curl option as a versatile alternative that can also be used to extract other debug information such as SSL data, text information messages, incoming headers, as well as headers sent out (which `CURLINFO_HEADER_OUT` makes available). The callables are allowed to throw exceptions, but the return values are ignored. `CURLOPT_DEBUGFUNCTION` requires `CURLOPT_VERBOSE` enabled, and setting `CURLOPT_DEBUGFUNCTION` does _not_ implicitly enable `CURLOPT_VERBOSE`. If the `CURLOPT_DEBUGFUNCTION` option is set, setting `CURLINFO_HEADER_OUT` throws a `ValueError` exception. Setting `CURLOPT_DEBUGFUNCTION` _after_ enabling `CURLINFO_HEADER_OUT` is allowed. Technically, it is possible for both functionality (calling user-provided callback _and_ storing header-out data) is possible, setting `CURLINFO_HEADER_OUT` is not allowed to encourage the use of `CURLOPT_DEBUGFUNCTION` function. This commit also adds the rest of the `CURLINFO_` constants used as the `type` integer value in `CURLOPT_DEBUGFUNCTION` callback. --- [^1]: [cur.se - CURLOPT_DEBUGFUNCTION](https://curl.se/libcurl/c/CURLOPT_DEBUGFUNCTION.html) [^2]: [`5f25d80`](php@5f25d80) [^3]: [curl_setopt doc mentioning `CURLINFO_` prefix is intentional](https://www.php.net/manual/en/function.curl-setopt.php#:~:text=prefix%20is%20intentional) [^4]: [bugs.php.net - `CURLOPT_VERBOSE` does not work with `CURLINFO_HEADER_OUT`](https://bugs.php.net/bug.php?id=65348)
2c54ff1
to
daa21de
Compare
Thank you! |
Commit: php/php-src#15674 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_DEBUGFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_DEBUGFUNCTION)
Commit: php/php-src#15674 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_DEBUGFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_DEBUGFUNCTION)
Commit: php/php-src#15674 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_DEBUGFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_DEBUGFUNCTION)
Commit: php/php-src#15674 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_DEBUGFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_DEBUGFUNCTION)
Commit: php/php-src#15674 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_DEBUGFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_DEBUGFUNCTION)
This adds support for
CURLOPT_DEBUGFUNCTION
1 Curl option to set a custom callback that gets called with debug information during the lifetime of a Curl request.The callback gets called with the
CurlHandle
object, an integer containing the type of the debug message, and a string containing the debug message. The callback may get called multiple times with the same message type during a request.PHP already uses
CURLOPT_DEBUGFUNCTION
functionality to internally to expose a Curl option namedCURLINFO_HEADER_OUT
.However,
CURLINFO_HEADER_OUT
is not a "real" Curl option supported by libcurl. Back in 2006,CURLINFO_HEADER_OUT
was added2 as a Curl option by using the debug-callback feature. Git history does not run that back to show whyCURLINFO_HEADER_OUT
was added as a Curl option, and why the other debug types (such asCURLINFO_HEADER_IN
were not added as Curl options, but this seems to be a historical artifact when we added features without trying to be close to libcurl options.This approach has a few issues:
CURLINFO_HEADER_OUT
is not an actual Curl option supported by upstream libcurl.All of the Curl options have
CURLOPT_
prefix, andCURLINFO_HEADER_OUT
is the only Curl "option" that uses theCURLINFO
prefix. This exception is, however, noted3 in docs.When
CURLINFO_HEADER_OUT
is set, theCURLOPT_VERBOSE
is also implicitly set. This was reported4 to bugs.php.net, but the bug is marked as wontfix.This commit adds support for
CURLOPT_DEBUGFUNCTION
. It extends the existingcurl_debug
callback to store the header-in information if it encounters a debug message withCURLINFO_HEADER_OUT
. In all cases, if a callable is set, it gets called.CURLOPT_DEBUGFUNCTION
intends to replaceCURLINFO_HEADER_OUT
Curl option as a versatile alternative that can also be used to extract other debug information such as SSL data, text information messages, incoming headers, as well as headers sent out (whichCURLINFO_HEADER_OUT
makes available).The callables are allowed to throw exceptions, but the return values are ignored.
CURLOPT_DEBUGFUNCTION
requiresCURLOPT_VERBOSE
enabled, and settingCURLOPT_DEBUGFUNCTION
does not implicitly enableCURLOPT_VERBOSE
.If the
CURLOPT_DEBUGFUNCTION
option is set, settingCURLINFO_HEADER_OUT
throws aValueError
exception. SettingCURLOPT_DEBUGFUNCTION
after enablingCURLINFO_HEADER_OUT
is allowed. Technically, it is possible for both functionality (calling user-provided callback and storing header-out data) is possible, settingCURLINFO_HEADER_OUT
is not allowed to encourage the use ofCURLOPT_DEBUGFUNCTION
function.This commit also adds the rest of the
CURLINFO_
constants used as thetype
integer value inCURLOPT_DEBUGFUNCTION
callback.Footnotes
cur.se - CURLOPT_DEBUGFUNCTION ↩
5f25d80
↩curl_setopt doc mentioning
CURLINFO_
prefix is intentional ↩bugs.php.net -
CURLOPT_VERBOSE
does not work withCURLINFO_HEADER_OUT
↩