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

Ensure that 206 responses are never compressed #3291

Merged
merged 1 commit into from Aug 22, 2016

Conversation

@kanongil
Copy link
Contributor

@kanongil kanongil commented Aug 17, 2016

Since the content-range header relies on an unmodified entity response, we need to leave it alone.

The content-range header relies on an unmodified entity response
@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Aug 22, 2016

I don't see anything in the spec that forbids applying content encoding to a 206 response.

@mnot any advice?

Loading

@kanongil
Copy link
Contributor Author

@kanongil kanongil commented Aug 22, 2016

@hueniverse 206 + content-encoding is indeed perfectly valid, and Inert supports this for pre-compressed files.

However, the current implementation doesn't work for dynamic responses, as the content-range header is determined based on the non-encoded representation. Once it is encoded, it is no longer valid (length doesn't match).

Also, I dont believe the compressed stream is actually guaranteed to be idempotent, which it needs to be in order to reliably stitch a complete response together.

Loading

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Aug 22, 2016

Should the range be calculated based on the encoded or raw payload? I'm too busy to dig up the spec.

Loading

@hueniverse hueniverse merged commit 1853c80 into hapijs:master Aug 22, 2016
2 checks passed
Loading
@hueniverse hueniverse added the bug label Aug 22, 2016
@hueniverse hueniverse added this to the 15.0.0 milestone Aug 22, 2016
@hueniverse hueniverse self-assigned this Aug 22, 2016
@mnot
Copy link

@mnot mnot commented Aug 23, 2016

It's buried a bit. In http://httpwg.org/specs/rfc7233.html#header.content-range -

The "Content-Range" header field is sent in a single part 206 (Partial Content) response to indicate the partial range of the selected representation enclosed as the message payload, sent in each part of a multipart 206 response to indicate the range enclosed within each body part, and sent in 416 (Range Not Satisfiable) responses to provide information about the selected representation.

Emphasis mine. "Selected representation" is defined in http://httpwg.org/specs/rfc7231.html#representations -

An origin server might be provided with, or be capable of generating, multiple representations that are each intended to reflect the current state of a target resource. In such cases, some algorithm is used by the origin server to select one of those representations as most applicable to a given request, usually based on content negotiation. This "selected representation" is used to provide the data and metadata for evaluating conditional requests [RFC7232] and constructing the payload for 200 (OK) and 304 (Not Modified) responses to GET (Section 4.3.1).

So, Content-Range is specifying a range of the result of any content negotiation, including encoding.

Loading

@hueniverse
Copy link
Contributor

@hueniverse hueniverse commented Aug 23, 2016

Thanks @mnot!

Loading

@lock
Copy link

@lock lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

Loading

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants