Recompute Content-Length on compression
#480
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Content-Length holds the number of bytes in an HTTP response after encoding and compression. Our compress_response() function breaks that: when the response it's compressing already includes a Content-Length header, it may shorten the body but leave the header unchanged. I noticed this because of redlib-org#465, since the textual error message proxied from Reddit gets compressed but already has Content-Length. The issue is masked in normal operation because we don't compress the binary image data that Reddit is supposed to return. In debug builds, hyper catches the issue[1] and panics, but in release builds we return a nonconformant HTTP response. Fix the issue by removing the header if we choose to compress, letting hyper compute the new value for us. [1] https://github.com/hyperium/hyper/blob/v0.14.32/src/proto/h1/role.rs#L708-L725
Member
|
Thanks for the PR! I thought Hyper would recompute that regardless but alas it's low level enough to take me at my word 😅 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Content-Lengthholds the number of bytes in an HTTP response after encoding and compression. Ourcompress_response()function breaks that: when the response it's compressing already includes aContent-Lengthheader, it may shorten the body but leave the header unchanged.I noticed this because of #465, since the textual error message proxied from Reddit gets compressed but already has
Content-Length. The issue is masked in normal operation because we don't compress the binary image data that Reddit is supposed to return. In debug builds, hyper catches the issue and panics, but in release builds we return a nonconformant HTTP response.Fix the issue by removing the header if we choose to compress, letting hyper compute the new value for us.