Skip to content

Commit

Permalink
stream: improve respondWithNewView()
Browse files Browse the repository at this point in the history
This fixes validating an ArrayBufferView given to
ReadableStreamBYOBRequest.respondWithNewView() to improve the web
streams compatibility.

Signed-off-by: Daeyeon Jeong daeyeon.dev@gmail.com

PR-URL: #43866
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
daeyeon committed Jul 24, 2022
1 parent 3b4c0e4 commit 0c27ca4
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 0 deletions.
5 changes: 5 additions & 0 deletions lib/internal/webstreams/readablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ const {
extractHighWaterMark,
extractSizeAlgorithm,
lazyTransfer,
isViewedArrayBufferDetached,
isBrandCheck,
resetQueue,
setPromiseHandled,
Expand Down Expand Up @@ -681,6 +682,10 @@ class ReadableStreamBYOBRequest {
'This BYOB request has been invalidated');
}

if (isViewedArrayBufferDetached(view)) {
throw new ERR_INVALID_STATE.TypeError('Viewed ArrayBuffer is detached');
}

readableByteStreamControllerRespondWithNewView(controller, view);
}

Expand Down
20 changes: 20 additions & 0 deletions lib/internal/webstreams/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const {
PromiseReject,
ReflectGet,
Symbol,
Uint8Array,
} = primordials;

const {
Expand Down Expand Up @@ -128,6 +129,24 @@ function transferArrayBuffer(buffer) {
return res;
}

function isArrayBufferDetached(buffer) {
if (ArrayBufferGetByteLength(buffer) === 0) {
try {
new Uint8Array(buffer);
} catch {
return true;
}
}
return false;
}

function isViewedArrayBufferDetached(view) {
return (
ArrayBufferViewGetByteLength(view) === 0 &&
isArrayBufferDetached(ArrayBufferViewGetBuffer(view))
);
}

function dequeueValue(controller) {
assert(controller[kState].queue !== undefined);
assert(controller[kState].queueTotalSize !== undefined);
Expand Down Expand Up @@ -225,6 +244,7 @@ module.exports = {
lazyTransfer,
isBrandCheck,
isPromisePending,
isViewedArrayBufferDetached,
peekQueueValue,
resetQueue,
setPromiseHandled,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
'use strict';

const common = require('../common');
const assert = require('node:assert');

let pass = 0;

{
// ReadableStream with byte source: respondWithNewView() throws if the
// supplied view's buffer has a different length (in the closed state)
const stream = new ReadableStream({
pull: common.mustCall(async (c) => {
const view = new Uint8Array(new ArrayBuffer(10), 0, 0);

c.close();

assert.throws(() => c.byobRequest.respondWithNewView(view), {
code: 'ERR_INVALID_ARG_VALUE',
name: 'RangeError',
});
pass++;
}),
type: 'bytes',
});

const reader = stream.getReader({ mode: 'byob' });
reader.read(new Uint8Array([4, 5, 6]));
}

{
// ReadableStream with byte source: respondWithNewView() throws if the
// supplied view's buffer has been detached (in the closed state)
const stream = new ReadableStream({
pull: common.mustCall((c) => {
c.close();

// Detach it by reading into it
const view = new Uint8Array([1, 2, 3]);
reader.read(view);

assert.throws(() => c.byobRequest.respondWithNewView(view), {
code: 'ERR_INVALID_STATE',
name: 'TypeError',
});
pass++;
}),
type: 'bytes',
});

const reader = stream.getReader({ mode: 'byob' });
reader.read(new Uint8Array([4, 5, 6]));
}

process.on('exit', () => assert.strictEqual(pass, 2));

0 comments on commit 0c27ca4

Please sign in to comment.