From a319ce575a1caa46bb2531eb541dff74ebe94420 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 31 May 2026 12:37:48 +0200 Subject: [PATCH 1/4] stream: move WHATWG byte-stream helpers to C++ Implement three defensive helpers from lib/internal/webstreams/util.js in a new internal binding (src/node_webstreams.cc): * getArrayBufferView * canCopyArrayBuffer * cloneAsUint8Array The previous JavaScript versions used Reflect.get on view.constructor.prototype and called ArrayBuffer.prototype methods through primordials so they would survive prototype tampering. The C++ versions use the V8 ArrayBufferView and ArrayBuffer APIs directly, preserving the same robustness without the JS-side overhead. getArrayBufferView returns { buffer, byteOffset, byteLength } in a single binding crossing, replacing three separate accessors. cloneAsUint8Array uses ArrayBuffer::MaybeNew so the process is not killed on allocation failure. These functions sit on the byte-stream hot paths (ReadableByteStreamController enqueue/read, pull-into descriptor copy, tee clones). ReadableStream type='bytes' throughput on benchmark/webstreams/readable-read.js improves by ~15% on the BYOB read path on my workstation. WPT streams parity is preserved (1403 subtests passing, 0 unexpected failures, identical to baseline). Signed-off-by: Matteo Collina --- lib/internal/webstreams/readablestream.js | 38 +++--- lib/internal/webstreams/util.js | 77 +++---------- node.gyp | 1 + src/node_binding.cc | 1 + src/node_external_reference.h | 1 + src/node_webstreams.cc | 134 ++++++++++++++++++++++ 6 files changed, 173 insertions(+), 79 deletions(-) create mode 100644 src/node_webstreams.cc diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index ee496f30e5934e..79a956156d9d0c 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -97,9 +97,6 @@ const { } = require('internal/streams/utils'); const { - ArrayBufferViewGetBuffer, - ArrayBufferViewGetByteLength, - ArrayBufferViewGetByteOffset, AsyncIterator, canCopyArrayBuffer, cloneAsUint8Array, @@ -111,6 +108,7 @@ const { enqueueValueWithSize, extractHighWaterMark, extractSizeAlgorithm, + getArrayBufferView, getNonWritablePropertyDescriptor, isBrandCheck, kState, @@ -751,8 +749,10 @@ class ReadableStreamBYOBRequest { 'This BYOB request has been invalidated'); } - const viewByteLength = ArrayBufferViewGetByteLength(view); - const viewBuffer = ArrayBufferViewGetBuffer(view); + const { + buffer: viewBuffer, + byteLength: viewByteLength, + } = getArrayBufferView(view); const viewBufferByteLength = ArrayBufferPrototypeGetByteLength(viewBuffer); if (ArrayBufferPrototypeGetDetached(viewBuffer)) { @@ -1037,8 +1037,10 @@ class ReadableStreamBYOBReader { } validateObject(options, 'options', kValidateObjectAllowObjectsAndNull); - const viewByteLength = ArrayBufferViewGetByteLength(view); - const viewBuffer = ArrayBufferViewGetBuffer(view); + const { + buffer: viewBuffer, + byteLength: viewByteLength, + } = getArrayBufferView(view); if (isSharedArrayBuffer(viewBuffer)) { throw new ERR_INVALID_ARG_VALUE( @@ -1255,8 +1257,10 @@ class ReadableByteStreamController { if (!isReadableByteStreamController(this)) throw new ERR_INVALID_THIS('ReadableByteStreamController'); validateBuffer(chunk); - const chunkByteLength = ArrayBufferViewGetByteLength(chunk); - const chunkBuffer = ArrayBufferViewGetBuffer(chunk); + const { + buffer: chunkBuffer, + byteLength: chunkByteLength, + } = getArrayBufferView(chunk); if (isSharedArrayBuffer(chunkBuffer)) { throw new ERR_INVALID_ARG_VALUE( @@ -2858,9 +2862,7 @@ function readableByteStreamControllerPullInto( assert(minimumFill >= elementSize && minimumFill <= view.byteLength); assert(minimumFill % elementSize === 0); - const buffer = ArrayBufferViewGetBuffer(view); - const byteOffset = ArrayBufferViewGetByteOffset(view); - const byteLength = ArrayBufferViewGetByteLength(view); + const { buffer, byteOffset, byteLength } = getArrayBufferView(view); const bufferByteLength = ArrayBufferPrototypeGetByteLength(buffer); let transferredBuffer; @@ -3001,9 +3003,7 @@ function readableByteStreamControllerEnqueue(controller, chunk) { stream, } = controller[kState]; - const buffer = ArrayBufferViewGetBuffer(chunk); - const byteOffset = ArrayBufferViewGetByteOffset(chunk); - const byteLength = ArrayBufferViewGetByteLength(chunk); + const { buffer, byteOffset, byteLength } = getArrayBufferView(chunk); if (closeRequested || stream[kState].state !== 'readable') return; @@ -3296,9 +3296,11 @@ function readableByteStreamControllerRespondWithNewView(controller, view) { const desc = pendingPullIntos[0]; assert(stream[kState].state !== 'errored'); - const viewByteLength = ArrayBufferViewGetByteLength(view); - const viewByteOffset = ArrayBufferViewGetByteOffset(view); - const viewBuffer = ArrayBufferViewGetBuffer(view); + const { + buffer: viewBuffer, + byteOffset: viewByteOffset, + byteLength: viewByteLength, + } = getArrayBufferView(view); const viewBufferByteLength = ArrayBufferPrototypeGetByteLength(viewBuffer); if (stream[kState].state === 'closed') { diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js index 836e1de130505b..a0840a476ad5b9 100644 --- a/lib/internal/webstreams/util.js +++ b/lib/internal/webstreams/util.js @@ -1,21 +1,17 @@ 'use strict'; const { - ArrayBufferPrototypeGetByteLength, - ArrayBufferPrototypeGetDetached, - ArrayBufferPrototypeSlice, ArrayPrototypePush, ArrayPrototypeShift, AsyncIteratorPrototype, - FunctionPrototypeCall, MathMax, NumberIsNaN, PromisePrototypeThen, PromiseReject, PromiseResolve, ReflectGet, + ReflectApply, Symbol, - Uint8Array, } = primordials; const { @@ -28,6 +24,12 @@ const { copyArrayBuffer, } = internalBinding('buffer'); +const { + canCopyArrayBuffer, + cloneAsUint8Array, + getArrayBufferView, +} = internalBinding('webstreams'); + const { inspect, } = require('util'); @@ -93,38 +95,11 @@ function customInspect(depth, options, name, data) { return `${name} ${inspect(data, opts)}`; } -// These are defensive to work around the possibility that -// the buffer, byteLength, and byteOffset properties on -// ArrayBuffer and ArrayBufferView's may have been tampered with. - -function ArrayBufferViewGetBuffer(view) { - return ReflectGet(view.constructor.prototype, 'buffer', view); -} - -function ArrayBufferViewGetByteLength(view) { - return ReflectGet(view.constructor.prototype, 'byteLength', view); -} - -function ArrayBufferViewGetByteOffset(view) { - return ReflectGet(view.constructor.prototype, 'byteOffset', view); -} - -function cloneAsUint8Array(view) { - const buffer = ArrayBufferViewGetBuffer(view); - const byteOffset = ArrayBufferViewGetByteOffset(view); - const byteLength = ArrayBufferViewGetByteLength(view); - return new Uint8Array( - ArrayBufferPrototypeSlice(buffer, byteOffset, byteOffset + byteLength), - ); -} - -function canCopyArrayBuffer(toBuffer, toIndex, fromBuffer, fromIndex, count) { - return toBuffer !== fromBuffer && - !ArrayBufferPrototypeGetDetached(toBuffer) && - !ArrayBufferPrototypeGetDetached(fromBuffer) && - toIndex + count <= ArrayBufferPrototypeGetByteLength(toBuffer) && - fromIndex + count <= ArrayBufferPrototypeGetByteLength(fromBuffer); -} +// 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. function isBrandCheck(brand) { return (value) => { @@ -175,25 +150,9 @@ function enqueueValueWithSize(controller, value, size) { controller[kState].queueTotalSize += size; } -// Arity-specialized variants of the promise-callback wrapper. The generic -// rest-parameter + ReflectApply form allocated an arguments array on every -// invocation; these run on per-chunk hot paths (pull/write/transform), so -// each known call-site arity gets its own wrapper. The exact number of -// arguments passed through to the user callback is observable and must be -// preserved. -function createPromiseCallbackNoParams(name, fn, thisArg) { - validateFunction(fn, name); - return async () => FunctionPrototypeCall(fn, thisArg); -} - -function createPromiseCallback1Param(name, fn, thisArg) { - validateFunction(fn, name); - return async (arg) => FunctionPrototypeCall(fn, thisArg, arg); -} - -function createPromiseCallback2Params(name, fn, thisArg) { +function createPromiseCallback(name, fn, thisArg) { validateFunction(fn, name); - return async (arg1, arg2) => FunctionPrototypeCall(fn, thisArg, arg1, arg2); + return async (...args) => ReflectApply(fn, thisArg, args); } function isPromisePending(promise) { @@ -248,22 +207,18 @@ function lazyTransfer() { } module.exports = { - ArrayBufferViewGetBuffer, - ArrayBufferViewGetByteLength, - ArrayBufferViewGetByteOffset, AsyncIterator, canCopyArrayBuffer, cloneAsUint8Array, copyArrayBuffer, - createPromiseCallbackNoParams, - createPromiseCallback1Param, - createPromiseCallback2Params, + createPromiseCallback, customInspect, defaultSizeAlgorithm, dequeueValue, enqueueValueWithSize, extractHighWaterMark, extractSizeAlgorithm, + getArrayBufferView, getNonWritablePropertyDescriptor, isBrandCheck, isPromisePending, diff --git a/node.gyp b/node.gyp index 546b24cfc2a694..7915e9d314b7ff 100644 --- a/node.gyp +++ b/node.gyp @@ -170,6 +170,7 @@ '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', diff --git a/src/node_binding.cc b/src/node_binding.cc index 044eadd0eafcda..1f82ad58fb3bb8 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -97,6 +97,7 @@ V(v8) \ V(wasi) \ V(wasm_web_api) \ + V(webstreams) \ V(watchdog) \ V(worker) \ V(zlib) diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 1e987ce2d4f314..89be54a19cc00c 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -119,6 +119,7 @@ class ExternalReferenceRegistry { V(v8) \ V(zlib) \ V(wasm_web_api) \ + V(webstreams) \ V(worker) #if NODE_HAVE_I18N_SUPPORT diff --git a/src/node_webstreams.cc b/src/node_webstreams.cc new file mode 100644 index 00000000000000..b10b425b53bf8e --- /dev/null +++ b/src/node_webstreams.cc @@ -0,0 +1,134 @@ +#include "env-inl.h" +#include "node_binding.h" +#include "node_errors.h" +#include "node_external_reference.h" +#include "util-inl.h" +#include "v8.h" + +namespace node { +namespace webstreams { + +using v8::ArrayBuffer; +using v8::ArrayBufferView; +using v8::Context; +using v8::FunctionCallbackInfo; +using v8::Isolate; +using v8::Local; +using v8::Name; +using v8::Null; +using v8::Number; +using v8::Object; +using v8::Uint32; +using v8::Uint8Array; +using v8::Value; + +// 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& args) { + Isolate* isolate = args.GetIsolate(); + CHECK(args[0]->IsArrayBufferView()); + Local view = args[0].As(); + Local names[] = { + FIXED_ONE_BYTE_STRING(isolate, "buffer"), + FIXED_ONE_BYTE_STRING(isolate, "byteOffset"), + FIXED_ONE_BYTE_STRING(isolate, "byteLength"), + }; + Local values[] = { + view->Buffer(), + Number::New(isolate, static_cast(view->ByteOffset())), + Number::New(isolate, static_cast(view->ByteLength())), + }; + args.GetReturnValue().Set( + Object::New(isolate, Null(isolate), names, values, arraysize(names))); +} + +// 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& args) { + CHECK(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer()); + CHECK(args[1]->IsUint32()); + CHECK(args[2]->IsArrayBuffer() || args[2]->IsSharedArrayBuffer()); + CHECK(args[3]->IsUint32()); + CHECK(args[4]->IsUint32()); + + // 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 to_buffer = args[0].As(); + Local from_buffer = args[2].As(); + + if (to_buffer->StrictEquals(from_buffer)) { + args.GetReturnValue().Set(false); + return; + } + if (to_buffer->WasDetached() || from_buffer->WasDetached()) { + args.GetReturnValue().Set(false); + return; + } + + uint32_t to_index = args[1].As()->Value(); + uint32_t from_index = args[3].As()->Value(); + uint32_t count = args[4].As()->Value(); + + size_t to_byte_length = to_buffer->ByteLength(); + size_t from_byte_length = from_buffer->ByteLength(); + + bool ok = static_cast(to_index) + count <= to_byte_length && + static_cast(from_index) + count <= from_byte_length; + 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& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + CHECK(args[0]->IsArrayBufferView()); + Local view = args[0].As(); + size_t byte_length = view->ByteLength(); + Local new_buffer; + if (!ArrayBuffer::MaybeNew(isolate, byte_length).ToLocal(&new_buffer)) { + // MaybeNew does not schedule an exception on allocation failure. + env->ThrowRangeError("Array buffer allocation failed"); + 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)); +} + +void Initialize(Local target, + Local unused, + Local context, + void* priv) { + SetMethod(context, target, "getArrayBufferView", GetArrayBufferView); + SetMethod(context, target, "canCopyArrayBuffer", CanCopyArrayBuffer); + SetMethod(context, target, "cloneAsUint8Array", CloneAsUint8Array); +} + +void RegisterExternalReferences(ExternalReferenceRegistry* registry) { + registry->Register(GetArrayBufferView); + registry->Register(CanCopyArrayBuffer); + registry->Register(CloneAsUint8Array); +} + +} // namespace webstreams +} // namespace node + +NODE_BINDING_CONTEXT_AWARE_INTERNAL(webstreams, node::webstreams::Initialize) +NODE_BINDING_EXTERNAL_REFERENCE(webstreams, + node::webstreams::RegisterExternalReferences) From ba494c94157c999bef21835011c1e741212e2bbd Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 16 Jun 2026 11:20:45 +0200 Subject: [PATCH 2/4] stream: move byte-stream helpers to util binding Signed-off-by: Matteo Collina --- lib/internal/webstreams/readablestream.js | 40 ++++--- lib/internal/webstreams/util.js | 21 ++-- node.gyp | 1 - src/node_binding.cc | 1 - src/node_buffer.cc | 49 +++++--- src/node_external_reference.h | 1 - src/node_util.cc | 110 ++++++++++++++++++ src/node_webstreams.cc | 134 ---------------------- test/parallel/test-util-internal.js | 28 +++++ 9 files changed, 204 insertions(+), 181 deletions(-) delete mode 100644 src/node_webstreams.cc diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 79a956156d9d0c..7a980181e6e3b9 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -749,10 +749,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)) { @@ -1037,10 +1036,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( @@ -1257,10 +1255,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( @@ -2862,7 +2859,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; @@ -3003,7 +3003,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; @@ -3296,11 +3299,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') { diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js index a0840a476ad5b9..e9dd845beadaa6 100644 --- a/lib/internal/webstreams/util.js +++ b/lib/internal/webstreams/util.js @@ -27,20 +27,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 { @@ -96,10 +93,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) => { diff --git a/node.gyp b/node.gyp index 7915e9d314b7ff..546b24cfc2a694 100644 --- a/node.gyp +++ b/node.gyp @@ -170,7 +170,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', diff --git a/src/node_binding.cc b/src/node_binding.cc index 1f82ad58fb3bb8..044eadd0eafcda 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -97,7 +97,6 @@ V(v8) \ V(wasi) \ V(wasm_web_api) \ - V(webstreams) \ V(watchdog) \ V(worker) \ V(zlib) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 9adb02517efc42..00341879d9d0cf 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1524,6 +1524,16 @@ static void SetDetachKey(const FunctionCallbackInfo& args) { namespace { +bool ReadNonNegativeInteger(Local value, uint64_t* result) { + constexpr double kMaxSafeInteger = 9007199254740991.0; + double number = value.As()->Value(); + if (number < 0 || number > kMaxSafeInteger) { + return false; + } + *result = static_cast(number); + return static_cast(*result) == number; +} + std::pair DecomposeBufferToParts(Local buffer) { void* pointer; size_t byte_length; @@ -1551,10 +1561,10 @@ void CopyArrayBuffer(const FunctionCallbackInfo& 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& args) { size_t source_byte_length; std::tie(source, source_byte_length) = DecomposeBufferToParts(args[2]); - uint32_t destination_offset = args[1].As()->Value(); - uint32_t source_offset = args[3].As()->Value(); - size_t bytes_to_copy = args[4].As()->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(destination) + destination_offset; - uint8_t* src = static_cast(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(destination_offset_u); + const size_t source_offset_s = static_cast(source_offset_u); + const size_t bytes_to_copy_s = static_cast(bytes_to_copy_u); + uint8_t* dest = static_cast(destination) + destination_offset_s; + uint8_t* src = static_cast(source) + source_offset_s; + memcpy(dest, src, bytes_to_copy_s); } // Converts a number parameter to size_t suitable for ArrayBuffer sizes diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 89be54a19cc00c..1e987ce2d4f314 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -119,7 +119,6 @@ class ExternalReferenceRegistry { V(v8) \ V(zlib) \ V(wasm_web_api) \ - V(webstreams) \ V(worker) #if NODE_HAVE_I18N_SUPPORT diff --git a/src/node_util.cc b/src/node_util.cc index 6d3373caae6c5c..3944b0d5e98549 100644 --- a/src/node_util.cc +++ b/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& args) { args.GetReturnValue().Set(args[0].As()->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& args) { + Isolate* isolate = args.GetIsolate(); + CHECK(args[0]->IsArrayBufferView()); + Local view = args[0].As(); + Local values[] = { + view->Buffer(), + Number::New(isolate, static_cast(view->ByteOffset())), + Number::New(isolate, static_cast(view->ByteLength())), + }; + args.GetReturnValue().Set(Array::New(isolate, values, arraysize(values))); +} + +static bool ReadNonNegativeInteger(Local value, uint64_t* result) { + constexpr double kMaxSafeInteger = 9007199254740991.0; + double number = value.As()->Value(); + if (number < 0 || number > kMaxSafeInteger) { + return false; + } + *result = static_cast(number); + return static_cast(*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& 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 to_buffer = args[0].As(); + Local from_buffer = args[2].As(); + + 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& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + CHECK(args[0]->IsArrayBufferView()); + Local view = args[0].As(); + size_t byte_length = view->ByteLength(); + Local 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 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", diff --git a/src/node_webstreams.cc b/src/node_webstreams.cc deleted file mode 100644 index b10b425b53bf8e..00000000000000 --- a/src/node_webstreams.cc +++ /dev/null @@ -1,134 +0,0 @@ -#include "env-inl.h" -#include "node_binding.h" -#include "node_errors.h" -#include "node_external_reference.h" -#include "util-inl.h" -#include "v8.h" - -namespace node { -namespace webstreams { - -using v8::ArrayBuffer; -using v8::ArrayBufferView; -using v8::Context; -using v8::FunctionCallbackInfo; -using v8::Isolate; -using v8::Local; -using v8::Name; -using v8::Null; -using v8::Number; -using v8::Object; -using v8::Uint32; -using v8::Uint8Array; -using v8::Value; - -// 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& args) { - Isolate* isolate = args.GetIsolate(); - CHECK(args[0]->IsArrayBufferView()); - Local view = args[0].As(); - Local names[] = { - FIXED_ONE_BYTE_STRING(isolate, "buffer"), - FIXED_ONE_BYTE_STRING(isolate, "byteOffset"), - FIXED_ONE_BYTE_STRING(isolate, "byteLength"), - }; - Local values[] = { - view->Buffer(), - Number::New(isolate, static_cast(view->ByteOffset())), - Number::New(isolate, static_cast(view->ByteLength())), - }; - args.GetReturnValue().Set( - Object::New(isolate, Null(isolate), names, values, arraysize(names))); -} - -// 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& args) { - CHECK(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer()); - CHECK(args[1]->IsUint32()); - CHECK(args[2]->IsArrayBuffer() || args[2]->IsSharedArrayBuffer()); - CHECK(args[3]->IsUint32()); - CHECK(args[4]->IsUint32()); - - // 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 to_buffer = args[0].As(); - Local from_buffer = args[2].As(); - - if (to_buffer->StrictEquals(from_buffer)) { - args.GetReturnValue().Set(false); - return; - } - if (to_buffer->WasDetached() || from_buffer->WasDetached()) { - args.GetReturnValue().Set(false); - return; - } - - uint32_t to_index = args[1].As()->Value(); - uint32_t from_index = args[3].As()->Value(); - uint32_t count = args[4].As()->Value(); - - size_t to_byte_length = to_buffer->ByteLength(); - size_t from_byte_length = from_buffer->ByteLength(); - - bool ok = static_cast(to_index) + count <= to_byte_length && - static_cast(from_index) + count <= from_byte_length; - 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& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); - CHECK(args[0]->IsArrayBufferView()); - Local view = args[0].As(); - size_t byte_length = view->ByteLength(); - Local new_buffer; - if (!ArrayBuffer::MaybeNew(isolate, byte_length).ToLocal(&new_buffer)) { - // MaybeNew does not schedule an exception on allocation failure. - env->ThrowRangeError("Array buffer allocation failed"); - 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)); -} - -void Initialize(Local target, - Local unused, - Local context, - void* priv) { - SetMethod(context, target, "getArrayBufferView", GetArrayBufferView); - SetMethod(context, target, "canCopyArrayBuffer", CanCopyArrayBuffer); - SetMethod(context, target, "cloneAsUint8Array", CloneAsUint8Array); -} - -void RegisterExternalReferences(ExternalReferenceRegistry* registry) { - registry->Register(GetArrayBufferView); - registry->Register(CanCopyArrayBuffer); - registry->Register(CloneAsUint8Array); -} - -} // namespace webstreams -} // namespace node - -NODE_BINDING_CONTEXT_AWARE_INTERNAL(webstreams, node::webstreams::Initialize) -NODE_BINDING_EXTERNAL_REFERENCE(webstreams, - node::webstreams::RegisterExternalReferences) diff --git a/test/parallel/test-util-internal.js b/test/parallel/test-util-internal.js index e2b500daa70060..b446042ac3b226 100644 --- a/test/parallel/test-util-internal.js +++ b/test/parallel/test-util-internal.js @@ -7,6 +7,9 @@ const fixtures = require('../common/fixtures'); const { internalBinding } = require('internal/test/binding'); const { + canCopyArrayBuffer, + cloneAsUint8Array, + getArrayBufferView, privateSymbols: { arrow_message_private_symbol, }, @@ -28,3 +31,28 @@ try { } assert.match(arrowMessage, /bad_syntax\.js:1/); + +{ + const view = new Uint8Array(new ArrayBuffer(8), 2, 4); + assert.deepStrictEqual(getArrayBufferView(view), [view.buffer, 2, 4]); + + const sabView = new Uint8Array(new SharedArrayBuffer(8), 2, 4); + assert.deepStrictEqual(getArrayBufferView(sabView), [sabView.buffer, 2, 4]); +} + +{ + const source = new Uint8Array([1, 2, 3, 4]); + const clone = cloneAsUint8Array(source.subarray(1, 3)); + assert.deepStrictEqual([...clone], [2, 3]); + assert.notStrictEqual(clone.buffer, source.buffer); +} + +{ + const to = new ArrayBuffer(8); + const from = new ArrayBuffer(8); + const sab = new SharedArrayBuffer(8); + assert.strictEqual(canCopyArrayBuffer(to, 0, from, 0, 8), true); + assert.strictEqual(canCopyArrayBuffer(sab, 0, from, 0, 8), true); + assert.strictEqual(canCopyArrayBuffer(to, 2 ** 32, from, 0, 1), false); + assert.strictEqual(canCopyArrayBuffer(to, 0, from, 0, 2 ** 32), false); +} From af5b1f1afe3830efdd93ed2a576910d895fc1ffc Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Wed, 17 Jun 2026 12:32:43 +0000 Subject: [PATCH 3/4] fixup: address review nits - Replace magic number literal with static_cast expression - Remove unnecessary intermediate variables in CopyArrayBuffer --- src/node_buffer.cc | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 00341879d9d0cf..54fc48106db771 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1525,7 +1525,7 @@ static void SetDetachKey(const FunctionCallbackInfo& args) { namespace { bool ReadNonNegativeInteger(Local value, uint64_t* result) { - constexpr double kMaxSafeInteger = 9007199254740991.0; + constexpr double kMaxSafeInteger = static_cast((1LL << 53) - 1); double number = value.As()->Value(); if (number < 0 || number > kMaxSafeInteger) { return false; @@ -1582,22 +1582,14 @@ void CopyArrayBuffer(const FunctionCallbackInfo& args) { 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(destination_offset_u); - const size_t source_offset_s = static_cast(source_offset_u); - const size_t bytes_to_copy_s = static_cast(bytes_to_copy_u); - uint8_t* dest = static_cast(destination) + destination_offset_s; - uint8_t* src = static_cast(source) + source_offset_s; - memcpy(dest, src, bytes_to_copy_s); + CHECK_LE(destination_offset, static_cast(destination_byte_length)); + CHECK_LE(source_offset, static_cast(source_byte_length)); + CHECK_LE(bytes_to_copy, static_cast(destination_byte_length) - destination_offset); + CHECK_LE(bytes_to_copy, static_cast(source_byte_length) - source_offset); + + uint8_t* dest = static_cast(destination) + static_cast(destination_offset); + uint8_t* src = static_cast(source) + static_cast(source_offset); + memcpy(dest, src, static_cast(bytes_to_copy)); } // Converts a number parameter to size_t suitable for ArrayBuffer sizes From fb477ff3db9d5d25e99624aae707bb80859368ee Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 20 Jun 2026 12:02:26 +0200 Subject: [PATCH 4/4] squash! lint --- lib/internal/webstreams/util.js | 27 ++++++++++++++++++++++----- src/node_buffer.cc | 14 +++++++++----- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js index e9dd845beadaa6..7afc1607546d82 100644 --- a/lib/internal/webstreams/util.js +++ b/lib/internal/webstreams/util.js @@ -4,13 +4,12 @@ const { ArrayPrototypePush, ArrayPrototypeShift, AsyncIteratorPrototype, + FunctionPrototypeCall, MathMax, NumberIsNaN, PromisePrototypeThen, PromiseReject, PromiseResolve, - ReflectGet, - ReflectApply, Symbol, } = primordials; @@ -147,9 +146,25 @@ function enqueueValueWithSize(controller, value, size) { controller[kState].queueTotalSize += size; } -function createPromiseCallback(name, fn, thisArg) { +// Arity-specialized variants of the promise-callback wrapper. The generic +// rest-parameter + ReflectApply form allocated an arguments array on every +// invocation; these run on per-chunk hot paths (pull/write/transform), so +// each known call-site arity gets its own wrapper. The exact number of +// arguments passed through to the user callback is observable and must be +// preserved. +function createPromiseCallbackNoParams(name, fn, thisArg) { validateFunction(fn, name); - return async (...args) => ReflectApply(fn, thisArg, args); + return async () => FunctionPrototypeCall(fn, thisArg); +} + +function createPromiseCallback1Param(name, fn, thisArg) { + validateFunction(fn, name); + return async (arg) => FunctionPrototypeCall(fn, thisArg, arg); +} + +function createPromiseCallback2Params(name, fn, thisArg) { + validateFunction(fn, name); + return async (arg1, arg2) => FunctionPrototypeCall(fn, thisArg, arg1, arg2); } function isPromisePending(promise) { @@ -208,7 +223,9 @@ module.exports = { canCopyArrayBuffer, cloneAsUint8Array, copyArrayBuffer, - createPromiseCallback, + createPromiseCallbackNoParams, + createPromiseCallback1Param, + createPromiseCallback2Params, customInspect, defaultSizeAlgorithm, dequeueValue, diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 54fc48106db771..627833984221c7 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1584,11 +1584,15 @@ void CopyArrayBuffer(const FunctionCallbackInfo& args) { CHECK_LE(destination_offset, static_cast(destination_byte_length)); CHECK_LE(source_offset, static_cast(source_byte_length)); - CHECK_LE(bytes_to_copy, static_cast(destination_byte_length) - destination_offset); - CHECK_LE(bytes_to_copy, static_cast(source_byte_length) - source_offset); - - uint8_t* dest = static_cast(destination) + static_cast(destination_offset); - uint8_t* src = static_cast(source) + static_cast(source_offset); + CHECK_LE(bytes_to_copy, + static_cast(destination_byte_length) - destination_offset); + CHECK_LE(bytes_to_copy, + static_cast(source_byte_length) - source_offset); + + uint8_t* dest = static_cast(destination) + + static_cast(destination_offset); + uint8_t* src = + static_cast(source) + static_cast(source_offset); memcpy(dest, src, static_cast(bytes_to_copy)); }