Skip to content

Fix Blob.slice() for negative start and inverted ranges#57374

Open
durvesh1992 wants to merge 1 commit into
react:mainfrom
durvesh1992:fix-blob-slice-negative-inverted
Open

Fix Blob.slice() for negative start and inverted ranges#57374
durvesh1992 wants to merge 1 commit into
react:mainfrom
durvesh1992:fix-blob-slice-negative-inverted

Conversation

@durvesh1992

Copy link
Copy Markdown

Summary

Blob.slice(start, end) is modeled on the W3C Blob API, where both start and end may be negative (relative to the end of the blob) and an end that precedes start yields an empty blob.

The current implementation handles a negative end but not a negative start:

if (typeof start === 'number') {
  if (start > size) { start = size; }   // only the upper bound is checked
  offset += start;
  size -= start;
  if (typeof end === 'number') {
    if (end < 0) { end = this.size + end; }  // negative end IS handled
    if (end > this.size) { end = this.size; }
    size = end - start;                       // not clamped to >= 0
  }
}

So:

  • blob.slice(-100) sets offset to a negative value and makes size larger than the blob, instead of slicing the last 100 bytes.
  • blob.slice(200, 100) (end before start) produces a negative size.

Fix

  • Normalize a negative start relative to the blob size (Math.max(this.size + start, 0)), mirroring the existing end handling.
  • Clamp the resulting size with Math.max(end - start, 0) so an inverted range yields an empty blob.

Test plan

Added tests to Libraries/Blob/__tests__/Blob-test.js for negative start, negative end, and end-precedes-start. The negative-start and inverted-range cases fail before this change (negative offset / negative size) and pass after; existing slice tests and the rest of the suite still pass (8/8). ESLint clean on the changed files.

Changelog:

[General] [Fixed] - Fix Blob.slice() for negative start offsets and inverted ranges

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 30, 2026
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Jun 30, 2026

@javache javache left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please revert the changes to no-deep-imports

Blob.slice() handled a negative `end` (relative to the end of the blob)
but not a negative `start`, so `blob.slice(-100)` produced a negative
offset and an oversized blob instead of slicing the last 100 bytes.

It also computed `size = end - start` without clamping, so calling
`slice(start, end)` with `end` before `start` produced a negative size.

Normalize a negative start relative to the blob size (matching the
existing end handling) and clamp the resulting size to a minimum of 0,
per the W3C Blob.slice() semantics.

Add tests for negative start, negative end, and end-precedes-start.
@durvesh1992 durvesh1992 force-pushed the fix-blob-slice-negative-inverted branch from 3e8ace9 to 9d2aa9c Compare June 30, 2026 09:28
@durvesh1992

Copy link
Copy Markdown
Author

Thanks for the review! Done — I've rebased this branch to drop the no-deep-imports changes (they were unintentionally included from another branch). This PR now only contains the Blob.slice() fix and its tests. The no-deep-imports change is tracked separately in #57373.

@meta-codesync

meta-codesync Bot commented Jun 30, 2026

Copy link
Copy Markdown

@javache has imported this pull request. If you are a Meta employee, you can view this in D110173869.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants