ext/curl: Add `CURLOPT_PREREQFUNCTION` by Ayesh · Pull Request #13255 · php/php-src

Ayesh · 2024-01-26T16:40:45Z

Curl >= 7.80.0 supports declaring a function for CURLOPT_PREREQFUNCTION option that gets called after Curl establishes a connection (including the TLS handshake for HTTPS connections), but before the actual request is made.

The callable must return either CURL_PREREQFUNC_OK or CURL_PREREQFUNC_ABORT to allow or abort the request.

This adds support for it to PHP with required ifdef.

libc: https://curl.se/libcurl/c/CURLOPT_PREREQFUNCTION.html

UPGRADING

devnexen · 2024-01-26T17:46:41Z

Looking good overall, @mvorisek remark is valid.

ext/curl/tests/curl_setopt_CURLOPT_PREREQFUNCTION.phpt

SakiTakamachi · 2024-01-27T08:57:17Z

In my opinion, #if LIBCURL_VERSION_NUM >= 0x075000 is a hexadecimal literal, so it might be better to write the version in the comments as elsewhere.

Ayesh · 2024-01-27T12:38:48Z

Thank you @SakiTakamachi - I added comments to indicate it's for Curl 7.80.0 throughout everywhere I had ifdef calls.

ext/curl/curl_private.h

ext/curl/interface.c

Girgias · 2024-02-05T13:40:01Z

Can you wait for #13291 and rebase on top of this when it is merged? As the refactoring should simplify the code.

Ayesh · 2024-02-05T13:50:16Z

Awesome, thank you! I can also learn how to do it from your PR too, so this is perfect.

Ayesh · 2024-05-06T23:47:36Z

@Girgias now that #13291 is merged, I rebased and changed this to use your new FCC pattern. Could you review when you get some time please? Thank you.

Girgias

Minor nits, can you also add a trampoline callback test case?

ext/curl/interface.c

ext/curl/tests/curl_setopt_CURLOPT_PREREQFUNCTION.phpt

Girgias · 2024-08-21T16:44:04Z

@Ayesh ping

Girgias

LGTM if CI is green

Ayesh · 2024-08-21T20:30:11Z

Thank you @Girgias - I made a bunch of force-pushes with a few more added tests, but I tested them locally. fingers crossed CI marks them green too.

Ayesh · 2024-08-21T20:51:38Z

curl_setopt($ch, CURLOPT_PREREQFUNCTION, null) seems to segfault, I will sort out and fix soon :(

Girgias · 2024-08-24T12:26:51Z

ext/curl/tests/curl_setopt_CURLOPT_PREREQFUNCTION.phpt

+	var_dump(filter_var($args[1], FILTER_VALIDATE_IP) !== false);
+	var_dump(filter_var($args[2], FILTER_VALIDATE_IP) !== false);


Would it be possible to have the test not rely on the filter extension? Found this out while trying it locally without ext/filter compiled.

Otherwise it needs to be added to the EXTENSIONS section as I did.

Curl >= 7.80.0 supports declaring a function for `CURLOPT_PREREQFUNCTION` option that gets called after Curl establishes a connection (including the TLS handshake for HTTPS connections), but before the actual request is made. The callable must return either `CURL_PREREQFUNC_OK` or `CURL_PREREQFUNC_ABORT` to allow or abort the request. This adds support for it to PHP with required ifdef. - libc: https://curl.se/libcurl/c/CURLOPT_PREREQFUNCTION.html Co-Authored-By: Gina Peter Bnayard <[email protected]>

Ayesh · 2024-08-25T14:35:17Z

CI green finally, and curl_setopt($ch, CURLOPT_PREREQFUNCTION, null); curl_exec($ch); works now :)

