fix(compress): do not compress 206 Partial Content responses#5020
Open
dr4gan0x wants to merge 1 commit into
Open
fix(compress): do not compress 206 Partial Content responses#5020dr4gan0x wants to merge 1 commit into
dr4gan0x wants to merge 1 commit into
Conversation
compress() never looked at the response status, so a 206 was gzipped while its Content-Range header still described the uncompressed byte range, and the Content-Length matching the partial body was dropped. Range clients reassembling parts from such responses end up with corrupted data. Skip compression when the status is 206. Reported as the second issue in honojs#5010.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5020 +/- ##
=======================================
Coverage 79.01% 79.01%
=======================================
Files 154 154
Lines 10578 10579 +1
Branches 2215 2216 +1
=======================================
+ Hits 8358 8359 +1
Misses 2220 2220 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This addresses the second of the three problems reported in #5010 (compress encodes 206 Partial Content).
The middleware bails out for already-encoded, chunked, HEAD, small, non-compressible and no-transform responses, but it never checks the status code. A 206 ends up gzipped while its Content-Range header still refers to the uncompressed byte range, and the Content-Length that matched the partial body gets dropped. A range client stitching parts together from such responses gets corrupted data. This is easy to hit with a handler that does its own range handling, or when proxying an upstream that serves ranges, since the proxy helper passes the upstream status through.
The fix skips compression when the status is 206.
The new test serves a 1024 byte text/plain part with Content-Range and asserts it comes back uncompressed with status, Content-Length and Content-Range intact. It fails without the guard.
The author should do the following, if applicable
bun run format:fix && bun run lint:fixto format the code