Skip to content

fix: range reads for file backend#1087

Merged
ferhatelmas merged 1 commit intomasterfrom
ferhat/range-file
May 8, 2026
Merged

fix: range reads for file backend#1087
ferhatelmas merged 1 commit intomasterfrom
ferhat/range-file

Conversation

@ferhatelmas
Copy link
Copy Markdown
Member

@ferhatelmas ferhatelmas commented May 8, 2026

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

File backend range header parser assumes both numeric.
Upload part copy requires a range header. It also has off by one error.

What is the new behavior?

Parser is permissive as expected.
Upload part copy a range is optional. Size check is done correctly.

Copilot AI review requested due to automatic review settings May 8, 2026 11:46
@ferhatelmas ferhatelmas requested a review from a team as a code owner May 8, 2026 11:46
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes HTTP Range request handling in the file-backed storage backend by adding a more permissive and spec-aligned byte range parser (supporting explicit, open-ended, and suffix ranges) and validating invalid ranges with a 416 response.

Changes:

  • Replaced ad-hoc Range parsing in FileBackend.getObject() with a dedicated parseByteRange() helper that supports bytes=start-end, bytes=start-, and bytes=-suffixLength.
  • Added unit tests for file-backend range reads (including invalid range rejection).
  • Extended S3 acceptance tests to cover suffix/open-ended ranges and invalid range behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/storage/backend/file.ts Implements robust byte-range parsing and returns correct 206 metadata/content-range for file backend reads.
src/storage/backend/file.test.ts Adds unit coverage for explicit, open-ended, suffix, capped, and invalid range behaviors.
acceptance/specs/s3.test.ts Adds acceptance assertions for suffix/open-ended Range requests and invalid range (416) responses.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/storage/backend/file.ts Outdated
@coveralls
Copy link
Copy Markdown

coveralls commented May 8, 2026

Coverage Report for CI Build 25557465900

Coverage increased (+0.2%) to 74.249%

Details

  • Coverage increased (+0.2%) from the base build.
  • Patch coverage: 37 of 37 lines across 3 files are fully covered (100%).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 10275
Covered Lines: 8036
Line Coverage: 78.21%
Relevant Branches: 5934
Covered Branches: 3999
Branch Coverage: 67.39%
Branches in Coverage %: Yes
Coverage Strength: 409.27 hits per line

💛 - Coveralls

fix off by one size check in upload part copy
while making range optional

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas force-pushed the ferhat/range-file branch from 5b5ab18 to dedfd52 Compare May 8, 2026 13:11
@ferhatelmas ferhatelmas merged commit abdecce into master May 8, 2026
18 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/range-file branch May 8, 2026 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants