From e2a00123837a3913971b3634ca39050c4964cd81 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 16 Oct 2026 12:38:07 +0000 Subject: [PATCH 1/6] Pin how byte streams reject arguments that are not views DUP-22 step 1, test only. read(view), readAtLeast(), enqueue() and respondWithNewView() take an ArrayBufferView in WebIDL. No test passed them anything else: read(view) pinned only the error class, and the other two nothing, so a wrong message from their type checks would pass every test. nonViewArgumentsRejected pins a string, an object and undefined at all four, and a bare ArrayBuffer. Every non-view is a TypeError in both implementations, with their own messages, except a bare ArrayBuffer at enqueue() and respondWithNewView(): C++ types those parameters as a BufferSource and takes the buffer as a view over the whole of it, detaching it and delivering its bytes. TypeScript rejects it, as the spec does. Readable-byte ledger #41 records the divergence. --- src/tests/streams/readable-byte/AGENTS.md | 3 +- .../streams/readable-byte/buffer-lifecycle.js | 102 ++++++++++++++++-- src/tests/streams/readable-byte/main.js | 1 + 3 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/tests/streams/readable-byte/AGENTS.md b/src/tests/streams/readable-byte/AGENTS.md index ff97e6e562f..6e2b90e9ada 100644 --- a/src/tests/streams/readable-byte/AGENTS.md +++ b/src/tests/streams/readable-byte/AGENTS.md @@ -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 @@ -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 | | `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) | diff --git a/src/tests/streams/readable-byte/buffer-lifecycle.js b/src/tests/streams/readable-byte/buffer-lifecycle.js index 88fd5229b2f..3e61cf4d1e2 100644 --- a/src/tests/streams/readable-byte/buffer-lifecycle.js +++ b/src/tests/streams/readable-byte/buffer-lifecycle.js @@ -4,12 +4,15 @@ // Buffer ownership hazards at every stage of the BYOB cycle: detached // buffers, foreign buffers in respondWithNewView, resizable -// ArrayBuffers, and non-detachable (WebAssembly.Memory, SharedArrayBuffer) -// buffers. The BEHAVIOR is parity — only messages differ (the WPT -// bad-buffers-and-views and non-transferable-buffers families) — except -// for resizable buffers (ledger #30): TS transfers them to fixed length -// (spec TransferArrayBuffer), while C++ can hand the source a resizable -// byobRequest buffer and return resizable results. +// ArrayBuffers, non-detachable (WebAssembly.Memory, SharedArrayBuffer) +// buffers, and values that are not views. The BEHAVIOR is parity — only +// messages differ (the WPT bad-buffers-and-views and +// non-transferable-buffers families) — except for resizable buffers +// (ledger #30): TS transfers them to fixed length (spec +// TransferArrayBuffer), while C++ can hand the source a resizable +// byobRequest buffer and return resizable results; and for a bare +// ArrayBuffer at enqueue() and respondWithNewView(), which C++ accepts +// (ledger #41). import { deepStrictEqual, strictEqual, throws, rejects } from 'node:assert'; import { usingTsImpl } from 'which-impl'; @@ -291,6 +294,93 @@ export const sharedBuffersRejected = { }, }; +// A value that is not a view is rejected with TypeError by read(view), +// readAtLeast(), enqueue() and respondWithNewView(), whose WebIDL +// parameter is an ArrayBufferView. DIVERGENCE (ledger #41): C++ types the +// chunk of enqueue() and the view of respondWithNewView() as a +// BufferSource, so it takes a bare ArrayBuffer as a view over the whole +// buffer: enqueue() detaches it and delivers its bytes, and +// respondWithNewView() accepts one of the request's buffer length. TS +// rejects it as any other non-view: the buffer stays attached, nothing is +// enqueued, and the pending read is left for a later response. +export const nonViewArgumentsRejected = { + async test() { + for (const value of ['x', {}, undefined, new ArrayBuffer(4)]) { + const { rs } = byteStream(); + const reader = rs.getReader({ mode: 'byob' }); + await rejects(reader.read(value), { + name: 'TypeError', + message: usingTsImpl + ? 'view must be an ArrayBufferView' + : "Failed to execute 'read' on 'ReadableStreamBYOBReader': parameter 1 is not of type 'ArrayBufferView'.", + }); + await rejects(reader.readAtLeast(1, value), { + name: 'TypeError', + message: usingTsImpl + ? 'view must be an ArrayBufferView' + : "Failed to execute 'readAtLeast' on 'ReadableStreamBYOBReader': parameter 2 is not of type 'ArrayBufferView'.", + }); + } + for (const value of ['x', {}, undefined]) { + const { rs, controller } = byteStream(); + throws(() => controller().enqueue(value), { + name: 'TypeError', + message: usingTsImpl + ? 'chunk must be an ArrayBufferView' + : "Failed to execute 'enqueue' on 'ReadableByteStreamController': parameter 1 is not of type 'BufferSource'.", + }); + const reader = rs.getReader({ mode: 'byob' }); + const read = reader.read(new Uint8Array(4)); + await scheduler.wait(5); + throws(() => controller().byobRequest.respondWithNewView(value), { + name: 'TypeError', + message: usingTsImpl + ? 'view must be an ArrayBufferView' + : "Failed to execute 'respondWithNewView' on 'ReadableStreamBYOBRequest': parameter 1 is not of type 'BufferSource'.", + }); + controller().enqueue(new Uint8Array([5])); + deepStrictEqual([...(await read).value], [5]); + } + + { + const { rs, controller } = byteStream(); + const buffer = new Uint8Array([1, 2, 3]).buffer; + if (usingTsImpl) { + throws(() => controller().enqueue(buffer), { + name: 'TypeError', + message: 'chunk must be an ArrayBufferView', + }); + strictEqual(buffer.detached, false); + controller().close(); + strictEqual((await rs.getReader().read()).done, true); + } else { + controller().enqueue(buffer); + strictEqual(buffer.detached, true); + deepStrictEqual([...(await rs.getReader().read()).value], [1, 2, 3]); + } + } + { + const { rs, controller } = byteStream(); + const read = rs.getReader({ mode: 'byob' }).read(new Uint8Array(4)); + await scheduler.wait(5); + const buffer = new Uint8Array([9, 8, 7, 6]).buffer; + if (usingTsImpl) { + throws(() => controller().byobRequest.respondWithNewView(buffer), { + name: 'TypeError', + message: 'view must be an ArrayBufferView', + }); + strictEqual(buffer.detached, false); + controller().enqueue(new Uint8Array([5])); + deepStrictEqual([...(await read).value], [5]); + } else { + controller().byobRequest.respondWithNewView(buffer); + strictEqual(buffer.detached, true); + deepStrictEqual([...(await read).value], [9, 8, 7, 6]); + } + } + }, +}; + // Non-detachable buffers (WebAssembly.Memory) are rejected with // TypeError from both enqueue() and read(view). export const nonDetachableBuffersRejected = { diff --git a/src/tests/streams/readable-byte/main.js b/src/tests/streams/readable-byte/main.js index 9e6ec046136..faabf9c7e66 100644 --- a/src/tests/streams/readable-byte/main.js +++ b/src/tests/streams/readable-byte/main.js @@ -217,6 +217,7 @@ export { resizableByobRequestCannotShrink, resizableBuffersDeliveredFixedLength, sharedBuffersRejected, + nonViewArgumentsRejected, nonDetachableBuffersRejected, } from 'buffer-lifecycle'; From 55ea6c515926fed71899f6b23c58ba4c18f67236 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 16 Oct 2026 12:39:42 +0000 Subject: [PATCH 2/6] Validate byte-stream views through one pair of helpers DUP-22 step 2. read(view) and enqueue() ran the same view checks, twelve lines apart only in the noun, and all three byte-stream trust boundaries (with respondWithNewView()) checked for an ArrayBufferView before calling getUnsharedViewInfo. getUnsharedViewInfo now makes that check itself, which completes WebIDL's ArrayBufferView conversion in one place, and the new getNonEmptyUnsharedViewInfo adds the non-empty checks that read(view) and enqueue() share. The messages are unchanged. The comment on validateAndTransferView named respondWithNewView(), which has never called it; it now names enqueue() alone. --- src/per_isolate/webstreams/readable.ts | 61 ++++++++++++-------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 9c00b7daeb7..974e72be870 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -1134,18 +1134,7 @@ class ReadableStreamBYOBReader implements ReadableStreamBYOBReaderType { options: ReadableStreamBYOBReaderReadOptions ): Promise> { // --- 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' @@ -1344,10 +1333,14 @@ function getViewInfo(view: ArrayBufferView): ViewInfo { }; } -// getViewInfo() for the byte-stream trust boundaries, which reject -// SharedArrayBuffer-backed views (WebIDL ArrayBufferView without -// [AllowShared]). -function getUnsharedViewInfo(view: ArrayBufferView, what: string): ViewInfo { +// getViewInfo() for the byte-stream trust boundaries (read(view), +// enqueue(), respondWithNewView()): 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. +function getUnsharedViewInfo(view: unknown, what: string): ViewInfo { + if (!isArrayBufferView(view)) { + throw new TypeError(`${what} must be an ArrayBufferView`); + } const info = getViewInfo(view); if (isSharedArrayBuffer(info.buffer)) { throw new TypeError(`${what} must not be backed by a SharedArrayBuffer`); @@ -1355,26 +1348,29 @@ function getUnsharedViewInfo(view: ArrayBufferView, what: string): ViewInfo { 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, or a view over a +// zero-length buffer, throws. +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" + `${what}'s backing buffer must have a non-zero byteLength` ); } + return info; +} + +// A chunk enqueued into a byte stream, validated at the enqueue() trust +// boundary and normalized: 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 { + const info = getNonEmptyUnsharedViewInfo(view, 'chunk'); return { buffer: ArrayBufferPrototypeTransferToFixedLength(info.buffer), byteOffset: info.byteOffset, @@ -2043,11 +2039,8 @@ class ReadableStreamBYOBRequest implements ReadableStreamBYOBRequestType { 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'); + // Spec step 2: detached buffers are TypeError, not RangeError. if (ArrayBufferPrototypeDetachedGet(info.buffer)) { throw new TypeError("The view's buffer has been detached"); } From 1b63f774f66455533256f34a5e242ae8740627e8 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 16 Oct 2026 12:40:22 +0000 Subject: [PATCH 3/6] Read a respondWithNewView() view's metadata once DUP-22 step 3. byobRequest.respondWithNewView() read the view's metadata through getUnsharedViewInfo for its own checks, then handed the controller the view, which read it again through getViewInfo. No code runs in between, so the second read could only repeat the first. The controller now takes the ViewInfo the request read, which saves the read and its allocation on every call. Nothing observable changes. --- src/per_isolate/webstreams/readable.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 974e72be870..2cc109aadf6 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -1954,7 +1954,7 @@ let byteControllerRespond: ( ) => void; let byteControllerRespondWithNewView: ( controller: ReadableByteStreamController, - view: ArrayBufferView + info: ViewInfo ) => void; // The queued byte controller's autoAllocateChunkSize; undefined for any // other controller. @@ -2044,7 +2044,7 @@ class ReadableStreamBYOBRequest implements ReadableStreamBYOBRequestType { if (ArrayBufferPrototypeDetachedGet(info.buffer)) { throw new TypeError("The view's buffer has been detached"); } - byteControllerRespondWithNewView(this.#controller, view); + byteControllerRespondWithNewView(this.#controller, info); } } @@ -2121,8 +2121,8 @@ class ReadableByteStreamController implements ReadableByteStreamControllerType { controller.#respond(bytesWritten); }; - byteControllerRespondWithNewView = (controller, view) => { - controller.#respondWithNewView(view); + byteControllerRespondWithNewView = (controller, info) => { + controller.#respondWithNewView(info); }; getByteControllerAutoAllocateChunkSize = (controller) => { @@ -2495,8 +2495,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 @@ -2506,7 +2507,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) { From 94bd1750912222370616660e7c282de5dc50b9a9 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 16 Oct 2026 12:41:13 +0000 Subject: [PATCH 4/6] Drop the unreachable buffer-length check from byte-view validation DUP-22 step 4. read(view) and enqueue() checked, after a view's own byteLength, that its buffer's byteLength was not zero, as the spec's second step does. That check could never fail: the byteLength getters read 0 for a detached or out-of-bounds typed array, a DataView's throw, and a view with bytes lies within its buffer. The spec needs the step because a view's [[ByteLength]] slot keeps its value once its buffer is detached. A probe of seventeen kinds of input (non-views, shared, zero-length, detached, out-of-bounds and emptied length-tracking views) at all five entry points never reached it. The check goes, with its message, and a comment says why the spec's step needs no code. This saves a getter call per enqueue() and read(view). Nothing observable changes. --- src/per_isolate/webstreams/readable.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 2cc109aadf6..4dd95cedff3 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -1349,18 +1349,17 @@ function getUnsharedViewInfo(view: unknown, what: string): ViewInfo { } // getUnsharedViewInfo() for a view that must hold bytes (spec read(view) -// steps 1-3, enqueue() steps 1-2): a zero-length view, or a view over a -// zero-length buffer, throws. +// 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(`${what} must have a non-zero byteLength`); } - if (ArrayBufferPrototypeByteLengthGet(info.buffer) === 0) { - throw new TypeError( - `${what}'s backing buffer must have a non-zero byteLength` - ); - } return info; } From fc7687d49f17b1d448523c60da522ba5ace7677e Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 16 Oct 2026 19:03:41 +0000 Subject: [PATCH 5/6] Fold getViewInfo into getUnsharedViewInfo DUP-22 step 5. Since respondWithNewView()'s controller takes the ViewInfo its request read, getViewInfo has had one caller, getUnsharedViewInfo, which makes the isArrayBufferView check that getViewInfo's precondition asked of its callers. getViewInfo folds into it, and the precondition goes. The getters run in the same order. Nothing observable changes. --- src/per_isolate/webstreams/readable.ts | 50 ++++++++++++-------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 4dd95cedff3..673d92f80ac 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -1302,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), @@ -1320,28 +1326,18 @@ 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 (read(view), -// enqueue(), respondWithNewView()): 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. -function getUnsharedViewInfo(view: unknown, what: string): ViewInfo { - if (!isArrayBufferView(view)) { - throw new TypeError(`${what} must be an ArrayBufferView`); - } - const info = getViewInfo(view); if (isSharedArrayBuffer(info.buffer)) { throw new TypeError(`${what} must not be backed by a SharedArrayBuffer`); } From c556a530303a8aa432cdecfccfd965a9c7c5b635 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 16 Oct 2026 19:06:52 +0000 Subject: [PATCH 6/6] Convert byte-stream views before the method steps DUP-22 step 6. WebIDL converts a method's arguments before its steps run, and C++ converts its parameters before the method body. respondWithNewView() checked that its request was still valid, and enqueue() that its stream could still take a chunk, before converting the view, so a value that was not a view met those checks first: respondWithNewView('x') on an invalidated request threw 'This BYOB request has been invalidated', and enqueue('x') after close() 'Cannot enqueue a chunk into a stream that is closed or closing', where C++ threw its parameter-type error. Both now convert the view first. enqueue() also checks that the chunk is not empty (spec steps 1-2) before the stream's state (steps 3-4), as C++ does, and transfers the buffer only once both pass, so validateAndTransferView, which did both at once, goes. A view backed by a SharedArrayBuffer, which the conversion rejects, now fails as such on an invalidated request too, where C++, which converts it, still reports the invalidated request. Every error is still a TypeError; only which message wins changes. viewConvertedBeforeMethodSteps pins the new order on both implementations, and fails under TypeScript without the change. --- src/per_isolate/webstreams/readable.ts | 33 +++++++-------- src/tests/streams/readable-byte/AGENTS.md | 2 +- .../streams/readable-byte/buffer-lifecycle.js | 40 +++++++++++++++++++ src/tests/streams/readable-byte/main.js | 1 + 4 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 673d92f80ac..dd631f6e9c1 100644 --- a/src/per_isolate/webstreams/readable.ts +++ b/src/per_isolate/webstreams/readable.ts @@ -1359,20 +1359,6 @@ function getNonEmptyUnsharedViewInfo(view: unknown, what: string): ViewInfo { return info; } -// A chunk enqueued into a byte stream, validated at the enqueue() trust -// boundary and normalized: 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 { - const info = getNonEmptyUnsharedViewInfo(view, 'chunk'); - return { - buffer: ArrayBufferPrototypeTransferToFixedLength(info.buffer), - byteOffset: info.byteOffset, - byteLength: info.byteLength, - }; -} - interface SourceAlgorithms { pull: (() => Promise) | undefined; cancel: ((reason: unknown) => Promise) | undefined; @@ -2031,10 +2017,11 @@ 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'); } - const info = getUnsharedViewInfo(view, 'view'); // Spec step 2: detached buffers are TypeError, not RangeError. if (ArrayBufferPrototypeDetachedGet(info.buffer)) { throw new TypeError("The view's buffer has been detached"); @@ -2278,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 diff --git a/src/tests/streams/readable-byte/AGENTS.md b/src/tests/streams/readable-byte/AGENTS.md index 6e2b90e9ada..cacdb6c955a 100644 --- a/src/tests/streams/readable-byte/AGENTS.md +++ b/src/tests/streams/readable-byte/AGENTS.md @@ -136,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, #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 | +| `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) | diff --git a/src/tests/streams/readable-byte/buffer-lifecycle.js b/src/tests/streams/readable-byte/buffer-lifecycle.js index 3e61cf4d1e2..10f62f0dd22 100644 --- a/src/tests/streams/readable-byte/buffer-lifecycle.js +++ b/src/tests/streams/readable-byte/buffer-lifecycle.js @@ -381,6 +381,46 @@ export const nonViewArgumentsRejected = { }, }; +// WebIDL converts a method's arguments before its steps run, so a value +// that is not a view fails as such at respondWithNewView() on an +// invalidated request, and at enqueue() once the stream has closed or +// errored; so does a zero-length chunk, which enqueue() checks (spec steps +// 1-2) before the stream's state. Parity, messages aside. +export const viewConvertedBeforeMethodSteps = { + async test() { + const { rs, controller } = byteStream(); + const read = rs.getReader({ mode: 'byob' }).read(new Uint8Array(4)); + await scheduler.wait(5); + const request = controller().byobRequest; + request.view[0] = 1; + request.respond(1); + await read; + throws(() => request.respondWithNewView('x'), { + name: 'TypeError', + message: usingTsImpl + ? 'view must be an ArrayBufferView' + : "Failed to execute 'respondWithNewView' on 'ReadableStreamBYOBRequest': parameter 1 is not of type 'BufferSource'.", + }); + + for (const end of [(c) => c.close(), (c) => c.error(new Error('boom'))]) { + const { controller } = byteStream(); + end(controller()); + throws(() => controller().enqueue('x'), { + name: 'TypeError', + message: usingTsImpl + ? 'chunk must be an ArrayBufferView' + : "Failed to execute 'enqueue' on 'ReadableByteStreamController': parameter 1 is not of type 'BufferSource'.", + }); + throws(() => controller().enqueue(new Uint8Array(0)), { + name: 'TypeError', + message: usingTsImpl + ? 'chunk must have a non-zero byteLength' + : 'Cannot enqueue a zero-length ArrayBuffer.', + }); + } + }, +}; + // Non-detachable buffers (WebAssembly.Memory) are rejected with // TypeError from both enqueue() and read(view). export const nonDetachableBuffersRejected = { diff --git a/src/tests/streams/readable-byte/main.js b/src/tests/streams/readable-byte/main.js index faabf9c7e66..02981942bef 100644 --- a/src/tests/streams/readable-byte/main.js +++ b/src/tests/streams/readable-byte/main.js @@ -218,6 +218,7 @@ export { resizableBuffersDeliveredFixedLength, sharedBuffersRejected, nonViewArgumentsRejected, + viewConvertedBeforeMethodSteps, nonDetachableBuffersRejected, } from 'buffer-lifecycle';