From 82d44f4909faebe0ce425e7645e5ea0136645c7f Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli <yagiz@nizipli.com> Date: Fri, 18 Nov 2022 18:32:57 -0500 Subject: [PATCH 1/3] util: add isArrayBufferDetached method --- doc/api/util.md | 21 +++++++++++++++++++++ lib/internal/util/types.js | 13 ++++++++++++- lib/internal/webstreams/readablestream.js | 6 +++--- lib/internal/webstreams/util.js | 17 ++--------------- src/node_types.cc | 13 +++++++++++++ test/parallel/test-util-types.js | 2 +- typings/internalBinding/types.d.ts | 1 + 7 files changed, 53 insertions(+), 20 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 69549ebd0da993..6d3ed728e2888a 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -2024,6 +2024,27 @@ util.types.isAnyArrayBuffer(new ArrayBuffer()); // Returns true util.types.isAnyArrayBuffer(new SharedArrayBuffer()); // Returns true ``` +### `util.types.isArrayBufferDetached(value)` + +<!-- YAML +added: REPLACEME +--> + +* `value` {any} +* Returns: {boolean} + +Returns `true` if the value is a built-in [`ArrayBuffer`][] and +is detached. Detached arrays have a byte length of 0, which prevents +JavaScript from ever accessing underlying backing store. + +For example, we can end up with a detached buffer when using a BYOB +(bring your own buffer) on a ReadableStream. + +```js +util.types.isArrayBufferDetached(null); // Returns false +util.types.isArrayBufferDetached(new ArrayBuffer()); // Returns false +``` + ### `util.types.isArrayBufferView(value)` <!-- YAML diff --git a/lib/internal/util/types.js b/lib/internal/util/types.js index 544f4c3da49c72..7e1bff61b95ee6 100644 --- a/lib/internal/util/types.js +++ b/lib/internal/util/types.js @@ -2,10 +2,13 @@ const { ArrayBufferIsView, + ArrayBufferPrototypeGetByteLength, ObjectDefineProperties, TypedArrayPrototypeGetSymbolToStringTag, } = primordials; +const { isArrayBufferDetached: _isArrayBufferDetached, ...internalTypes } = internalBinding('types'); + function isTypedArray(value) { return TypedArrayPrototypeGetSymbolToStringTag(value) !== undefined; } @@ -54,8 +57,16 @@ function isBigUint64Array(value) { return TypedArrayPrototypeGetSymbolToStringTag(value) === 'BigUint64Array'; } +function isArrayBufferDetached(value) { + if (ArrayBufferPrototypeGetByteLength(value) === 0) { + return _isArrayBufferDetached(value); + } + return false; +} + module.exports = { - ...internalBinding('types'), + ...internalTypes, + isArrayBufferDetached, isArrayBufferView: ArrayBufferIsView, isTypedArray, isUint8Array, diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index 3239f3ee932caa..f7ec11ad21f64e 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -43,6 +43,7 @@ const { } = internalBinding('messaging'); const { + isArrayBufferDetached, isArrayBufferView, isDataView, } = require('internal/util/types'); @@ -104,7 +105,6 @@ const { extractHighWaterMark, extractSizeAlgorithm, lazyTransfer, - isDetachedBuffer, isViewedArrayBufferDetached, isBrandCheck, resetQueue, @@ -669,7 +669,7 @@ class ReadableStreamBYOBRequest { const viewBuffer = ArrayBufferViewGetBuffer(view); const viewBufferByteLength = ArrayBufferGetByteLength(viewBuffer); - if (isDetachedBuffer(viewBuffer)) { + if (isArrayBufferDetached(viewBuffer)) { throw new ERR_INVALID_STATE.TypeError('Viewed ArrayBuffer is detached'); } @@ -2643,7 +2643,7 @@ function readableByteStreamControllerEnqueue(controller, chunk) { if (pendingPullIntos.length) { const firstPendingPullInto = pendingPullIntos[0]; - if (isDetachedBuffer(firstPendingPullInto.buffer)) { + if (isArrayBufferDetached(firstPendingPullInto.buffer)) { throw new ERR_INVALID_STATE.TypeError( 'Destination ArrayBuffer is detached', ); diff --git a/lib/internal/webstreams/util.js b/lib/internal/webstreams/util.js index 0e260d074c73c2..cbfdbf8b965390 100644 --- a/lib/internal/webstreams/util.js +++ b/lib/internal/webstreams/util.js @@ -32,6 +32,7 @@ const { } = internalBinding('buffer'); const { + isArrayBufferDetached, isPromise, } = require('internal/util/types'); @@ -139,23 +140,10 @@ function transferArrayBuffer(buffer) { return res; } -function isDetachedBuffer(buffer) { - if (ArrayBufferGetByteLength(buffer) === 0) { - // TODO(daeyeon): Consider using C++ builtin to improve performance. - try { - new Uint8Array(buffer); - } catch (error) { - assert(error.name === 'TypeError'); - return true; - } - } - return false; -} - function isViewedArrayBufferDetached(view) { return ( ArrayBufferViewGetByteLength(view) === 0 && - isDetachedBuffer(ArrayBufferViewGetBuffer(view)) + isArrayBufferDetached(ArrayBufferViewGetBuffer(view)) ); } @@ -256,7 +244,6 @@ module.exports = { extractSizeAlgorithm, lazyTransfer, isBrandCheck, - isDetachedBuffer, isPromisePending, isViewedArrayBufferDetached, peekQueueValue, diff --git a/src/node_types.cc b/src/node_types.cc index 87550a1428bd34..252e833df4d9e2 100644 --- a/src/node_types.cc +++ b/src/node_types.cc @@ -61,6 +61,16 @@ static void IsBoxedPrimitive(const FunctionCallbackInfo<Value>& args) { args[0]->IsSymbolObject()); } +static void IsArrayBufferDetached(const FunctionCallbackInfo<Value>& args) { + if (args[0]->IsArrayBuffer()) { + auto buffer = args[0].As<v8::ArrayBuffer>(); + args.GetReturnValue().Set(buffer->WasDetached()); + return; + } + + args.GetReturnValue().Set(false); +} + void InitializeTypes(Local<Object> target, Local<Value> unused, Local<Context> context, @@ -71,6 +81,8 @@ void InitializeTypes(Local<Object> target, SetMethodNoSideEffect(context, target, "isAnyArrayBuffer", IsAnyArrayBuffer); SetMethodNoSideEffect(context, target, "isBoxedPrimitive", IsBoxedPrimitive); + SetMethodNoSideEffect( + context, target, "isArrayBufferDetached", IsArrayBufferDetached); } } // anonymous namespace @@ -82,6 +94,7 @@ void RegisterTypesExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(IsAnyArrayBuffer); registry->Register(IsBoxedPrimitive); + registry->Register(IsArrayBufferDetached); } } // namespace node diff --git a/test/parallel/test-util-types.js b/test/parallel/test-util-types.js index 60380dca09a298..c61d5f4c872d5a 100644 --- a/test/parallel/test-util-types.js +++ b/test/parallel/test-util-types.js @@ -56,7 +56,7 @@ for (const [ value, _method ] of [ for (const key of Object.keys(types)) { if ((types.isArrayBufferView(value) || types.isAnyArrayBuffer(value)) && key.includes('Array') || - key === 'isBoxedPrimitive') { + key === 'isBoxedPrimitive' || key === 'isArrayBufferDetached') { continue; } diff --git a/typings/internalBinding/types.d.ts b/typings/internalBinding/types.d.ts index c8efea84b69e9d..1b91471dcf87c7 100644 --- a/typings/internalBinding/types.d.ts +++ b/typings/internalBinding/types.d.ts @@ -5,6 +5,7 @@ declare function InternalBinding(binding: 'types'): { isArrayBuffer(value: unknown): value is ArrayBuffer; isArgumentsObject(value: unknown): value is ArrayLike<unknown>; isBoxedPrimitive(value: unknown): value is (BigInt | Boolean | Number | String | Symbol); + isArrayBufferDetached(value: unknown): boolean; isDataView(value: unknown): value is DataView; isExternal(value: unknown): value is Object; isMap(value: unknown): value is Map<unknown, unknown>; From 7aae6497d019ab8741e514ed9d1c35243d17f021 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli <yagiz@nizipli.com> Date: Mon, 21 Nov 2022 08:59:01 -0500 Subject: [PATCH 2/3] fixup! util: add isArrayBufferDetached method --- doc/api/util.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 6d3ed728e2888a..1af3fff2059a50 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -2034,15 +2034,18 @@ added: REPLACEME * Returns: {boolean} Returns `true` if the value is a built-in [`ArrayBuffer`][] and -is detached. Detached arrays have a byte length of 0, which prevents -JavaScript from ever accessing underlying backing store. - -For example, we can end up with a detached buffer when using a BYOB -(bring your own buffer) on a ReadableStream. +is detached. For example, we can end up with a detached buffer +when using a BYOB (bring your own buffer) on a ReadableStream. +Detached arrays have a `byteLength` of 0, and their contents can +not be accessed in JavaScript. ```js util.types.isArrayBufferDetached(null); // Returns false util.types.isArrayBufferDetached(new ArrayBuffer()); // Returns false + +const { buffer } = new Uint8Array([1, 2, 3]); +new MessageChannel().port1.postMessage('', [buffer]); +util.types.isArrayBufferDetached(buffer); // Returns true ``` ### `util.types.isArrayBufferView(value)` From f207b32231c1c96b702ba695109a02a7f6ae32d1 Mon Sep 17 00:00:00 2001 From: Yagiz Nizipli <yagiz@nizipli.com> Date: Mon, 21 Nov 2022 17:00:11 -0500 Subject: [PATCH 3/3] test: add util.types tests --- lib/internal/util/types.js | 3 ++- test/parallel/test-util-types.js | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/internal/util/types.js b/lib/internal/util/types.js index 7e1bff61b95ee6..63eec4553dc426 100644 --- a/lib/internal/util/types.js +++ b/lib/internal/util/types.js @@ -1,6 +1,7 @@ 'use strict'; const { + ArrayBuffer, ArrayBufferIsView, ArrayBufferPrototypeGetByteLength, ObjectDefineProperties, @@ -58,7 +59,7 @@ function isBigUint64Array(value) { } function isArrayBufferDetached(value) { - if (ArrayBufferPrototypeGetByteLength(value) === 0) { + if (value instanceof ArrayBuffer && ArrayBufferPrototypeGetByteLength(value) === 0) { return _isArrayBufferDetached(value); } return false; diff --git a/test/parallel/test-util-types.js b/test/parallel/test-util-types.js index c61d5f4c872d5a..fc64c484c3d58b 100644 --- a/test/parallel/test-util-types.js +++ b/test/parallel/test-util-types.js @@ -67,6 +67,15 @@ for (const [ value, _method ] of [ } } +// Check detached array buffers. +{ + [null, undefined].forEach((entry) => assert(types.isArrayBufferDetached(entry) === false)); + + const { buffer } = new Uint8Array([1, 2, 3]); + new MessageChannel().port1.postMessage('', [buffer]); + assert(types.isArrayBufferDetached(buffer)); +} + // Check boxed primitives. [ new Boolean(),