fix: skip compression for 206 and Content-Range responses#300
Conversation
A 206 Partial Content response carries a Content-Range header whose
byte offsets refer to the original unencoded payload. Compressing
such a response leaves Content-Range describing the wrong byte range,
corrupting data for clients that reassemble partial responses.
Add two guards to the early-exit block:
- +ctx.response.status === 206
- ctx.response.get('Content-Range')
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR extends the Koa compression middleware to skip compression for responses related to byte ranges. The middleware now detects responses with HTTP 206 status (Partial Content) or a Content-Range header and bypasses compression to preserve the range metadata. The implementation adds two guard conditions to the early-exit logic in the middleware, and a test suite validates that both 206 responses and 200 responses carrying Content-Range headers are not compressed. sequenceDiagram
participant Client
participant KoaServer
participant CompressionMiddleware
Client->>KoaServer: HTTP request
KoaServer->>CompressionMiddleware: response context (status, headers, body)
CompressionMiddleware->>CompressionMiddleware: check status/Content-Range/Content-Type
alt is 206 or has Content-Range or multipart/byteranges
CompressionMiddleware->>KoaServer: bypass compression (no Content-Encoding)
else
CompressionMiddleware->>KoaServer: apply compression (set Content-Encoding)
end
KoaServer->>Client: HTTP response (with/without Content-Encoding)
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Reviewer's GuideAdds early-exit guards to skip compression for HTTP 206 Partial Content and any responses carrying a Content-Range header, and introduces tests to verify that these range responses are not compressed and preserve headers/body intact. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #300 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 1 1
Lines 175 179 +4
Branches 30 33 +3
=========================================
+ Hits 175 179 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the early-exit condition, consider using
ctx.statusinstead of+ctx.response.statusfor the 206 check to match the style used elsewhere and avoid unnecessary coercion. - You might want to be explicit that any non-empty
Content-Rangeheader (regardless of status) disables compression by checking for truthiness (Boolean(ctx.response.get('Content-Range'))) and possibly adding that behavior to the comment for future maintainers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the early-exit condition, consider using `ctx.status` instead of `+ctx.response.status` for the 206 check to match the style used elsewhere and avoid unnecessary coercion.
- You might want to be explicit that any non-empty `Content-Range` header (regardless of status) disables compression by checking for truthiness (`Boolean(ctx.response.get('Content-Range'))`) and possibly adding that behavior to the comment for future maintainers.
## Individual Comments
### Comment 1
<location path="lib/index.js" line_range="105-107" />
<code_context>
ctx.compress === false ||
ctx.request.method === 'HEAD' ||
emptyBodyStatuses.has(+ctx.response.status) ||
+ // don't compress partial content: Content-Range describes unencoded byte offsets
+ +ctx.response.status === 206 ||
+ ctx.response.get('Content-Range') ||
ctx.response.get('Content-Encoding') ||
// forced compression or implied
</code_context>
<issue_to_address>
**suggestion:** Consider also guarding against multipart/byteranges responses, which can carry per-part Content-Range headers without a top-level Content-Range
Right now the guard relies on status 206 and a top-level `Content-Range` header to skip compression. In a `multipart/byteranges` response, the top-level header may be absent while each part still has its own `Content-Range`, and those offsets are defined over the uncompressed body. It would be safer to also bypass compression when `ctx.response.type` (or the `Content-Type` header) indicates `multipart/byteranges`.
```suggestion
// don't compress partial content: Content-Range describes unencoded byte offsets
+ctx.response.status === 206 ||
ctx.response.get('Content-Range') ||
// multipart/byteranges can carry per-part Content-Range headers over the uncompressed body
ctx.response.type === 'multipart/byteranges' ||
/^multipart\/byteranges\b/i.test(ctx.response.get('Content-Type') || '') ||
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Problem
koa-compresscompresses206 Partial Contentresponses even though theirContent-Rangeheader describes unencoded byte offsets. After compression the body shrinks butContent-Rangestill refers to the pre-compression range, so a range-aware client stitching parts together receives corrupted data.The same class of bug was recently fixed in hono/compress and @fastify/compress.
Fix
Two new guards in the early-exit block (
lib/index.js):Tests
New
Range Responsesdescribe block in__tests__/index.jswith two cases:206 Partial Content— noContent-Encoding,Content-Rangepreserved200withContent-Rangeheader — sameSummary by Sourcery
Skip response compression when serving partial or ranged content to avoid corrupting
Content-Rangesemantics.Bug Fixes:
Content-Rangeheaders.Content-Rangeheader to ensure clients receive correctly ranged bodies.Tests:
Content-Rangeheader and body.Content-Rangeheader are not compressed despite gzip being accepted.Summary by CodeRabbit
206 Partial Content, when aContent-Rangeheader is present, or when the content type ismultipart/byteranges.206replies and200responses that includeContent-Range, including proper local server setup/teardown after each test.