8000 [HttpClient] HttplugClient sets option http_version to null for any version except 1.0 · Issue #48087 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content
8000

[HttpClient] HttplugClient sets option http_version to null for any version except 1.0 #48087

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.

Sign up for GitHub

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

Jump to bottom
Closed
Tetragramat opened this issue Nov 2, 2022 · 9 comments
Closed

[HttpClient] HttplugClient sets option http_version to null for any version except 1.0 #48087

Tetragramat opened this issue Nov 2, 2022 · 9 comments
Labels
Bug HttpClient Status: Needs Review Status: Waiting feedback

Comments

@Tetragramat
Copy link
Contributor

Symfony version(s) affected

4.4.47

Description

\Symfony\Component\HttpClient\HttplugClient::sendPsr7Request overrides protocol version for any version other than 1.0 and sets http_version option as null. Where HttpClient chooses highest version available.

It is problem when target server does not support http2 and request fails.

How to reproduce

On server that supports http2 create request with version 1.1 and send it using \Symfony\Component\HttpClient\HttplugClient. The request will be made using http version 2.0 instead of requested 1.1.

Possible Solution

Stop overriding protocol version in \Symfony\Component\HttpClient\HttplugClient::sendPsr7Request

Additional Context

No response
@Tetragramat Tetragramat added the Bug label Nov 2, 2022
@MatTheCat
Copy link
Contributor
MatTheCat commented Nov 3, 2022 •
edited
Loading

This is also the case in

symfony/src/Symfony/Component/HttpClient/Psr18Client.php

Line 105 in ff56fe8

'http_version' => '1.0' === $request->getProtocolVersion() ? '1.0' : null,

Did not find the rationale in #30413 or #33743 🤔

@nicolas-grekas
Copy link
Member

That is on purpose: PSR-7 forces getProtocolVersion to return something, which means it forbids ALPN-based protocol negotiation. Since all implementations defaults to '1.1', the only way to do opportunistic-'2.0' is to ignore '1.1'.

It is problem when target server does not support http2 and request fails.

Why does a server that advertises '2.0' not support it?

@Tetragramat
Copy link
Contributor Author

Why does a server that advertises '2.0' not support it?

I have no idea. It's Raiffeisen Bank test api https://developers.rb.cz/premium/documentation/02rbczpremiumapi_sandbox

We are getting Stream error in the HTTP/2 framing layer for when 2.0 is being used.

@nicolas-grekas
Copy link
Member

Can you try configuring the client passed to HttplugClient with option http_version set to 1.1?

@Tetragramat
Copy link
Contributor Author

Already tried that. Option http_version is not overridden by default value if its already set even if it's null.

@nicolas-grekas
Copy link
Member

Then let's change that: can you please send a PR to NOT set the http_version in HttplugClient+Psr18Client instead of setting it to null?

@Tetragramat
Copy link
Contributor Author

on branch 4.4?

@nicolas-grekas
Copy link
Member

yes please

@Tetragramat Tetragramat mentioned this issue Nov 3, 2022
Merged
@Tetragramat
Copy link
Contributor Author

done

@fabpot fabpot closed this as completed Nov 4, 2022
fabpot added a commit that referenced this issue Nov 4, 2022
@fabpot
bug #48103 [HttpClient] Do not set http_version instead of setting it…
9b22123 
… to null (Tetragramat)

This PR was merged into the 4.4 branch.

Discussion
----------

[HttpClient] Do not set http_version instead of setting it to null

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       | Fix #48087
| License       | MIT
| Doc PR        |  -

Commits
-------

5f7004d don not set http_version instead of setting it to null
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpClient Status: Needs Review Status: Waiting feedback
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @nicolas-grekas @MatTheCat @xabbuh @Tetragramat @carsonbot
0