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

Gracefully handle HTTP/2 upgrade request #1501

Closed
ChristianCiach opened this issue May 25, 2022 · 7 comments
Closed

Gracefully handle HTTP/2 upgrade request #1501

ChristianCiach opened this issue May 25, 2022 · 7 comments

Comments

@ChristianCiach
Copy link

ChristianCiach commented May 25, 2022

I know that HTTP/2 is out of scope of this project. But I am not asking to support HTTP/2.

More and more http clients try to upgrade the connection to HTTP/2. Uvicorn is free to not honor this request. Unfortunately, instead of ignoring the upgrade request, Uvicorn responds with status 400 Bad Request and the message "Unsupported upgrade request".

According to https://developer.mozilla.org/en-US/docs/Web/HTTP/Protocol_upgrade_mechanism the server should just ignore the upgrade request:

If the server decides to upgrade the connection, it sends back a 101 Switching Protocols response status with an Upgrade header that specifies the protocol(s) being switched to. If it does not (or cannot) upgrade the connection, it ignores the Upgrade header and sends back a regular response (for example, a 200 OK).

We continue to encounter this issue because the HttpClient class of modern OpenJDK versions tries to upgrade the connection to HTTP/2 by default. Uvicorn should just ignore these headers and process the requests as if these headers were not present.

@euri10
Copy link
Member

euri10 commented Jun 14, 2022

interesting, low priority but something to consider

@ChristianCiach
Copy link
Author

I don't agree with the "low priority" assessment. This is a blatant violation of the http specification and more and more (standards compliant) http clients break just by using them with their default settings.

@Kludex
Copy link
Sponsor Member

Kludex commented Sep 7, 2022

PR welcome.

@ChristianCiach
Copy link
Author

The existence of a PR has nothing to do with the priority assessment. In fact, giving this issue a higher priority may incentivize people to actually fix this issue. Saying that this violation of the http spec is "low prio" is more a statement about the level of professionalism of this software, or the lack thereof.

For the record: Yes, I would try to fix this myself, but you have to realize that not every user of your product is a software developer that uses the language that your software is written in. I am fluent in Java and Go, but I cannot fix python code. Or at least not at a level that you would ever consider to accept contributions.

@encode encode locked as too heated and limited conversation to collaborators Sep 7, 2022
@Kludex
Copy link
Sponsor Member

Kludex commented Sep 7, 2022

Locking the issue as I'm not liking the tone of the reporter.

PR is still welcome. 🙏

If no one implements it, I will, at some point, on my free time (unpaid free time).

@euri10
Copy link
Member

euri10 commented Sep 9, 2022

omg, happy to come back to see your fluency in being a total asshat @ChristianCiach
raising the issue as high priority 👍 !

@Kludex
Copy link
Sponsor Member

Kludex commented Oct 19, 2022

It will be available on uvicorn 0.19.0.

@Kludex Kludex closed this as completed Oct 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants