Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[HttpClient] Fix type error with http_version 1.1#51876

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:6.3fromFilnor:http-version-bug
Oct 7, 2023
Merged

[HttpClient] Fix type error with http_version 1.1#51876

fabpot merged 1 commit intosymfony:6.3fromFilnor:http-version-bug
Oct 7, 2023

Conversation

@Filnor
Copy link
Contributor

Fix a type error by removing a nested setProtocolVersions() call in AmpHttpClient::request()

QA
Branch?6.3
Bug fix?yes
New feature?no
Deprecations?no
TicketsFix#51868
LicenseMIT

Description

In commit46a66956 the lines 114-118 of AmpHttpClient.php were changed to use thematch() expression instead ofswitch-case:

-            switch ((float) $options['http_version']) {-                case 1.0: $request->setProtocolVersions(['1.0']); break;-                case 1.1: $request->setProtocolVersions(['1.1', '1.0']); break;-                default: $request->setProtocolVersions(['2', '1.1', '1.0']); break;-            }+            $request->setProtocolVersions(match ((float) $options['http_version']) {+                1.0 => ['1.0'],+                1.1 => $request->setProtocolVersions(['1.1', '1.0']),+                default => ['2', '1.1', '1.0'],+            });

If the provided$options['http_version'] is1.1, the code will call$request->setProtocolVersions() inside thematch(), unlike with 1.0 and the default case. Thematch will use the return value ofsetProtocolVersions(), but since it's a void function, the return value isnull.

This then results in aTypeError assetProtocolVersions() (the "outer" one) expects an array.

How to reproduce

// First, run "composer require symfony/http-client" and " composer require amphp/http-client"// Then, execute this file:require_once__DIR__.'/vendor/autoload.php';useSymfony\Component\HttpClient\AmpHttpClient;$client =newAmpHttpClient();$response =$client->request('GET','http://example.com',    ['http_version' =>'1.1'    ]);// The script will fail with the TypeError and never reach this pointvar_dump($response->getStatusCode());

Solution

Change line 116 to match line 115 & 117:

$request->setProtocolVersions(match ((float) $options['http_version']) {    1.0 => ['1.0'],-   1.1 => $request->setProtocolVersions(['1.1', '1.0']),+   1.1 => ['1.1', '1.0'],    default => ['2', '1.1', '1.0'],});

pableu and emptycurse reacted with thumbs up emoji
Fix a type error by removing a nested setProtocolVersions() call in AmpHttpClient::request()
@FilnorFilnor marked this pull request as ready for reviewOctober 6, 2023 15:52
@fabpot
Copy link
Member

Good catch, thanks@Filnor.

pableu and emptycurse reacted with hooray emoji

@fabpotfabpot merged commit82f21e8 intosymfony:6.3Oct 7, 2023
@fabpotfabpot mentioned this pull requestOct 21, 2023
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@OskarStarkOskarStarkOskarStark approved these changes

@derrabusderrabusderrabus approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Filnor@fabpot@OskarStark@derrabus@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp