From 2266a4b6d656d53dc44933b8a3c755b7a48d97cc Mon Sep 17 00:00:00 2001
From: Daeyeon Jeong <daeyeon.dev@gmail.com>
Date: Mon, 25 Jul 2022 01:56:48 +0900
Subject: [PATCH] stream: improve `respondWithNewView()`

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: https://github.com/nodejs/node/pull/43866
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 lib/internal/webstreams/readablestream.js     |  5 ++
 lib/internal/webstreams/util.js               | 20 +++++++
 ...eadablebytestream-bad-buffers-and-views.js | 54 +++++++++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 test/parallel/test-whatwg-readablebytestream-bad-buffers-and-views.js

diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js
index 0e7d33e9b71eb3..724710fdb1749d 100644
--- 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
index 524093dc74df66..ed74a10801f8ff 100644
--- 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
new file mode 100644
index 00000000000000..545a0cd2db5128
--- /dev/null
+++ 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));