Skip to content

Pin how stream constructors read source, sink and transformer members #7784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jasnell merged 6 commits into from
Oct 16, 2026
Merged

Pin how stream constructors read source, sink and transformer members #7784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e2a0012
Pin how byte streams reject arguments that are not views
jasnell Oct 16, 2026
55ea6c5
Validate byte-stream views through one pair of helpers
jasnell Oct 16, 2026
1b63f77
Read a respondWithNewView() view's metadata once
jasnell Oct 16, 2026
94bd175
Drop the unreachable buffer-length check from byte-view validation
jasnell Oct 16, 2026
fc7687d
Fold getViewInfo into getUnsharedViewInfo
jasnell Oct 16, 2026
c556a53
Convert byte-stream views before the method steps
jasnell Oct 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 56 additions & 73 deletions src/per_isolate/webstreams/readable.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1134,18 +1134,7 @@ class ReadableStreamBYOBReader implements ReadableStreamBYOBReaderType {
options: ReadableStreamBYOBReaderReadOptions
): Promise<ReadableStreamReadResult<T>> {
// --- View validation (spec read(view, options) steps 1-3) ---
if (!isArrayBufferView(view)) {
throw new TypeError('view must be an ArrayBufferView');
}
const info = getUnsharedViewInfo(view, 'view');
if (info.byteLength === 0) {
throw new TypeError('view must have a non-zero byteLength');
}
if (ArrayBufferPrototypeByteLengthGet(info.buffer) === 0) {
throw new TypeError(
"view's backing buffer must have a non-zero byteLength"
);
}
const info = getNonEmptyUnsharedViewInfo(view, 'view');
options = toDictionary(
options,
'options must be an object'
Expand Down Expand Up @@ -1313,15 +1302,21 @@ interface ViewInfo {
) => ArrayBufferView;
}

// PRECONDITION: caller has verified isArrayBufferView(view). Every
// ArrayBufferView is either a typed array (identified via the internal
// [[TypedArrayName]] slot) or a DataView; the DataView getters brand-check
// the receiver, so nothing dishonest survives this function.
function getViewInfo(view: ArrayBufferView): ViewInfo {
// A view's ViewInfo at the byte-stream trust boundaries (read(view),
// enqueue(), respondWithNewView()), after WebIDL's ArrayBufferView
// conversion, without [AllowShared], so a value that is not a view, or a
// view backed by a SharedArrayBuffer, throws. `what` names the argument in
// messages. Every ArrayBufferView is either a typed array (identified via
// the internal [[TypedArrayName]] slot) or a DataView.
function getUnsharedViewInfo(view: unknown, what: string): ViewInfo {
if (!isArrayBufferView(view)) {
throw new TypeError(`${what} must be an ArrayBufferView`);
}
let info: ViewInfo;
const name = TypedArrayPrototypeGetSymbolToStringTag(view);
if (name !== undefined) {
const viewCtor = TypedArrayCtorByName[name];
return {
info = {
buffer: TypedArrayPrototypeGetBuffer(view),
byteOffset: TypedArrayPrototypeGetByteOffset(view),
byteLength: TypedArrayPrototypeGetByteLength(view),
Expand All @@ -1331,55 +1326,37 @@ function getViewInfo(view: ArrayBufferView): ViewInfo {
length: TypedArrayPrototypeGetLength(view),
viewCtor,
};
} else {
const dataView = view as DataView;
const byteLength = DataViewPrototypeGetByteLength(dataView);
info = {
buffer: DataViewPrototypeGetBuffer(dataView),
byteOffset: DataViewPrototypeGetByteOffset(dataView),
byteLength,
elementSize: 1,
length: byteLength,
viewCtor: DataView,
};
}
const dataView = view as DataView;
const byteLength = DataViewPrototypeGetByteLength(dataView);
return {
buffer: DataViewPrototypeGetBuffer(dataView),
byteOffset: DataViewPrototypeGetByteOffset(dataView),
byteLength,
elementSize: 1,
length: byteLength,
viewCtor: DataView,
};
}

// getViewInfo() for the byte-stream trust boundaries, which reject
// SharedArrayBuffer-backed views (WebIDL ArrayBufferView without
// [AllowShared]).
function getUnsharedViewInfo(view: ArrayBufferView, what: string): ViewInfo {
const info = getViewInfo(view);
if (isSharedArrayBuffer(info.buffer)) {
throw new TypeError(`${what} must not be backed by a SharedArrayBuffer`);
}
return info;
}

// Validate and normalize a user-provided chunk for a byte stream at the
// enqueue()/respondWithNewView() trust boundary: snapshot metadata, reject
// zero-length views and zero-length (or detached — detached buffers report
// byteLength 0) buffers, and transfer the backing buffer. The returned
// triple references the TRANSFERRED buffer; the caller's view is detached.
// All byte-stream transfers are to fixed length (spec
// TransferArrayBuffer), so a source cannot shrink a buffer we hold.
function validateAndTransferView(view: ArrayBufferView): ByteQueueEntry {
if (!isArrayBufferView(view)) {
throw new TypeError('chunk must be an ArrayBufferView');
}
const info = getUnsharedViewInfo(view, 'chunk');
// getUnsharedViewInfo() for a view that must hold bytes (spec read(view)
// steps 1-3, enqueue() steps 1-2): a zero-length view throws. The spec
// also checks that the view's buffer is neither zero-length nor detached,
// because a view's [[ByteLength]] slot keeps its value once the buffer is
// detached. Here the byteLength getters read 0 for a detached or
// out-of-bounds typed array (a DataView's throw), and a view that holds
// bytes lies within its buffer, so those checks cannot fail.
function getNonEmptyUnsharedViewInfo(view: unknown, what: string): ViewInfo {
const info = getUnsharedViewInfo(view, what);
if (info.byteLength === 0) {
throw new TypeError('chunk must have a non-zero byteLength');
throw new TypeError(`${what} must have a non-zero byteLength`);
}
if (ArrayBufferPrototypeByteLengthGet(info.buffer) === 0) {
throw new TypeError(
"chunk's backing buffer must have a non-zero byteLength"
);
}
return {
buffer: ArrayBufferPrototypeTransferToFixedLength(info.buffer),
byteOffset: info.byteOffset,
byteLength: info.byteLength,
};
return info;
}

interface SourceAlgorithms {
Expand Down Expand Up @@ -1958,7 +1935,7 @@ let byteControllerRespond: (
) => void;
let byteControllerRespondWithNewView: (
controller: ReadableByteStreamController,
view: ArrayBufferView
info: ViewInfo
) => void;
// The queued byte controller's autoAllocateChunkSize; undefined for any
// other controller.
Expand Down Expand Up @@ -2040,18 +2017,16 @@ class ReadableStreamBYOBRequest implements ReadableStreamBYOBRequestType {

respondWithNewView(view: ArrayBufferView): void {
assertIsReadableStreamBYOBRequest(this);
// WebIDL converts the view before the method steps run.
const info = getUnsharedViewInfo(view, 'view');
if (this.#controller === null) {
throw new TypeError('This BYOB request has been invalidated');
}
if (!isArrayBufferView(view)) {
throw new TypeError('view must be an ArrayBufferView');
}
// Spec step 2: detached buffers are TypeError, not RangeError.
const info = getUnsharedViewInfo(view, 'view');
if (ArrayBufferPrototypeDetachedGet(info.buffer)) {
throw new TypeError("The view's buffer has been detached");
}
byteControllerRespondWithNewView(this.#controller, view);
byteControllerRespondWithNewView(this.#controller, info);
}
}

Expand Down Expand Up @@ -2128,8 +2103,8 @@ class ReadableByteStreamController implements ReadableByteStreamControllerType {
controller.#respond(bytesWritten);
};

byteControllerRespondWithNewView = (controller, view) => {
controller.#respondWithNewView(view);
byteControllerRespondWithNewView = (controller, info) => {
controller.#respondWithNewView(info);
};

getByteControllerAutoAllocateChunkSize = (controller) => {
Expand Down Expand Up @@ -2290,15 +2265,23 @@ class ReadableByteStreamController implements ReadableByteStreamControllerType {
// and pull is not called (byteControllerEnqueueBatch does both with its
// last chunk).
#enqueueChunk(chunk: ArrayBufferView, notify: boolean): void {
// Trust boundary: snapshot metadata via captured getters and validate:
// WebIDL's conversion and spec steps 1-2 come before the state checks.
const info = getNonEmptyUnsharedViewInfo(chunk, 'chunk');
if (!this.#core.canCloseOrEnqueue()) {
throw new TypeError(
'Cannot enqueue a chunk into a stream that is closed or closing'
);
}
// Trust boundary: snapshot metadata via captured getters, validate,
// transfer the backing buffer, normalize to a {buffer, byteOffset,
// byteLength} triple. Queue internals never touch the user's view.
const entry = validateAndTransferView(chunk);
// Transfer the backing buffer and normalize to a {buffer, byteOffset,
// byteLength} triple; queue internals never touch the user's view,
// which is now detached. The transfer is to fixed length (spec
// TransferArrayBuffer), so a source cannot shrink a buffer we hold.
const entry: ByteQueueEntry = {
buffer: ArrayBufferPrototypeTransferToFixedLength(info.buffer),
byteOffset: info.byteOffset,
byteLength: info.byteLength,
};
// EXPECTED-LENGTH CONTRACT: overflow check before the entry reaches
// the queue. (The buffer was already transferred above — a refused
// overflow chunk's buffer is detached. Contract violators lose the
Expand Down Expand Up @@ -2502,8 +2485,9 @@ class ReadableByteStreamController implements ReadableByteStreamControllerType {
this.#bytesDelivered = delivered;
}

// byobRequest.respondWithNewView(view).
#respondWithNewView(view: ArrayBufferView): void {
// byobRequest.respondWithNewView(view), given the metadata the request
// read from the view at its trust boundary (getUnsharedViewInfo).
#respondWithNewView(info: ViewInfo): void {
const released = this.#releasedHead;
const cursor =
released === undefined
Expand All @@ -2513,7 +2497,6 @@ class ReadableByteStreamController implements ReadableByteStreamControllerType {
if (head === undefined) {
throw new TypeError('No pending BYOB request');
}
const info = getViewInfo(view);
const state = getReadableStreamGetState(this.#stream);
if (state === 'closed') {
if (info.byteLength !== 0) {
Expand Down
3 changes: 2 additions & 1 deletion src/tests/streams/readable-byte/AGENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ behavior-parity (messages aside).
| 38 | where `closed` settles relative to a readAtLeast() short of its minimum at a native body's end (#13) | the read fulfills with closed still pending, and the stream closes at the next read | closed first, then the read, as at a JS byte source's close() below min (#27). At a zero-byte end both resolve closed first, for read() and read(view) alike (parity, pinned alongside) | `closedOrderAtNativeEndOfData` |
| 39 | ctor argument conversion order, every member of a byte source (the readable suite's #2 for a default source) | the source first, every member in its own order (type, autoAllocateChunkSize, start, pull, cancel, expectedLength), then the strategy: hwm and its ToNumber, size | the strategy first (spec); then the source's members in lexicographic order (spec: autoAllocateChunkSize, cancel, expectedLength, pull, start, type) | `argumentConversionOrder` |
| 40 | which error wins when two are possible (the readable suite's #32; in both, spec: every source member converts before the strategy and a zero autoAllocateChunkSize are checked, so a non-callable pull, TypeError with different messages, beats an invalid highWaterMark, a size function and a zero autoAllocateChunkSize; a zero autoAllocateChunkSize is checked after an invalid highWaterMark, whose error wins, TypeError in C++ and RangeError in TypeScript) | a non-callable pull also beats a negative autoAllocateChunkSize, which the int conversion accepts and a later range check rejects | a negative autoAllocateChunkSize (TypeError) beats a non-callable pull (spec: it converts first) | `conversionErrorPrecedence` |
| 41 | a bare ArrayBuffer passed to enqueue() or respondWithNewView(), whose WebIDL parameter is an ArrayBufferView (any other non-view there, and every non-view at read(view) and readAtLeast(), is a TypeError in both, messages aside) | taken as a view over the whole buffer (the parameter is a BufferSource): enqueue() detaches it and delivers its bytes; respondWithNewView() accepts one of the request's buffer length, detaches it and delivers its bytes | TypeError 'chunk must be an ArrayBufferView' or 'view must be an ArrayBufferView' (spec); the buffer stays attached, nothing is enqueued, and the pending read is left for a later response | `nonViewArgumentsRejected` |

Parity worth noting (probed, pinned): byte hwm defaults to 0 with NO
automatic pull; pull-throw and error-then-throw identity; enqueue
Expand Down Expand Up @@ -135,7 +136,7 @@ named suite test pins directly, differing only in incidental asserts.
| `release-relock.js` | ledger #9, #10, #32; the WPT releaseLock→second-reader cluster; release with two pending reads or a partially filled head |
| `read-min.js` | ledger #11-#13, #27, #38; byobMin/constraints/readAtLeast (migrated streams-test.js); read(view, options) options as a WebIDL dictionary (parity: null, a function's members, primitives rejected); /chunked SELF endpoint |
| `tee.js` | ledger #7 (on a branch), #14, #24, #25, #29, #32; clone-per-branch; migrated byte-tee pair; error propagation; released branch reads, incl. partially filled ones and tee() after a release; byobRequest held across tee(); tee() of a closed native body locking it |
| `buffer-lifecycle.js` | ledger #18, #30; resizable ArrayBuffers; WASM Memory; SharedArrayBuffer-backed typed arrays and DataViews rejected by read(view), readAtLeast(), enqueue() and respondWithNewView() (TS also after close(), #19) |
| `buffer-lifecycle.js` | ledger #18, #30, #41; resizable ArrayBuffers; WASM Memory; SharedArrayBuffer-backed typed arrays and DataViews rejected by read(view), readAtLeast(), enqueue() and respondWithNewView() (TS also after close(), #19); values that are not views rejected by the same four, and converted before the method steps: a non-view at respondWithNewView() on an invalidated request, or at enqueue() after close() or error(), fails as one, as does a zero-length chunk there (parity) |
| `gc.js` | pending BYOB read + byobRequest survive gc(); both tee branches collected while the controller is held: enqueue() accepted, desiredSize at the high-water mark, byobRequest null, close() then enqueue() as ever (a parity pin of the observable surface — the retention checks are the readable suite's, a transferred buffer leaving nothing to WeakRef), one branch cancelled and the other collected, observed in the gc()'s own job (the readable suite's teeSurvivorBranchCollected, plus byobRequest null), and pull() stops (ledger #26); a byobRequest held across tee() is retired once both branches are collected (ledger #25); a closed, cancelled or errored stream releases its underlying source, and a source with only start() is released while its stream is still open (WeakRef-checked; parity) |
| `integration.js` | BYOB round-trips via SELF; readAtLeast on echoed body; bytes() |
| `js-compat.js` | ledger #17, #22; byte halves of the mixed streams-js-test tests (closed promise, cancel reads, locked ops, globals) |
Expand Down
Loading
Loading