Skip to content

Added missing "reason" to ensure backward compatibility #190

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

Merged
merged 2 commits into from
May 1, 2025

Conversation

bsnych
Copy link
Contributor

@bsnych bsnych commented Oct 23, 2024

Reason added for Informational response to socket connection upgrade. This ensures backwards compatibility for older socket libraries like: OCaml/websokets

@Kriechi
Copy link
Member

Kriechi commented Oct 23, 2024

Please add a test to make sure we don't regress in the future.

I couldn't find an explicit statement in RFC6455 about mandating exactly this reason - though if you are reporting that some clients are expecting exactly this reason, I think its valid to define it as default - and until somebody requests otherwise - not configurable.

@Kriechi
Copy link
Member

Kriechi commented Jan 25, 2025

@mhils have you seen issues around this also with mitmproxy?

@mhils
Copy link
Member

mhils commented Apr 26, 2025

I've seen problems with missing reason phrases before, not in this specific context.

In mitmproxy we just forward as-is and internally operate on the status code only.

bsnych and others added 2 commits May 1, 2025 16:38
Reason added for Informational response to socket connection upgrade. This ensures backwards compatibility for older socket libraries like: OCaml/websokets
@Kriechi
Copy link
Member

Kriechi commented May 1, 2025

for reference: this is the section in RFC6455 that mentions Switching Protocols:
https://datatracker.ietf.org/doc/html/rfc6455#section-1.2
https://datatracker.ietf.org/doc/html/rfc6455#section-1.3

A Status-Line with a 101 response code as per RFC 2616
[RFC2616]. Such a response could look like "HTTP/1.1 101
Switching Protocols".

@Kriechi Kriechi merged commit 8b0bac4 into python-hyper:main May 1, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants