Skip to content

lib: reject SharedArrayBuffer in web APIs per spec#62632

Open
thisalihassan wants to merge 2 commits intonodejs:mainfrom
thisalihassan:fix/reject-sharedarraybuffer-web-apis
Open

lib: reject SharedArrayBuffer in web APIs per spec#62632
thisalihassan wants to merge 2 commits intonodejs:mainfrom
thisalihassan:fix/reject-sharedarraybuffer-web-apis

Conversation

@thisalihassan
Copy link
Copy Markdown
Contributor

@thisalihassan thisalihassan commented Apr 8, 2026

The WebIDL spec requires BufferSource/ArrayBufferView (without AllowShared) to reject SharedArrayBuffer and views backed by one.
Node.js web APIs handle this inconsistently, moved the SAB BufferSource/ArrayBufferView validators from internal/crypto/webidl into the shared internal/webidl module and fixes the non-compliant call sites:

  • ReadableStreamBYOBReader.read() failed late at transfer now rejects upfront with a clear error
  • ReadableByteStreamController.enqueue() same as above

Blob constructor changes (semver-major) will be submitted as a separate PR.

Quick Research:
Deno already rejects these cases while Bun does not (as of Bun 1.3).

Fixes: #59688

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 8, 2026
@panva panva added the semver-major PRs that contain breaking changes and should be released in the next major version. label Apr 8, 2026
@Renegade334
Copy link
Copy Markdown
Member

The webstream changes are not semver-major IMO, as passing a shared view is currently a broken operation – this change simply validates the buffer up-front, rather than throwing a difficult-to-catch exception when data starts flowing.

As such, this could be split down into two PRs, one semver-patch for the webstream changes, one semver-major for blob.

The compression stream change here is unnecessary because the underlying adapter doesn't accept shared array buffers, so the validation change here is a no-op. (ref #62163)

@Renegade334 Renegade334 added the web-standards Issues and PRs related to Web APIs label Apr 8, 2026
@thisalihassan
Copy link
Copy Markdown
Contributor Author

thisalihassan commented Apr 8, 2026

@Renegade334 thanks for the detailed feedback.

  1. passing SAB backed views already throws during transfer I am just failing earlier with a clearer message
  2. Makes sense to split this into two PRs. I will separate the Blob change (semver-major) once this lands, will wait for other maintainers to weigh in on the approach.
  3. Will revert compression stream changes as covered by your PR already

I will check how the adapter handles it with your #62163 in mind.

Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
@thisalihassan thisalihassan force-pushed the fix/reject-sharedarraybuffer-web-apis branch from 2f0223b to cec023c Compare April 8, 2026 11:00
@thisalihassan
Copy link
Copy Markdown
Contributor Author

@panva @Renegade334 can we drop semver-major label and add semver-patch instead? As I have reverted the blob changes

@panva panva added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed semver-major PRs that contain breaking changes and should be released in the next major version. labels Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. web-standards Issues and PRs related to Web APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Non-compliant handling of shared array buffers/views in web APIs

4 participants