`CURLOPT_PREREQFUNCTION` is a new Curl option added in PHP 8.4, along with two constants `CURL_PREREQFUNC_ABORT` and `CURL_PREREQFUNC_OK`. - [php/php-src#13255](php/php-src#13255) - [libcurl doc - `CURLOPT_PREREQFUNCTION`](https://curl.se/libcurl/c/CURLOPT_PREREQFUNCTION.html) - [PHP.Watch Codex - `CURLOPT_PREREQFUNCTION`](https://php.watch/codex/CURLOPT_PREREQFUNCTION) - [PHP.Watch Codex - `CURL_PREREQFUNC_ABORT`](https://php.watch/codex/CURL_PREREQFUNC_ABORT) - [PHP.Watch Codex - `CURL_PREREQFUNC_OK`](https://php.watch/codex/CURL_PREREQFUNC_OK)

[ci skip]

…tions This is a follow-up to phpGH-13255. When the `CURLOPT_PREREQFUNCTION` callback does not return any value or return an invalid value, we throw a `ValueError` or a `TypeError` exception. However, it does not prevent the Curl request from going forward because our default return value is `CURL_PREREQFUNC_OK`. This changes the default value to `CURL_PREREQFUNC_ABORT`.

Commit: php/php-src#13255 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_PREREQFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_PREREQFUNCTION)

Ayesh requested review from adoy and kocsismate as code owners January 26, 2024 16:40

github-actions bot added the Extension: curl label Jan 26, 2024

mvorisek reviewed Jan 26, 2024

View reviewed changes

UPGRADING Outdated Show resolved Hide resolved

Ayesh force-pushed the curl-prereq-func branch 3 times, most recently from f0efdd4 to 1e856f5 Compare January 26, 2024 17:56

devnexen reviewed Jan 26, 2024

View reviewed changes

ext/curl/tests/curl_setopt_CURLOPT_PREREQFUNCTION.phpt Outdated Show resolved Hide resolved

Ayesh force-pushed the curl-prereq-func branch from 1e856f5 to 5c06c5b Compare January 27, 2024 04:50

Ayesh force-pushed the curl-prereq-func branch 3 times, most recently from 81c6b26 to c7bb6f0 Compare January 27, 2024 12:10

Girgias reviewed Jan 28, 2024

View reviewed changes

ext/curl/curl_private.h Outdated Show resolved Hide resolved

ext/curl/interface.c Outdated Show resolved Hide resolved

ext/curl/interface.c Outdated Show resolved Hide resolved

Ayesh force-pushed the curl-prereq-func branch 2 times, most recently from eafa628 to c388954 Compare February 5, 2024 09:51

Ayesh marked this pull request as draft February 5, 2024 13:50

Ayesh force-pushed the curl-prereq-func branch 2 times, most recently from 4f1e413 to 545e929 Compare March 11, 2024 15:26

Ayesh force-pushed the curl-prereq-func branch 2 times, most recently from 60eda9b to e53986f Compare March 19, 2024 17:58

Ayesh force-pushed the curl-prereq-func branch from e53986f to 11246af Compare May 6, 2024 23:44

Ayesh marked this pull request as ready for review May 6, 2024 23:51

Ayesh force-pushed the curl-prereq-func branch from 11246af to 3350b4a Compare May 6, 2024 23:59

Girgias reviewed May 7, 2024

View reviewed changes

ext/curl/interface.c Outdated Show resolved Hide resolved

ext/curl/interface.c Outdated Show resolved Hide resolved

ext/curl/tests/curl_setopt_CURLOPT_PREREQFUNCTION.phpt Outdated Show resolved Hide resolved

Ayesh force-pushed the curl-prereq-func branch 3 times, most recently from 962835e to 68e357c Compare August 21, 2024 20:17

Girgias approved these changes Aug 21, 2024

View reviewed changes

Ayesh force-pushed the curl-prereq-func branch from 68e357c to fce7d17 Compare August 21, 2024 20:29

Girgias force-pushed the curl-prereq-func branch from fce7d17 to 9c50e79 Compare August 24, 2024 12:26

Girgias reviewed Aug 24, 2024

View reviewed changes

Ayesh force-pushed the curl-prereq-func branch 4 times, most recently from b74ce81 to 885364c Compare August 25, 2024 07:58

Ayesh force-pushed the curl-prereq-func branch from 885364c to 882e64e Compare August 25, 2024 14:08

Girgias merged commit a3b7cc2 into php:master Aug 26, 2024

Ayesh deleted the curl-prereq-func branch August 26, 2024 12:35

Ayesh mentioned this pull request Aug 26, 2024

[PHP 8.4][Curl] Add CURLOPT_PREREQFUNCTION and constants JetBrains/phpstorm-stubs#1668

Merged

Ayesh mentioned this pull request Aug 27, 2024

[PHP 8.4] Add CURL_HTTP_VERSION_3(ONLY) constants symfony/polyfill#489

Merged

Ayesh added a commit to Ayesh/php-src that referenced this pull request Aug 30, 2024

ext/curl: Update UPGRADING and NEWS for phpGH-13255

ae0aa7a

ndossche pushed a commit that referenced this pull request Sep 1, 2024

ext/curl: Update UPGRADING and NEWS for GH-13255 (#15668)

fde34bc

[ci skip]

jrfnl mentioned this pull request Sep 15, 2024

PHP 8.4 | NewConstants: detect yet more new constants PHPCompatibility/PHPCompatibility#1767

Merged

Ayesh mentioned this pull request Nov 14, 2024

ext/curl: Fix CURLOPT_PREREQFUNCTION to not continue on error conditions #16790

Open

		var_dump(filter_var($args[1], FILTER_VALIDATE_IP) !== false);
		var_dump(filter_var($args[2], FILTER_VALIDATE_IP) !== false);

Conversation

Ayesh commented Jan 26, 2024

Uh oh!

Uh oh!

devnexen commented Jan 26, 2024

Uh oh!

Uh oh!

SakiTakamachi commented Jan 27, 2024

Uh oh!

Ayesh commented Jan 27, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Girgias commented Feb 5, 2024

Uh oh!

Ayesh commented Feb 5, 2024

Uh oh!

Ayesh commented May 6, 2024

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Girgias commented Aug 21, 2024

Uh oh!

Girgias left a comment

Choose a reason for hiding this comment

Uh oh!

Ayesh commented Aug 21, 2024

Uh oh!

Ayesh commented Aug 21, 2024

Uh oh!

Girgias Aug 24, 2024

Choose a reason for hiding this comment

Uh oh!

Ayesh commented Aug 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants