diff --git a/README.md b/README.md index f1bdb6fea738ce..33ec62d271db5e 100644 --- a/README.md +++ b/README.md @@ -889,15 +889,15 @@ releases on a rotation basis as outlined in the * [bengl](https://github.com/bengl) - **Bryan English** <> (he/him) * [HeroDevs](https://www.herodevs.com/) - * [juanarbol](https://github.com/juanarbol) - OpenJSF handle: `juanarbol` + * [juanarbol](https://github.com/juanarbol) - OpenJS Slack handle: `juanarbol` **Juan José Arboleda** <> (he/him) - * [marco-ippolito](https://github.com/marco-ippolito) - OpenJSF handle: `Marco Ippolito` + * [marco-ippolito](https://github.com/marco-ippolito) - OpenJS Slack handle: `Marco Ippolito` **Marco Ippolito** <> (he/him) * [NodeSource](https://nodesource.com/) - * [RafaelGSS](https://github.com/RafaelGSS) - OpenJSF handle: `RafaelGSS` + * [RafaelGSS](https://github.com/RafaelGSS) - OpenJS Slack handle: `RafaelGSS` **Rafael Gonzaga** <> (he/him) * [Platformatic](https://platformatic.dev/) - * [mcollina](https://github.com/mcollina) - OpenJSF handle: `mcollina` + * [mcollina](https://github.com/mcollina) - OpenJS Slack handle: `mcollina` **Matteo Collina** <> (he/him) * [Red Hat](https://redhat.com) / [IBM](https://ibm.com) * [BethGriggs](https://github.com/BethGriggs) - diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js index d23f5d0ab6abe1..bfb4cd5ef7f900 100644 --- a/lib/internal/buffer.js +++ b/lib/internal/buffer.js @@ -33,7 +33,7 @@ const { hexWrite, ucs2Write, utf8WriteStatic, - createUnsafeArrayBuffer, + createUnsafeBuffer: createUnsafeBufferSlow, setDetachKey, } = internalBinding('buffer'); @@ -1101,7 +1101,7 @@ function createUnsafeBuffer(size) { return new FastBuffer(size); } - return new FastBuffer(createUnsafeArrayBuffer(size)); + return createUnsafeBufferSlow(size); } module.exports = { diff --git a/src/node_buffer.cc b/src/node_buffer.cc index d4a63cf610ca7f..acbc217552b4d5 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -1457,6 +1457,30 @@ inline size_t CheckNumberToSize(Local number) { return size; } +MaybeLocal CreateUnsafeArrayBufferFromSize(Environment* env, + size_t size) { + Isolate* isolate = env->isolate(); + + // 0-length, or zero-fill flag is set, or building snapshot + if (size == 0 || per_process::cli_options->zero_fill_all_buffers || + env->isolate_data()->is_building_snapshot()) { + return ArrayBuffer::New(isolate, size); + } + + std::unique_ptr store = ArrayBuffer::NewBackingStore( + isolate, + size, + BackingStoreInitializationMode::kUninitialized, + BackingStoreOnFailureMode::kReturnNull); + + if (!store) [[unlikely]] { + THROW_ERR_MEMORY_ALLOCATION_FAILED(env); + return MaybeLocal(); + } + + return ArrayBuffer::New(isolate, std::move(store)); +} + void CreateUnsafeArrayBuffer(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); if (args.Length() != 1) { @@ -1466,30 +1490,36 @@ void CreateUnsafeArrayBuffer(const FunctionCallbackInfo& args) { size_t size = CheckNumberToSize(args[0]); - Isolate* isolate = env->isolate(); - Local buf; + if (!CreateUnsafeArrayBufferFromSize(env, size).ToLocal(&buf)) + return; - // 0-length, or zero-fill flag is set, or building snapshot - if (size == 0 || per_process::cli_options->zero_fill_all_buffers || - env->isolate_data()->is_building_snapshot()) { - buf = ArrayBuffer::New(isolate, size); - } else { - std::unique_ptr store = ArrayBuffer::NewBackingStore( - isolate, - size, - BackingStoreInitializationMode::kUninitialized, - v8::BackingStoreOnFailureMode::kReturnNull); + args.GetReturnValue().Set(buf); +} - if (!store) [[unlikely]] { - THROW_ERR_MEMORY_ALLOCATION_FAILED(env); - return; - } +void CreateUnsafeBuffer(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + if (args.Length() != 1) { + env->ThrowRangeError("Invalid array buffer length"); + return; + } + + size_t size = CheckNumberToSize(args[0]); + + Local array_buffer; + if (!CreateUnsafeArrayBufferFromSize(env, size).ToLocal(&array_buffer)) + return; - buf = ArrayBuffer::New(isolate, std::move(store)); + if (env->buffer_prototype_object().IsEmpty()) { + args.GetReturnValue().Set(Uint8Array::New(array_buffer, 0, size)); + return; } - args.GetReturnValue().Set(buf); + Local buffer; + if (!Buffer::New(env, array_buffer, 0, size).ToLocal(&buffer)) + return; + + args.GetReturnValue().Set(buffer); } template @@ -1621,6 +1651,8 @@ void Initialize(Local target, SetMethod(context, target, "copyArrayBuffer", CopyArrayBuffer); SetMethodNoSideEffect( context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer); + SetMethodNoSideEffect( + context, target, "createUnsafeBuffer", CreateUnsafeBuffer); SetMethod(context, target, "swap16", Swap16); SetMethod(context, target, "swap32", Swap32); @@ -1723,6 +1755,7 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(CopyArrayBuffer); registry->Register(CreateUnsafeArrayBuffer); + registry->Register(CreateUnsafeBuffer); registry->Register(Atob); registry->Register(Btoa); diff --git a/test/parallel/test-buffer-create-unsafe-buffer-large-allocation-path.js b/test/parallel/test-buffer-create-unsafe-buffer-large-allocation-path.js new file mode 100644 index 00000000000000..bf4dfde923e914 --- /dev/null +++ b/test/parallel/test-buffer-create-unsafe-buffer-large-allocation-path.js @@ -0,0 +1,55 @@ +// Flags: --expose-internals +'use strict'; + +const assert = require('assert'); +const { internalBinding } = require('internal/test/binding'); +const { loadBuiltinModule } = require('internal/modules/helpers'); + +const binding = internalBinding('buffer'); +const internalBufferModule = loadBuiltinModule('internal/buffer'); + +function reloadInternalBuffer() { + internalBufferModule.loaded = false; + internalBufferModule.loading = false; + internalBufferModule.exports = {}; + internalBufferModule.exportKeys = undefined; + internalBufferModule.module = undefined; + internalBufferModule.compileForPublicLoader(); + return internalBufferModule.exports; +} + +const originalCreateUnsafeArrayBuffer = binding.createUnsafeArrayBuffer; +const originalCreateUnsafeBuffer = binding.createUnsafeBuffer; + +// Regression: Large allocations should not route through createUnsafeArrayBuffer. +binding.createUnsafeArrayBuffer = () => { + throw new Error('createUnsafeArrayBuffer should not be called for large buffers'); +}; + +let internalBuffer = reloadInternalBuffer(); +const large = internalBuffer.createUnsafeBuffer(65); +assert(large instanceof Uint8Array); +assert.strictEqual(large.length, 65); + +// Edge case: Small sizes should remain on the in-heap path. +binding.createUnsafeBuffer = () => { + throw new Error('createUnsafeBuffer should not be called for small buffers'); +}; + +internalBuffer = reloadInternalBuffer(); +const small = internalBuffer.createUnsafeBuffer(64); +assert(small instanceof Uint8Array); +assert.strictEqual(small.length, 64); + +// Nearby-path safety check: Public API behavior is unchanged for large sizes. +const userVisible = Buffer.allocUnsafeSlow(65); +assert(Buffer.isBuffer(userVisible)); +assert.strictEqual(userVisible.length, 65); + +binding.createUnsafeArrayBuffer = originalCreateUnsafeArrayBuffer; +if (originalCreateUnsafeBuffer === undefined) { + delete binding.createUnsafeBuffer; +} else { + binding.createUnsafeBuffer = originalCreateUnsafeBuffer; +} +reloadInternalBuffer();