Handle Range header for File/Blob responses#1802
Conversation
- Parse Range request header in handleFile and return 206 Partial Content with the correct Content-Range and sliced body - Return 416 Range Not Satisfiable when start is out of bounds - Thread request through all mapResponse/mapCompactResponse call sites in both the Bun and web-standard adapters - Fix compose.ts to always pass mapResponseContext (c.request) to response mappers, not only when maybeStream is true - Add regression tests covering open-ended, bounded, suffix, clamped, and out-of-range cases
WalkthroughAdds HTTP Range header support for File/Blob responses and threads the incoming Request through file-handling code paths across adapters; implements range parsing, validation, 206 partial responses, 416 unsatisfiable responses, and adds tests covering multiple Range scenarios. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant Handler as Request Handler
participant FileHandler as handleFile()
participant RangeProc as Range Processor
participant Response as HTTP Response
Client->>Handler: GET /file (may include Range header)
Handler->>FileHandler: handleFile(file/blob, set, request)
FileHandler->>RangeProc: read request.headers["range"] and file size
alt Range header present & valid
RangeProc->>RangeProc: compute start/end (suffix & clamp support)
RangeProc->>FileHandler: provide sliced Blob (start..end)
FileHandler->>Response: build 206 Partial Content with Content-Range/Content-Length/Accept-Ranges
Response->>Client: 206 + partial body
else Range header present but invalid/unsatisfiable
RangeProc->>Response: build 416 Range Not Satisfiable with Content-Range: */size
Response->>Client: 416
else No Range header
FileHandler->>Response: return 200 OK with full body
Response->>Client: 200 + full body
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/response/range.test.ts (1)
8-10: Consider adding edge case tests (optional).A couple of additional test cases could strengthen coverage:
bytes=0-0(single byte range)bytes=0-4(full file via range should return 206, not 200)These are optional improvements and not blockers.
💡 Additional test cases
it('handles single byte range', async () => { const res = await app.handle( req('/file', { headers: { range: 'bytes=0-0' } }) ) expect(res.status).toBe(206) expect(res.headers.get('content-range')).toBe('bytes 0-0/5') expect(res.headers.get('content-length')).toBe('1') expect(await res.text()).toBe('1') }) it('returns 206 for full file requested via range', async () => { const res = await app.handle( req('/file', { headers: { range: 'bytes=0-4' } }) ) expect(res.status).toBe(206) expect(res.headers.get('content-range')).toBe('bytes 0-4/5') })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/response/range.test.ts` around lines 8 - 10, Add two edge-case tests to the existing Range header suite: use the existing app and content variables in test/response/range.test.ts to create requests for range 'bytes=0-0' and 'bytes=0-4'; for 'bytes=0-0' assert status 206, Content-Range 'bytes 0-0/5', Content-Length '1' and body '1'; for 'bytes=0-4' assert status 206 and Content-Range 'bytes 0-4/5' to ensure full-file range requests return 206 rather than 200.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/adapter/utils.ts`:
- Around line 18-68: The range parsing accepts the invalid "bytes=-" form;
update the handling in the block that uses rangeHeader and the regex
/bytes=(\d*)-(\d*)/ (the logic that inspects match, match[1], match[2],
start/end and returns 416) to treat a match where both capture groups are empty
(match[1] === "" && match[2] === "") as an invalid range: return the same 416
Response with headers { 'content-range': `bytes */${size}` }. Also add tests
covering the invalid "bytes=-" case and a multi-range input like
"bytes=0-10,20-30" to assert current behavior (either rejection or ignored) so
the behavior is documented.
---
Nitpick comments:
In `@test/response/range.test.ts`:
- Around line 8-10: Add two edge-case tests to the existing Range header suite:
use the existing app and content variables in test/response/range.test.ts to
create requests for range 'bytes=0-0' and 'bytes=0-4'; for 'bytes=0-0' assert
status 206, Content-Range 'bytes 0-0/5', Content-Length '1' and body '1'; for
'bytes=0-4' assert status 206 and Content-Range 'bytes 0-4/5' to ensure
full-file range requests return 206 rather than 200.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1406b0ab-6300-4389-8b30-44358e6756f7
📒 Files selected for processing (5)
src/adapter/bun/handler.tssrc/adapter/utils.tssrc/adapter/web-standard/handler.tssrc/compose.tstest/response/range.test.ts
Both capture groups empty is not a valid RFC 7233 range specifier. Previously it fell through to the else branch and was silently treated as bytes=0- (full file, 206). Now returns 416 Range Not Satisfiable. Also documents multi-range behaviour (only first range honoured) via test.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/adapter/utils.ts`:
- Around line 18-19: The range-check currently gates on `size` (e.g., the `if
(rangeHeader && size)`), which skips validation for empty bodies and prevents
returning 416 for `Range: bytes=0-0` when `response.size === 0`; remove the
`size` truthy guard so the `rangeHeader` branch runs even for zero-length
responses (keep the existing unsatisfiable-range logic that emits `416` and
`Content-Range: bytes */0`) — update the conditional around
`request?.headers.get('range')` to only rely on `rangeHeader` and let the
existing `response.size` checks inside the range-handling code determine
satisfiability.
- Around line 22-26: The 416 and 206 range-response branches drop headers
because they either build headers directly or use Object.assign on set.headers
(which fails for Headers instances); update the 416 branches (where Response is
created for no range match and for invalid range) and the 206 branch (where
rangeHeaders is merged with set.headers) to call the existing mergeHeaders()
helper to merge protocol range headers with set.headers so that Headers
instances and CORS/ETag/etc. entries are preserved; ensure you pass the
constructed rangeHeaders and set.headers into mergeHeaders() (or the helper's
defined arg order) before creating the Response.
- Around line 62-68: The sliced Blob loses its MIME type because Blob.slice was
called without the third contentType argument; update the slice call used inside
the Response constructor (the casted (response as unknown as { slice(start:
number, end: number): Blob }).slice(start, end + 1)) to pass the original blob's
MIME type as the third argument (use response.type) so the resulting 206
Response preserves Content-Type for typed File/Blob bodies.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 401f67af-073e-49be-be0a-18ecb572e2ee
📒 Files selected for processing (2)
src/adapter/utils.tstest/response/range.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/response/range.test.ts
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/adapter/utils.ts (1)
17-77: Range handling implementation looks solid.The implementation correctly handles RFC 7233 range forms including bounded ranges, open-ended ranges, suffix ranges, and invalid inputs. Previous review feedback has been properly addressed:
- Invalid
bytes=-returns 416mergeHeaders()preserves existing headers on range responses- MIME type is preserved via the third argument to
slice()One optional enhancement: the regex on line 20 is case-sensitive. While RFC 7233 specifies the "bytes" unit is case-insensitive, ignoring unrecognized range formats and returning 200 is valid behavior per the spec, so this is a nitpick.
💅 Optional: case-insensitive range matching
- const match = /bytes=(\d*)-(\d*)/.exec(rangeHeader) + const match = /bytes=(\d*)-(\d*)/i.exec(rangeHeader)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/adapter/utils.ts` around lines 17 - 77, The regex used to parse the Range header is currently case-sensitive; update the matching in the rangeHeader handling (where /bytes=(\d*)-(\d*)/.exec(rangeHeader) is used) to perform case-insensitive matching (or normalize the header before matching) so units like "Bytes=" or "BYTES=" are accepted per RFC 7233; keep the rest of the start/end/suffix logic unchanged and ensure behavior for unrecognized formats still falls through to the 200 path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/adapter/utils.ts`:
- Around line 17-77: The regex used to parse the Range header is currently
case-sensitive; update the matching in the rangeHeader handling (where
/bytes=(\d*)-(\d*)/.exec(rangeHeader) is used) to perform case-insensitive
matching (or normalize the header before matching) so units like "Bytes=" or
"BYTES=" are accepted per RFC 7233; keep the rest of the start/end/suffix logic
unchanged and ensure behavior for unrecognized formats still falls through to
the 200 path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6be033a9-02d4-4708-bf6f-645f7acb3774
📒 Files selected for processing (1)
src/adapter/utils.ts
this closes #1790
When returning a Blob or File response, the Range request header was ignored, resulting in clients always receiving the full file.
The bug had two layers. First,
handleFilenever received therequestobject, preventing it from reading the header. Additionally,compose.tsonly forwardedc.requestto the response mappers whenmaybeStreamwastrue. This meant that for any plain file route, the request was dropped before reachinghandleFile.Changes:
maybeStreamguard incompose.ts, ensuringc.requestis always forwarded to response mappers.handleFilenow accepts an optional request parameter and handles range requests by slicing the blob and returning a206status with the appropriateContent-RangeandContent-Lengthheaders.bytes=start-end,bytes=start-,bytes=-suffix), clamps oversized end positions per specification, and returns a416status when the start is out of bounds.mapResponseandmapCompactResponsecall sites in both the bun and web-standard adapters.A regression test was added to cover all the above cases.
Summary by CodeRabbit
New Features
Bug Fixes
Tests