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

Remove content-length header from 101 Switching Protocols response #2164

Merged

Conversation

chadbarth
Copy link
Contributor

RFC 9110-8.6 (and RFC 7230 before it) states:

A server MUST NOT send a Content-Length header field in any response with a status code of 1xx
(Informational) or 204 (No Content).

The websocket connect response is 101 (Switching Protocols) and should not contain Content-Length as specified above. Drogon's response incorrectly included the Content-Length header.

This causes an issue with the .NET websockets client implementation as described in this Stack Overflow question.

Using setPassthrough on Switching Protocols response suppresses the Content-Length header as well as Content-Type, Date, and Server. I added explicit code to add back the Server header. I'm not sure how desirable that actually is.

And there is a test added that fails without the code changes.

@an-tao
Copy link
Member

an-tao commented Sep 20, 2024

@chadbarth Thank you very much for your patch. However, I think modifying the makeHeaderString function of the HttpResponseImpl class seems to be a better solution, as it would conveniently exclude cases where the status code is 1xx or 204.

@chadbarth
Copy link
Contributor Author

@an-tao Thank you for reviewing. I modified the solution as suggested. Please let me know if you would like me to squash commits or anything else.

@an-tao an-tao merged commit b0c5331 into drogonframework:master Sep 25, 2024
33 of 34 checks passed
@an-tao
Copy link
Member

an-tao commented Sep 25, 2024

@chadbarth Sorry for the late reply. I missed your changes, I've merged them now. Thank you very much!

@tripleslash
Copy link
Contributor

This PR seems to break websockets in chrome

@Mis1eader-dev
Copy link
Member

This PR seems to break websockets in chrome

According to my tests this PR isn't the one that broke WebSockets in Drogon, it's the PR after this one that it starts not working #2170

tripleslash pushed a commit to tripleslash/drogon that referenced this pull request Oct 1, 2024
@tripleslash
Copy link
Contributor

I can confirm this PR is the one that breaks websockets in Chrome

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