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

fix(server): skip automatic Content-Length header for HTTP 304 responses #1801

Merged
merged 1 commit into from
May 7, 2019

Conversation

jxs
Copy link
Contributor

@jxs jxs commented Apr 25, 2019

closes #1797

@jxs jxs changed the title fix(server): skip automatic Content-Length header for HTTP 304 responses feat(server): skip automatic Content-Length header for HTTP 304 responses Apr 25, 2019
Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great start! This prevents adding a content-length: 0, but I believe there's another case that needs to be address also, which when there was a non-empty body. The body should still be dropped and no content-length should be written, I believe.

@jxs
Copy link
Contributor Author

jxs commented May 4, 2019

updated! dropped the body and not setting the content-length for BodyLength::Known(len). But even if body isn't dropped on match msg.bodyit still is being dropped somewhere else.
Added a new test case to assert body is dropped though

@KamilaBorowska
Copy link

HTTP 304 cannot have a body, so the case of HTTP 304 with body shouldn't happen, just saying. From RFC 7232.

A 304 response cannot contain a message-body; it is always terminated by the first empty line after the header fields.

@seanmonstar seanmonstar changed the title feat(server): skip automatic Content-Length header for HTTP 304 responses fix(server): skip automatic Content-Length header for HTTP 304 responses May 7, 2019
@seanmonstar seanmonstar merged commit b342c38 into hyperium:master May 7, 2019
@seanmonstar
Copy link
Member

Thanks!

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.

Hyper should skip automatic Content-Length header for HTTP 304 responses
3 participants