stream: fix isDetachedBuffer validations

Signed-off-by: Daeyeon Jeong [email protected] PR-URL: #44114 Refs: #43866 Reviewed-By: LiviaMedeiros <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
nodejs · Aug 22, 2022 · a3ac445 · a3ac445
1 parent bce8041
commit a3ac445
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 21 deletions.
diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
@@ -59,6 +59,7 @@ const {
 } = require('v8');
 
 const {
+  validateBuffer,
   validateObject,
 } = require('internal/validators');
 
@@ -101,6 +102,7 @@ const {
   extractHighWaterMark,
   extractSizeAlgorithm,
   lazyTransfer,
+  isDetachedBuffer,
   isViewedArrayBufferDetached,
   isBrandCheck,
   resetQueue,
@@ -658,11 +660,13 @@ class ReadableStreamBYOBRequest {
     const viewBuffer = ArrayBufferViewGetBuffer(view);
     const viewBufferByteLength = ArrayBufferGetByteLength(viewBuffer);
 
-    if (viewByteLength === 0 || viewBufferByteLength === 0) {
-      throw new ERR_INVALID_STATE.TypeError(
-        'View ArrayBuffer is zero-length or detached');
+    if (isDetachedBuffer(viewBuffer)) {
+      throw new ERR_INVALID_STATE.TypeError('Viewed ArrayBuffer is detached');
     }
 
+    assert(viewByteLength > 0);
+    assert(viewBufferByteLength > 0);
+
     readableByteStreamControllerRespond(controller, bytesWritten);
   }
 
@@ -681,6 +685,8 @@ class ReadableStreamBYOBRequest {
         'This BYOB request has been invalidated');
     }
 
+    validateBuffer(view, 'view');
+
     if (isViewedArrayBufferDetached(view)) {
       throw new ERR_INVALID_STATE.TypeError('Viewed ArrayBuffer is detached');
     }
@@ -894,15 +900,19 @@ class ReadableStreamBYOBReader {
           ],
           view));
     }
+
     const viewByteLength = ArrayBufferViewGetByteLength(view);
     const viewBuffer = ArrayBufferViewGetBuffer(view);
     const viewBufferByteLength = ArrayBufferGetByteLength(viewBuffer);
 
     if (viewByteLength === 0 || viewBufferByteLength === 0) {
       return PromiseReject(
         new ERR_INVALID_STATE.TypeError(
-          'View ArrayBuffer is zero-length or detached'));
+          'View or Viewed ArrayBuffer is zero-length or detached',
+        ),
+      );
     }
+
     // Supposed to assert here that the view's buffer is not
     // detached, but there's no API available to use to check that.
     if (this[kState].stream === undefined) {
@@ -2298,11 +2308,10 @@ function readableByteStreamControllerEnqueue(
   if (pendingPullIntos.length) {
     const firstPendingPullInto = pendingPullIntos[0];
 
-    const pendingBufferByteLength =
-      ArrayBufferGetByteLength(firstPendingPullInto.buffer);
-    if (pendingBufferByteLength === 0) {
+    if (isDetachedBuffer(firstPendingPullInto.buffer)) {
       throw new ERR_INVALID_STATE.TypeError(
-        'Destination ArrayBuffer is zero-length or detached');
+        'Destination ArrayBuffer is detached',
+      );
     }
 
     firstPendingPullInto.buffer =
@@ -2501,16 +2510,6 @@ function readableByteStreamControllerRespondWithNewView(controller, view) {
   const desc = pendingPullIntos[0];
   assert(stream[kState].state !== 'errored');
 
-  if (!isArrayBufferView(view)) {
-    throw new ERR_INVALID_ARG_TYPE(
-      'view',
-      [
-        'Buffer',
-        'TypedArray',
-        'DataView',
-      ],
-      view);
-  }
   const viewByteLength = ArrayBufferViewGetByteLength(view);
   const viewByteOffset = ArrayBufferViewGetByteOffset(view);
   const viewBuffer = ArrayBufferViewGetBuffer(view);

diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js
@@ -129,11 +129,13 @@ function transferArrayBuffer(buffer) {
   return res;
 }
 
-function isArrayBufferDetached(buffer) {
+function isDetachedBuffer(buffer) {
   if (ArrayBufferGetByteLength(buffer) === 0) {
+    // TODO(daeyeon): Consider using C++ builtin to improve performance.
     try {
       new Uint8Array(buffer);
-    } catch {
+    } catch (error) {
+      assert(error.name === 'TypeError');
       return true;
     }
   }
@@ -143,7 +145,7 @@ function isArrayBufferDetached(buffer) {
 function isViewedArrayBufferDetached(view) {
   return (
     ArrayBufferViewGetByteLength(view) === 0 &&
-    isArrayBufferDetached(ArrayBufferViewGetBuffer(view))
+    isDetachedBuffer(ArrayBufferViewGetBuffer(view))
   );
 }
 
@@ -243,6 +245,7 @@ module.exports = {
   extractSizeAlgorithm,
   lazyTransfer,
   isBrandCheck,
+  isDetachedBuffer,
   isPromisePending,
   isViewedArrayBufferDetached,
   peekQueueValue,

diff --git a/test/parallel/test-whatwg-readablebytestream-bad-buffers-and-views.js b/test/parallel/test-whatwg-readablebytestream-bad-buffers-and-views.js
@@ -51,4 +51,38 @@ let pass = 0;
   reader.read(new Uint8Array([4, 5, 6]));
 }
 
+{
+  const stream = new ReadableStream({
+    start(c) {
+      c.enqueue(new Uint8Array([1, 2, 3]));
+    },
+    type: 'bytes',
+  });
+  const reader = stream.getReader({ mode: 'byob' });
+  const view = new Uint8Array();
+  assert
+    .rejects(reader.read(view), {
+      code: 'ERR_INVALID_STATE',
+      name: 'TypeError',
+    })
+    .then(common.mustCall());
+}
+
+{
+  const stream = new ReadableStream({
+    start(c) {
+      c.enqueue(new Uint8Array([1, 2, 3]));
+    },
+    type: 'bytes',
+  });
+  const reader = stream.getReader({ mode: 'byob' });
+  const view = new Uint8Array(new ArrayBuffer(10), 0, 0);
+  assert
+    .rejects(reader.read(view), {
+      code: 'ERR_INVALID_STATE',
+      name: 'TypeError',
+    })
+    .then(common.mustCall());
+}
+
 process.on('exit', () => assert.strictEqual(pass, 2));