Apply byte-range options to default Web-file responses - #6983
Conversation
fubhy
added
the
audit
🦋 Changeset detectedLatest commit: a02ada8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
This PR is a failing-test-only branch; the implementation fix described in the PR body still needs to land before it can merge.
Reviewed changes
This run reviewed the single-file test addition in packages/effect/test/unstable/http/HttpPlatform.test.ts and the surrounding HttpPlatform implementation. The new test correctly reproduces the bug: the default web fileWebResponse ignores offset and bytesToRead, returning all four source bytes instead of the requested [2, 3].
- Added regression test for
fileWebResponsebyte-range options. - Confirmed
packages/effect/src/unstable/http/HttpPlatform.ts:159-166streamsfile.stream()in full and discards_options.
⚠️ Implementation fix is still missing
The PR body says the branch starts with focused failing reproduction tests and that the implementation fix should be added to the same branch. The current commit only adds the test; HttpPlatform.ts:159 still discards the options. Please add the fix that makes the test pass before merging.
Technical details
# Missing fileWebResponse byte-range implementation
## Affected sites
- packages/effect/src/unstable/http/HttpPlatform.ts:159-166 — default `web` `fileWebResponse` ignores `_options` and streams `file.stream()` whole.
## Required outcome
- `fileWebResponse` must respect `offset`, `bytesToRead`, and ideally `chunkSize` from its options.
- The regression test at `packages/effect/test/unstable/http/HttpPlatform.test.ts:6-20` must pass.
## Suggested approach
Compute `start = Number(options?.offset ?? 0)` and `end = options?.bytesToRead !== undefined ? start + Number(options.bytesToRead) : undefined`, mirroring the logic already used for `fileResponse` in `HttpPlatform.make:92-102`. Then either:
- slice the `FileLike` if it supports Blob slicing (e.g. `(file as Blob).slice(start, end).stream()`), or
- wrap the stream with `Stream.drop(start)` and `Stream.take(bytesToRead)` and pass the resulting `contentLength` to `Response.stream`.
Set `contentLength` on the `Response.stream` options when serving a range so clients see the correct response length.ℹ️ Nitpicks
- Consider adding a test case for
chunkSizeand for an explicitoffset: 0/bytesToReadat the start of the file, to lock down edge cases once the fix is in place.
@v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
This re-review covered the implementation fix that landed since the prior pullfrog review (cfc4f6afe). The default web fileWebResponse now honors offset, bytesToRead, and chunkSize instead of discarding them.
- Added byte-range slicing and chunk-boundary handling to
packages/effect/src/unstable/http/HttpPlatform.ts:159-188. - Set
contentLengthon the streamed response so clients see the correct range length. - Added a
chunkSizeregression test alongside the existingoffset/bytesToReadtest. - Added a patch changeset describing the fix.
The focused regression test file passes and the effect package type-checks cleanly.
@v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
2 participants
Summary
The default Web-file layer ignores offset, bytesToRead, and chunkSize and streams the complete File-like value; a request for bytes two and three returns all four source bytes.
Important
This PR starts with focused failing reproduction tests. Add the implementation fix to this same branch; CI is expected to fail until that fix is included.
Default Web-file responses ignore byte-range options
Module:
HttpPlatformAudit ID:
unstable-http-httpplatform-fileweb-rangeSeverity / confidence: medium / high
What happens
The default Web-file layer ignores offset, bytesToRead, and chunkSize and streams the complete File-like value; a request for bytes two and three returns all four source bytes.
Why it happens
The default layer names the argument _options and always streams file.stream() in full, making HttpServerResponse.fileWeb range options ineffective.
Expected behavior
fileWebResponse accepts offset, bytesToRead, and chunkSize options and must serve the selected file region.
Relevant implementation
These links and excerpts are pinned to audit base
c9b56ab507f224426ee8388dc450da447ec4715f.packages/effect/src/unstable/http/HttpPlatform.ts:159-166View problematic code at
packages/effect/src/unstable/http/HttpPlatform.ts:159-166View exact lines on GitHub
Reproduction
pnpm test --run packages/effect/test/unstable/http/HttpPlatform.test.tsObserved failure: Failed as intended with [1, 2, 3, 4] instead of [2, 3].
Implementation handoff
The initial reproduction tests on this branch are the regression specification for the implementation fix that should follow in this PR.
pnpm test --run packages/effect/test/unstable/http/HttpPlatform.test.tsAudit provenance
c9b56ab507f224426ee8388dc450da447ec4715fc9b56ab507f224426ee8388dc450da447ec4715funstable-http-httpplatform-fileweb-rangeCloses EFF-421