From 9c681e0d13a4f6e50cfb0507962ea63b7b2af25f Mon Sep 17 00:00:00 2001
From: Daeyeon Jeong <daeyeon.dev@gmail.com>
Date: Sun, 17 Jul 2022 02:44:24 +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
---
 lib/internal/webstreams/readablestream.js     | 12 +++++
 src/node_util.cc                              |  8 ++++
 ...eadablebytestream-bad-buffers-and-views.js | 46 +++++++++++++++++++
 .../test-whatwg-readablebytestream.js         |  4 +-
 4 files changed, 67 insertions(+), 3 deletions(-)
 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..1d96470269154c 100644
--- a/lib/internal/webstreams/readablestream.js
+++ b/lib/internal/webstreams/readablestream.js
@@ -28,6 +28,8 @@ const {
   Uint8Array,
 } = primordials;
 
+const { arrayBufferIsDetached } = internalBinding('util');
+
 const {
   codes: {
     ERR_ILLEGAL_CONSTRUCTOR,
@@ -681,6 +683,16 @@ class ReadableStreamBYOBRequest {
         'This BYOB request has been invalidated');
     }
 
+    const viewedBuffer = ArrayBufferViewGetBuffer(view);
+    const viewedBufferByteLength = ArrayBufferGetByteLength(viewedBuffer);
+    const isDetached = arrayBufferIsDetached(viewedBuffer);
+
+    if (viewedBufferByteLength === 0 && isDetached) {
+      throw new ERR_INVALID_STATE.TypeError(
+        'Viewed ArrayBuffer is detached',
+      );
+    }
+
     readableByteStreamControllerRespondWithNewView(controller, view);
   }
 
diff --git a/src/node_util.cc b/src/node_util.cc
index 5b5dab36f08fbf..c979f5ea2ca882 100644
--- a/src/node_util.cc
+++ b/src/node_util.cc
@@ -207,6 +207,12 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(args[0].As<ArrayBufferView>()->HasBuffer());
 }
 
+void ArrayBufferIsDetached(const FunctionCallbackInfo<Value>& args) {
+  CHECK(args[0]->IsArrayBuffer());
+  v8::Local<v8::ArrayBuffer> ab = args[0].As<v8::ArrayBuffer>();
+  args.GetReturnValue().Set(ab->GetBackingStore()->Data() == nullptr);
+}
+
 class WeakReference : public BaseObject {
  public:
   WeakReference(Environment* env, Local<Object> object, Local<Object> target)
@@ -334,6 +340,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(GetExternalValue);
   registry->Register(Sleep);
   registry->Register(ArrayBufferViewHasBuffer);
+  registry->Register(ArrayBufferIsDetached);
   registry->Register(WeakReference::New);
   registry->Register(WeakReference::Get);
   registry->Register(WeakReference::IncRef);
@@ -380,6 +387,7 @@ void Initialize(Local<Object> target,
   env->SetMethod(target, "sleep", Sleep);
 
   env->SetMethod(target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
+  env->SetMethod(target, "arrayBufferIsDetached", ArrayBufferIsDetached);
   Local<Object> constants = Object::New(env->isolate());
   NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
   NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);
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..056aaa3eb85344
--- /dev/null
+++ b/test/parallel/test-whatwg-readablebytestream-bad-buffers-and-views.js
@@ -0,0 +1,46 @@
+'use strict';
+
+const common = require('../common');
+const assert = require('node:assert');
+
+{
+  // 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);
+      }, RangeError);
+    }),
+    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);
+      }, TypeError);
+    }),
+    type: 'bytes',
+  });
+
+  const reader = stream.getReader({ mode: 'byob' });
+  reader.read(new Uint8Array([4, 5, 6]));
+}
diff --git a/test/parallel/test-whatwg-readablebytestream.js b/test/parallel/test-whatwg-readablebytestream.js
index 6b305a0d8365f2..45b6a34d7af631 100644
--- a/test/parallel/test-whatwg-readablebytestream.js
+++ b/test/parallel/test-whatwg-readablebytestream.js
@@ -98,9 +98,7 @@ class Source {
       this.controller.close();
     }
 
-    assert.throws(() => byobRequest.respondWithNewView({}), {
-      code: 'ERR_INVALID_ARG_TYPE',
-    });
+    assert.throws(() => byobRequest.respondWithNewView({}), TypeError);
 
     byobRequest.respond(bytesRead);