stream: improve respondWithNewView()

daeyeon · danielleadams · commit 2266a4b6d656 · 2022-07-25T21:44:25.000-04:00
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>
diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
@@ -102,6 +102,7 @@ const {
   extractHighWaterMark,
   extractSizeAlgorithm,
   lazyTransfer,
+  isViewedArrayBufferDetached,
   isBrandCheck,
   resetQueue,
   setPromiseHandled,
@@ -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);
   }
 
diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js
@@ -14,6 +14,7 @@ const {
   PromiseReject,
   ReflectGet,
   Symbol,
+  Uint8Array,
 } = primordials;
 
 const {
@@ -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);
@@ -225,6 +244,7 @@ module.exports = {
   lazyTransfer,
   isBrandCheck,
   isPromisePending,
+  isViewedArrayBufferDetached,
   peekQueueValue,
   resetQueue,
   setPromiseHandled,
diff --git a/test/parallel/test-whatwg-readablebytestream-bad-buffers-and-views.js b/test/parallel/test-whatwg-readablebytestream-bad-buffers-and-views.js
@@ -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));
