diff --git a/src/per_isolate/webstreams/readable.ts b/src/per_isolate/webstreams/readable.ts index 9c00b7daeb7..dd631f6e9c1 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' @@ -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), @@ -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 { @@ -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. @@ -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); } } @@ -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) => { @@ -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 @@ -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 @@ -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) { diff --git a/src/tests/streams/readable-byte/AGENTS.md b/src/tests/streams/readable-byte/AGENTS.md index ff97e6e562f..cacdb6c955a 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, 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 88fd5229b2f..10f62f0dd22 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,133 @@ 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]); + } + } + }, +}; + +// 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 9e6ec046136..02981942bef 100644 --- a/src/tests/streams/readable-byte/main.js +++ b/src/tests/streams/readable-byte/main.js @@ -217,6 +217,8 @@ export { resizableByobRequestCannotShrink, resizableBuffersDeliveredFixedLength, sharedBuffersRejected, + nonViewArgumentsRejected, + viewConvertedBeforeMethodSteps, nonDetachableBuffersRejected, } from 'buffer-lifecycle';