-
Notifications
You must be signed in to change notification settings - Fork 7.8k
ext/curl: Add CURLOPT_PREREQFUNCTION
#13255
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
Looking good overall, @mvorisek remark is valid. |
f0efdd4
to
1e856f5
Compare
1e856f5
to
5c06c5b
Compare
In my opinion, |
81c6b26
to
c7bb6f0
Compare
Thank you @SakiTakamachi - I added comments to indicate it's for Curl 7.80.0 throughout everywhere I had |
eafa628
to
c388954
Compare
Can you wait for #13291 and rebase on top of this when it is merged? As the refactoring should simplify the code. |
Awesome, thank you! I can also learn how to do it from your PR too, so this is perfect. |
4f1e413
to
545e929
Compare
60eda9b
to
e53986f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nits, can you also add a trampoline callback test case?
962835e
to
68e357c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM if CI is green
68e357c
to
fce7d17
Compare
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. |
|
fce7d17
to
9c50e79
Compare
var_dump(filter_var($args[1], FILTER_VALIDATE_IP) !== false); | ||
var_dump(filter_var($args[2], FILTER_VALIDATE_IP) !== false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
b74ce81
to
885364c
Compare
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 <girgias@php.net>
885364c
to
882e64e
Compare
CI green finally, and |
`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)
…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)
Commit: php/php-src#13255 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_PREREQFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_PREREQFUNCTION)
Commit: php/php-src#13255 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_PREREQFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_PREREQFUNCTION)
Commit: php/php-src#13255 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_PREREQFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_PREREQFUNCTION)
Commit: php/php-src#13255 PHP.Watch: [PHP 8.4: Curl: New `CURLOPT_PREREQFUNCTION` option](https://php.watch/versions/8.4/CURLOPT_PREREQFUNCTION)
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
orCURL_PREREQFUNC_ABORT
to allow or abort the request.This adds support for it to PHP with required ifdef.