Skip to content

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

Merged
seanmonstar merged 1 commit intohyperium:masterfrom
jxs:master
May 7, 2019
Merged

fix(server): skip automatic Content-Length header for HTTP 304 responses#1801
seanmonstar merged 1 commit intohyperium:masterfrom
jxs:master

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
seanmonstar
seanmonstar requested changes May 3, 2019
View reviewed changes
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

@sugar700
Copy link

sugar700 commented May 6, 2019

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

seanmonstar commented May 7, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanmonstar seanmonstar seanmonstar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

3 participants

@jxs @sugar700 @seanmonstar