stream: move byte-stream helpers to util binding

Signed-off-by: Matteo Collina <hello@matteocollina.com>

Author: mcollina

lib/internal/webstreams/readablestream.js

@@ -690,10 +690,9 @@ class ReadableStreamBYOBRequest {
         'This BYOB request has been invalidated');
     }
 
-    const {
-      buffer: viewBuffer,
-      byteLength: viewByteLength,
-    } = getArrayBufferView(view);
+    const arrayBufferView = getArrayBufferView(view);
+    const viewBuffer = arrayBufferView[0];
+    const viewByteLength = arrayBufferView[2];
     const viewBufferByteLength = ArrayBufferPrototypeGetByteLength(viewBuffer);
 
     if (ArrayBufferPrototypeGetDetached(viewBuffer)) {
@@ -984,10 +983,9 @@ class ReadableStreamBYOBReader {
     }
     validateObject(options, 'options', kValidateObjectAllowObjectsAndNull);
 
-    const {
-      buffer: viewBuffer,
-      byteLength: viewByteLength,
-    } = getArrayBufferView(view);
+    const arrayBufferView = getArrayBufferView(view);
+    const viewBuffer = arrayBufferView[0];
+    const viewByteLength = arrayBufferView[2];
 
     if (isSharedArrayBuffer(viewBuffer)) {
       throw new ERR_INVALID_ARG_VALUE(
@@ -1204,10 +1202,9 @@ class ReadableByteStreamController {
     if (!isReadableByteStreamController(this))
       throw new ERR_INVALID_THIS('ReadableByteStreamController');
     validateBuffer(chunk);
-    const {
-      buffer: chunkBuffer,
-      byteLength: chunkByteLength,
-    } = getArrayBufferView(chunk);
+    const arrayBufferView = getArrayBufferView(chunk);
+    const chunkBuffer = arrayBufferView[0];
+    const chunkByteLength = arrayBufferView[2];
 
     if (isSharedArrayBuffer(chunkBuffer)) {
       throw new ERR_INVALID_ARG_VALUE(
@@ -2753,7 +2750,10 @@ function readableByteStreamControllerPullInto(
   assert(minimumFill >= elementSize && minimumFill <= view.byteLength);
   assert(minimumFill % elementSize === 0);
 
-  const { buffer, byteOffset, byteLength } = getArrayBufferView(view);
+  const arrayBufferView = getArrayBufferView(view);
+  const buffer = arrayBufferView[0];
+  const byteOffset = arrayBufferView[1];
+  const byteLength = arrayBufferView[2];
   const bufferByteLength = ArrayBufferPrototypeGetByteLength(buffer);
 
   let transferredBuffer;
@@ -2894,7 +2894,10 @@ function readableByteStreamControllerEnqueue(controller, chunk) {
     stream,
   } = controller[kState];
 
-  const { buffer, byteOffset, byteLength } = getArrayBufferView(chunk);
+  const arrayBufferView = getArrayBufferView(chunk);
+  const buffer = arrayBufferView[0];
+  const byteOffset = arrayBufferView[1];
+  const byteLength = arrayBufferView[2];
 
   if (closeRequested || stream[kState].state !== 'readable')
     return;
@@ -3187,11 +3190,10 @@ function readableByteStreamControllerRespondWithNewView(controller, view) {
   const desc = pendingPullIntos[0];
   assert(stream[kState].state !== 'errored');
 
-  const {
-    buffer: viewBuffer,
-    byteOffset: viewByteOffset,
-    byteLength: viewByteLength,
-  } = getArrayBufferView(view);
+  const arrayBufferView = getArrayBufferView(view);
+  const viewBuffer = arrayBufferView[0];
+  const viewByteOffset = arrayBufferView[1];
+  const viewByteLength = arrayBufferView[2];
   const viewBufferByteLength = ArrayBufferPrototypeGetByteLength(viewBuffer);
 
   if (stream[kState].state === 'closed') {

lib/internal/webstreams/util.js

@@ -24,20 +24,17 @@ const {
 const {
   canCopyArrayBuffer,
   cloneAsUint8Array,
-  getArrayBufferView,
-} = internalBinding('webstreams');
-
-const {
-  inspect,
-} = require('util');
-
-const {
   constants: {
     kPending,
   },
+  getArrayBufferView,
   getPromiseDetails,
 } = internalBinding('util');
 
+const {
+  inspect,
+} = require('util');
+
 const assert = require('internal/assert');
 
 const {
@@ -89,10 +86,10 @@ function customInspect(depth, options, name, data) {
 }
 
 // getArrayBufferView, canCopyArrayBuffer, and cloneAsUint8Array are
-// implemented in src/node_webstreams.cc via direct V8 API calls. They are
-// immune to user tampering of typed-array prototypes (matching the defensive
-// behavior of the previous Reflect.get-based JS implementation) and faster on
-// hot byte-stream paths.
+// implemented in src/node_util.cc via direct V8 API calls. They are immune to
+// user tampering of typed-array prototypes (matching the defensive behavior of
+// the previous Reflect.get-based JS implementation) and faster on hot
+// byte-stream paths.
 
 function isBrandCheck(brand) {
   return (value) => {

node.gyp

@@ -169,7 +169,6 @@
       'src/node_types.cc',
       'src/node_url.cc',
       'src/node_url_pattern.cc',
-      'src/node_webstreams.cc',
       'src/node_util.cc',
       'src/node_v8.cc',
       'src/node_wasi.cc',

src/node_binding.cc

@@ -96,7 +96,6 @@
   V(v8)                                                                        \
   V(wasi)                                                                      \
   V(wasm_web_api)                                                              \
-  V(webstreams)                                                                \
   V(watchdog)                                                                  \
   V(worker)                                                                    \
   V(zlib)

src/node_buffer.cc

@@ -1524,6 +1524,16 @@ static void SetDetachKey(const FunctionCallbackInfo<Value>& args) {
 
 namespace {
 
+bool ReadNonNegativeInteger(Local<Value> value, uint64_t* result) {
+  constexpr double kMaxSafeInteger = 9007199254740991.0;
+  double number = value.As<Number>()->Value();
+  if (number < 0 || number > kMaxSafeInteger) {
+    return false;
+  }
+  *result = static_cast<uint64_t>(number);
+  return static_cast<double>(*result) == number;
+}
+
 std::pair<void*, size_t> DecomposeBufferToParts(Local<Value> buffer) {
   void* pointer;
   size_t byte_length;
@@ -1551,10 +1561,10 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   // args[4] == bytesToCopy
 
   CHECK(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer());
-  CHECK(args[1]->IsUint32());
+  CHECK(args[1]->IsNumber());
   CHECK(args[2]->IsArrayBuffer() || args[2]->IsSharedArrayBuffer());
-  CHECK(args[3]->IsUint32());
-  CHECK(args[4]->IsUint32());
+  CHECK(args[3]->IsNumber());
+  CHECK(args[4]->IsNumber());
 
   void* destination;
   size_t destination_byte_length;
@@ -1565,16 +1575,29 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   size_t source_byte_length;
   std::tie(source, source_byte_length) = DecomposeBufferToParts(args[2]);
 
-  uint32_t destination_offset = args[1].As<Uint32>()->Value();
-  uint32_t source_offset = args[3].As<Uint32>()->Value();
-  size_t bytes_to_copy = args[4].As<Uint32>()->Value();
-
-  CHECK_GE(destination_byte_length - destination_offset, bytes_to_copy);
-  CHECK_GE(source_byte_length - source_offset, bytes_to_copy);
-
-  uint8_t* dest = static_cast<uint8_t*>(destination) + destination_offset;
-  uint8_t* src = static_cast<uint8_t*>(source) + source_offset;
-  memcpy(dest, src, bytes_to_copy);
+  uint64_t destination_offset;
+  uint64_t source_offset;
+  uint64_t bytes_to_copy;
+  CHECK(ReadNonNegativeInteger(args[1], &destination_offset));
+  CHECK(ReadNonNegativeInteger(args[3], &source_offset));
+  CHECK(ReadNonNegativeInteger(args[4], &bytes_to_copy));
+
+  const uint64_t destination_offset_u = destination_offset;
+  const uint64_t source_offset_u = source_offset;
+  const uint64_t bytes_to_copy_u = bytes_to_copy;
+  const uint64_t destination_byte_length_u = destination_byte_length;
+  const uint64_t source_byte_length_u = source_byte_length;
+  CHECK_LE(destination_offset_u, destination_byte_length_u);
+  CHECK_LE(source_offset_u, source_byte_length_u);
+  CHECK_LE(bytes_to_copy_u, destination_byte_length_u - destination_offset_u);
+  CHECK_LE(bytes_to_copy_u, source_byte_length_u - source_offset_u);
+
+  const size_t destination_offset_s = static_cast<size_t>(destination_offset_u);
+  const size_t source_offset_s = static_cast<size_t>(source_offset_u);
+  const size_t bytes_to_copy_s = static_cast<size_t>(bytes_to_copy_u);
+  uint8_t* dest = static_cast<uint8_t*>(destination) + destination_offset_s;
+  uint8_t* src = static_cast<uint8_t*>(source) + source_offset_s;
+  memcpy(dest, src, bytes_to_copy_s);
 }
 
 // Converts a number parameter to size_t suitable for ArrayBuffer sizes

src/node_external_reference.h

@@ -118,7 +118,6 @@ class ExternalReferenceRegistry {
   V(v8)                                                                        \
   V(zlib)                                                                      \
   V(wasm_web_api)                                                              \
-  V(webstreams)                                                                \
   V(worker)
 
 #if NODE_HAVE_I18N_SUPPORT

src/node_util.cc

@@ -27,6 +27,7 @@ using v8::Local;
 using v8::LocalVector;
 using v8::MaybeLocal;
 using v8::Name;
+using v8::Number;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::ONLY_CONFIGURABLE;
@@ -42,6 +43,7 @@ using v8::StackFrame;
 using v8::StackTrace;
 using v8::String;
 using v8::Uint32;
+using v8::Uint8Array;
 using v8::Value;
 
 // If a UTF-16 character is a low/trailing surrogate.
@@ -194,6 +196,106 @@ void ArrayBufferViewHasBuffer(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(args[0].As<ArrayBufferView>()->HasBuffer());
 }
 
+// Returns [buffer, byteOffset, byteLength] in a single binding crossing,
+// equivalent to reading the three properties via
+// Reflect.get(view.constructor.prototype, ..., view). Uses the V8 API
+// directly so it is immune to prototype tampering and avoids the JS-side
+// overhead of the defensive accessors in lib/internal/.
+void GetArrayBufferView(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  CHECK(args[0]->IsArrayBufferView());
+  Local<ArrayBufferView> view = args[0].As<ArrayBufferView>();
+  Local<Value> values[] = {
+      view->Buffer(),
+      Number::New(isolate, static_cast<double>(view->ByteOffset())),
+      Number::New(isolate, static_cast<double>(view->ByteLength())),
+  };
+  args.GetReturnValue().Set(Array::New(isolate, values, arraysize(values)));
+}
+
+static bool ReadNonNegativeInteger(Local<Value> value, uint64_t* result) {
+  constexpr double kMaxSafeInteger = 9007199254740991.0;
+  double number = value.As<Number>()->Value();
+  if (number < 0 || number > kMaxSafeInteger) {
+    return false;
+  }
+  *result = static_cast<uint64_t>(number);
+  return static_cast<double>(*result) == number;
+}
+
+// Returns true iff bytes can be safely copied between the buffers given the
+// requested offsets and count. Matches lib/internal/webstreams/util.js:
+//   toBuffer !== fromBuffer &&
+//   !toBuffer.detached &&
+//   !fromBuffer.detached &&
+//   toIndex + count <= toBuffer.byteLength &&
+//   fromIndex + count <= fromBuffer.byteLength
+void CanCopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
+  CHECK(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer());
+  CHECK(args[1]->IsNumber());
+  CHECK(args[2]->IsArrayBuffer() || args[2]->IsSharedArrayBuffer());
+  CHECK(args[3]->IsNumber());
+  CHECK(args[4]->IsNumber());
+
+  // SharedArrayBuffer handles are interoperable with ArrayBuffer handles in
+  // V8, so we can use the ArrayBuffer accessors uniformly. WasDetached()
+  // always returns false on a SAB.
+  Local<ArrayBuffer> to_buffer = args[0].As<ArrayBuffer>();
+  Local<ArrayBuffer> from_buffer = args[2].As<ArrayBuffer>();
+
+  if (to_buffer->StrictEquals(from_buffer)) {
+    args.GetReturnValue().Set(false);
+    return;
+  }
+  if (to_buffer->WasDetached() || from_buffer->WasDetached()) {
+    args.GetReturnValue().Set(false);
+    return;
+  }
+
+  uint64_t to_index;
+  uint64_t from_index;
+  uint64_t count;
+  if (!ReadNonNegativeInteger(args[1], &to_index) ||
+      !ReadNonNegativeInteger(args[3], &from_index) ||
+      !ReadNonNegativeInteger(args[4], &count)) {
+    args.GetReturnValue().Set(false);
+    return;
+  }
+
+  const uint64_t to_byte_length = to_buffer->ByteLength();
+  const uint64_t from_byte_length = from_buffer->ByteLength();
+
+  bool ok = to_index <= to_byte_length && count <= to_byte_length - to_index &&
+            from_index <= from_byte_length &&
+            count <= from_byte_length - from_index;
+  args.GetReturnValue().Set(ok);
+}
+
+// Equivalent to:
+//   new Uint8Array(view.buffer.slice(view.byteOffset,
+//                                    view.byteOffset + view.byteLength))
+// Allocates a fresh ArrayBuffer with the view's bytes copied into it, then
+// returns a Uint8Array over the full new buffer. Avoids the JS-side
+// Reflect.get + slice round-trip.
+void CloneAsUint8Array(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  Isolate* isolate = env->isolate();
+  CHECK(args[0]->IsArrayBufferView());
+  Local<ArrayBufferView> view = args[0].As<ArrayBufferView>();
+  size_t byte_length = view->ByteLength();
+  Local<ArrayBuffer> new_buffer;
+  if (!ArrayBuffer::MaybeNew(isolate, byte_length).ToLocal(&new_buffer)) {
+    // MaybeNew does not schedule an exception on allocation failure.
+    THROW_ERR_MEMORY_ALLOCATION_FAILED(isolate);
+    return;
+  }
+  if (byte_length > 0) {
+    size_t copied = view->CopyContents(new_buffer->Data(), byte_length);
+    CHECK_EQ(copied, byte_length);
+  }
+  args.GetReturnValue().Set(Uint8Array::New(new_buffer, 0, byte_length));
+}
+
 static uint32_t GetUVHandleTypeCode(const uv_handle_type type) {
   // TODO(anonrig): We can use an enum here and then create the array in the
   // binding, which will remove the hard-coding in C++ and JS land.
@@ -480,6 +582,9 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(GetExternalValue);
   registry->Register(Sleep);
   registry->Register(ArrayBufferViewHasBuffer);
+  registry->Register(GetArrayBufferView);
+  registry->Register(CanCopyArrayBuffer);
+  registry->Register(CloneAsUint8Array);
   registry->Register(GuessHandleType);
   registry->Register(fast_guess_handle_type_);
   registry->Register(ParseEnv);
@@ -589,6 +694,11 @@ void Initialize(Local<Object> target,
   SetMethod(context, target, "parseEnv", ParseEnv);
   SetMethod(
       context, target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
+  SetMethodNoSideEffect(
+      context, target, "getArrayBufferView", GetArrayBufferView);
+  SetMethodNoSideEffect(
+      context, target, "canCopyArrayBuffer", CanCopyArrayBuffer);
+  SetMethod(context, target, "cloneAsUint8Array", CloneAsUint8Array);
   SetMethod(context,
             target,
             "constructSharedArrayBuffer",