Skip to content
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

[HttpClient] Fix cURL default options for PHP 8.4 #54830

Merged
merged 1 commit into from
May 3, 2024

Conversation

alexandre-daubois
Copy link
Contributor

@alexandre-daubois alexandre-daubois commented May 3, 2024

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Issues -
License MIT

PHP 8.4 brings a change in ext/curl (php/php-src#13291) that requires CurlResponse to be updated. Curl callbacks cannot be set to null anymore and requires real callable.

Here is (one of) the CI error it fixes:

10) Symfony\Component\HttpClient\Tests\CurlHttpClientTest::testGzipBroken
Failed asserting that exception of type "TypeError" matches expected exception "Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface". Message was: "curl_setopt_array(): Argument #2 ($options) must be a valid callback for option CURLOPT_PROGRESSFUNCTION, no array or string given" at
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Response/CurlResponse.php:175
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Internal/Canary.php:32
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php:90
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Response/TransportResponseTrait.php:218
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Response/CommonResponseTrait.php:68
/home/runner/work/symfony/symfony/src/Symfony/Component/HttpClient/Response/CurlResponse.php:232
/home/runner/work/symfony/symfony/src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php:1113

@carsonbot

This comment was marked as resolved.

@alexandre-daubois alexandre-daubois changed the base branch from 7.1 to 5.4 May 3, 2024 08:34
@xabbuh xabbuh modified the milestones: 7.1, 5.4 May 3, 2024
@xabbuh
Copy link
Member

xabbuh commented May 3, 2024

see https://github.com/symfony/symfony/actions/runs/8936565926/job/24547155340?pr=54832#step:8:2947 for how it looks like without these changes

@derrabus derrabus changed the title [HttpClient] Fix cURL default options [HttpClient] Fix cURL default options for PHP 8.4 May 3, 2024
@derrabus
Copy link
Member

derrabus commented May 3, 2024

Thank you @alexandre-daubois.

@derrabus derrabus merged commit 52099f9 into symfony:5.4 May 3, 2024
7 of 10 checks passed
nicolas-grekas added a commit that referenced this pull request May 13, 2024
…-daubois)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpClient] Revert fixing curl default options

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Reverts #54830 because php/php-src#14165 got merged and fixes the unintentional BC break.

(CI still red but should go to green when new nightlies are built)

Commits
-------

365f7b4 [HttpClient] Revert fixing curl default options
@fabpot fabpot mentioned this pull request May 17, 2024
This was referenced Jun 2, 2024
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.

None yet

4 participants