Skip to content

ext/curl: Add CURLOPT_DEBUGFUNCTION #16

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

Closed
wants to merge 9 commits into from
Closed

ext/curl: Add CURLOPT_DEBUGFUNCTION #16

wants to merge 9 commits into from

Conversation

Ayesh
Copy link
Owner

@Ayesh Ayesh commented Aug 30, 2024

This adds support for CURLOPT_DEBUGFUNCTION1 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 2016, CURLINFO_HEADER_OUT was added 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, noted2 in docs.

  3. When CURLINFO_HEADER_OUT is set, the CURLOPT_VERBOSE is also implicitly set. This was reported3 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.


Footnotes

  1. cur.se - CURLOPT_DEBUGFUNCTION

  2. curl_setopt doc mentioning CURLINFO_ prefix is intentional

  3. bugs.php.net - CURLOPT_VERBOSE does not work with CURLINFO_HEADER_OUT

dstogov and others added 8 commits September 23, 2024 17:09
* PHP-8.2:
  Fix phpGH-15973: Segmentation fault in JIT mode 1135 (php#16006)
* PHP-8.3:
  Fix phpGH-15973: Segmentation fault in JIT mode 1135 (php#16006)
* PHP-8.2:
  ext/curl: mark certain tests as xfail on curl 8.10.0
* PHP-8.3:
  ext/curl: mark certain tests as xfail on curl 8.10.0
Such that nobody actually attempts to implement this.
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)
Ayesh pushed a commit that referenced this pull request Oct 16, 2024
even without sanitizers, it is reproducible but with the following

```
<?php
$g = gmp_init(256);
var_dump(gmp_pow($g, PHP_INT_MAX));
```

we get this

```
AddressSanitizer:DEADLYSIGNAL
=================================================================
==286922==ERROR: AddressSanitizer: FPE on unknown address 0x03e8000460ca (pc 0x7faf6c69de5c bp 0x400000000000004 sp 0x7ffe9843c740 T0)
    #0 0x7faf6c69de5c in __pthread_kill_implementation nptl/pthread_kill.c:44
    #1 0x7faf6c649c81 in __GI_raise ../sysdeps/posix/raise.c:26
    #2 0x7faf6db9386c in __gmp_exception (/lib/x86_64-linux-gnu/libgmp.so.10+0xd86c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #3 0x7faf6db938d3 in __gmp_overflow_in_mpz (/lib/x86_64-linux-gnu/libgmp.so.10+0xd8d3) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #4 0x7faf6dbac95c in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x2695c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #5 0x7faf6dba9038 in __gmpz_n_pow_ui (/lib/x86_64-linux-gnu/libgmp.so.10+0x23038) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #6 0x5565ae1ccd9f in zif_gmp_pow /home/dcarlier/Contribs/php-src/ext/gmp/gmp.c:1286
    #7 0x5565aee96ea9 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1312
    #8 0x5565af144320 in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:56075
    #9 0x5565af160f07 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:60439
    #10 0x5565aed6fafe in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1842
    #11 0x5565aeae70a8 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2578
    #12 0x5565af532f4e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:964
    #13 0x5565af535877 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1334
    #14 0x7faf6c633d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #15 0x7faf6c633e24 in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0x5565adc04040 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2604040) (BuildId: 949049955bdf8b7197390b1978a1dfc3ef6fdf38)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: FPE nptl/pthread_kill.c:44 in __pthread_kill_implementation
==286922==ABORTING
```
Ayesh pushed a commit that referenced this pull request Oct 29, 2024
even without sanitizers, it is reproducible but with the following

```
<?php
$g = gmp_init(256);
var_dump(gmp_pow($g, PHP_INT_MAX));
```

we get this

```
AddressSanitizer:DEADLYSIGNAL
=================================================================
==286922==ERROR: AddressSanitizer: FPE on unknown address 0x03e8000460ca (pc 0x7faf6c69de5c bp 0x400000000000004 sp 0x7ffe9843c740 T0)
    #0 0x7faf6c69de5c in __pthread_kill_implementation nptl/pthread_kill.c:44
    #1 0x7faf6c649c81 in __GI_raise ../sysdeps/posix/raise.c:26
    #2 0x7faf6db9386c in __gmp_exception (/lib/x86_64-linux-gnu/libgmp.so.10+0xd86c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #3 0x7faf6db938d3 in __gmp_overflow_in_mpz (/lib/x86_64-linux-gnu/libgmp.so.10+0xd8d3) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #4 0x7faf6dbac95c in __gmpz_realloc (/lib/x86_64-linux-gnu/libgmp.so.10+0x2695c) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #5 0x7faf6dba9038 in __gmpz_n_pow_ui (/lib/x86_64-linux-gnu/libgmp.so.10+0x23038) (BuildId: 1af68a49fe041a5bb48a2915c3d47541f713bb38)
    #6 0x5565ae1ccd9f in zif_gmp_pow /home/dcarlier/Contribs/php-src/ext/gmp/gmp.c:1286
    #7 0x5565aee96ea9 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:1312
    #8 0x5565af144320 in execute_ex /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:56075
    #9 0x5565af160f07 in zend_execute /home/dcarlier/Contribs/php-src/Zend/zend_vm_execute.h:60439
    #10 0x5565aed6fafe in zend_execute_scripts /home/dcarlier/Contribs/php-src/Zend/zend.c:1842
    #11 0x5565aeae70a8 in php_execute_script /home/dcarlier/Contribs/php-src/main/main.c:2578
    #12 0x5565af532f4e in do_cli /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:964
    #13 0x5565af535877 in main /home/dcarlier/Contribs/php-src/sapi/cli/php_cli.c:1334
    #14 0x7faf6c633d67 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #15 0x7faf6c633e24 in __libc_start_main_impl ../csu/libc-start.c:360
    #16 0x5565adc04040 in _start (/home/dcarlier/Contribs/php-src/sapi/cli/php+0x2604040) (BuildId: 949049955bdf8b7197390b1978a1dfc3ef6fdf38)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: FPE nptl/pthread_kill.c:44 in __pthread_kill_implementation
==286922==ABORTING
```

close phpGH-16384
@Ayesh Ayesh deleted the curl-debugfunc branch November 25, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants