Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const {
TypedArrayPrototypeGetByteOffset,
TypedArrayPrototypeGetLength,
TypedArrayPrototypeSet,
TypedArrayPrototypeSubarray,
Uint8Array,
} = primordials;

Expand Down Expand Up @@ -632,7 +631,13 @@ function fromArrayLike(obj) {
if (length > (poolSize - poolOffset))
createPool();
const b = new FastBuffer(allocPool, poolBase + poolOffset, length);
TypedArrayPrototypeSet(b, obj, 0);
if (isUint8Array(obj)) {
// The native copy reads from a source backed by an immutable ArrayBuffer,
// which %TypedArray%.prototype.set() rejects.
_copy(obj, b, 0, 0, length);
} else {
TypedArrayPrototypeSet(b, obj, 0);
}
poolOffset += length;
alignPool();
return b;
Expand Down Expand Up @@ -705,7 +710,9 @@ Buffer.concat = function concat(list, length) {
for (let i = 0; i < list.length; i++) {
const buf = list[i];
const bufLength = TypedArrayPrototypeGetByteLength(buf);
TypedArrayPrototypeSet(buffer, buf, pos);
// The native copy reads from a source backed by an immutable ArrayBuffer,
// which %TypedArray%.prototype.set() rejects.
_copy(buf, buffer, pos, 0, bufLength);
pos += bufLength;
}

Expand All @@ -731,13 +738,11 @@ Buffer.concat = function concat(list, length) {
const buf = list[i];
const bufLength = TypedArrayPrototypeGetByteLength(buf);
if (pos + bufLength > length) {
TypedArrayPrototypeSet(buffer,
TypedArrayPrototypeSubarray(buf, 0, length - pos),
pos);
_copy(buf, buffer, pos, 0, length - pos);
pos = length;
break;
}
TypedArrayPrototypeSet(buffer, buf, pos);
_copy(buf, buffer, pos, 0, bufLength);
pos += bufLength;
}

Expand Down
11 changes: 6 additions & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ const {
stringToSymlinkType,
toUnixTimestamp,
validateBufferArray,
validateWritableBufferArray,
validateCpOptions,
validateOffsetLengthRead,
validateOffsetLengthWrite,
Expand All @@ -141,7 +142,7 @@ const {
parseFileMode,
validateAbortSignal,
validateBoolean,
validateBuffer,
validateWritableBuffer,
validateEncoding,
validateFunction,
validateInteger,
Expand Down Expand Up @@ -886,7 +887,7 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
} = params ?? kEmptyObject);
}

validateBuffer(buffer);
validateWritableBuffer(buffer);
validateFunction(callback, 'cb');

if (offset == null) {
Expand Down Expand Up @@ -955,7 +956,7 @@ ObjectDefineProperty(read, kCustomPromisifyArgsSymbol,
* @returns {number}
*/
function readSync(fd, buffer, offsetOrOptions, length, position) {
validateBuffer(buffer);
validateWritableBuffer(buffer);

let offset = offsetOrOptions;
if (arguments.length <= 3 || typeof offsetOrOptions === 'object') {
Expand Down Expand Up @@ -1023,7 +1024,7 @@ function readv(fd, buffers, position, callback) {
}

fd = getValidatedFd(fd);
validateBufferArray(buffers);
validateWritableBufferArray(buffers);
callback ||= position;
validateFunction(callback, 'cb');

Expand Down Expand Up @@ -1059,7 +1060,7 @@ ObjectDefineProperty(readv, kCustomPromisifyArgsSymbol,
* @returns {number}
*/
function readvSync(fd, buffers, position) {
validateBufferArray(buffers);
validateWritableBufferArray(buffers);

if (typeof position !== 'number')
position = null;
Expand Down
9 changes: 5 additions & 4 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ const {
toUnixTimestamp,
handleErrorFromBinding: handleSyncErrorFromBinding,
validateBufferArray,
validateWritableBufferArray,
validateCpOptions,
validateOffsetLengthRead,
validateOffsetLengthWrite,
Expand All @@ -94,7 +95,7 @@ const {
parseFileMode,
validateAbortSignal,
validateBoolean,
validateBuffer,
validateWritableBuffer,
validateEncoding,
validateInteger,
validateObject,
Expand Down Expand Up @@ -1442,10 +1443,10 @@ async function read(handle, bufferOrParams, offset, length, position) {
length = buffer.byteLength - offset,
position = null,
} = bufferOrParams ?? kEmptyObject);

validateBuffer(buffer);
}

validateWritableBuffer(buffer);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bikeshed: I'd prefer validateMutableBuffer but non-blocking.


if (offset !== null && typeof offset === 'object') {
// This is fh.read(buffer, options)
({
Expand Down Expand Up @@ -1490,7 +1491,7 @@ async function read(handle, bufferOrParams, offset, length, position) {
}

async function readv(handle, buffers, position) {
validateBufferArray(buffers);
validateWritableBufferArray(buffers);

if (typeof position !== 'number')
position = null;
Expand Down
26 changes: 20 additions & 6 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,12 @@ const { toPathIfFileURL } = require('internal/url');
const {
validateAbortSignal,
validateBoolean,
validateBuffer,
validateFunction,
validateInt32,
validateInteger,
validateObject,
validateUint32,
validateWritableBuffer,
} = require('internal/validators');
const pathModule = require('path');
const binding = internalBinding('fs');
Expand Down Expand Up @@ -962,6 +962,17 @@ const validateBufferArray = hideStackFrames((buffers, propName = 'buffers') => {
return buffers;
});

// Like validateBufferArray(), for buffers that are going to be written to.
const validateWritableBufferArray = hideStackFrames((buffers, propName = 'buffers') => {
validateBufferArray.withoutStackTrace(buffers, propName);

for (let i = 0; i < buffers.length; i++) {
validateWritableBuffer.withoutStackTrace(buffers[i], `${propName}[${i}]`);
}

return buffers;
});

let nonPortableTemplateWarn = true;

function warnOnNonPortableTemplate(template) {
Expand Down Expand Up @@ -1139,15 +1150,17 @@ function getReadFileBufferByteLengthName(options) {
const getReadFileBuffer = hideStackFrames((options, size) => {
let { buffer } = options;

if (typeof buffer === 'function') {
buffer = options.buffer(size);
validateBuffer.withoutStackTrace(buffer, 'options.buffer()');
}

if (buffer === undefined) {
return undefined;
}

if (typeof buffer === 'function') {
buffer = options.buffer(size);
validateWritableBuffer.withoutStackTrace(buffer, 'options.buffer()');
} else {
validateWritableBuffer.withoutStackTrace(buffer, 'options.buffer');
}

if (!BufferIsBuffer(buffer)) {
buffer = Buffer.from(buffer.buffer, buffer.byteOffset, buffer.byteLength);
}
Expand Down Expand Up @@ -1223,6 +1236,7 @@ module.exports = {
Stats: deprecate(Stats, 'fs.Stats constructor is deprecated.', 'DEP0180'),
toUnixTimestamp,
validateBufferArray,
validateWritableBufferArray,
validateCpOptions,
validateOffsetLengthRead,
validateOffsetLengthWrite,
Expand Down
22 changes: 22 additions & 0 deletions lib/internal/validators.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const {
isRegExp,
} = require('internal/util/types');
const { signals } = internalBinding('constants').os;
const { isImmutable: isImmutableArrayBufferView } = internalBinding('buffer');

/**
* @param {*} value
Expand Down Expand Up @@ -411,6 +412,26 @@ const validateBuffer = hideStackFrames((buffer, name = 'buffer') => {
}
});

/**
* @callback validateWritableBuffer
* @param {*} buffer
* @param {string} [name='buffer']
* @returns {asserts buffer is ArrayBufferView}
*/

/**
* Validates an ArrayBufferView that is going to be written to. A view backed
* by an immutable ArrayBuffer is rejected, since its bytes cannot be changed.
* @type {validateWritableBuffer}
*/
const validateWritableBuffer = hideStackFrames((buffer, name = 'buffer') => {
validateBuffer.withoutStackTrace(buffer, name);
if (isImmutableArrayBufferView(buffer)) {
throw new ERR_INVALID_ARG_VALUE(
name, buffer, 'is backed by an immutable ArrayBuffer and cannot be written');
}
});

/**
* @param {string} data
* @param {string} encoding
Expand Down Expand Up @@ -668,6 +689,7 @@ module.exports = {
validateAbortSignalArray,
validateBoolean,
validateBuffer,
validateWritableBuffer,
validateDictionary,
validateEncoding,
validateFunction,
Expand Down
25 changes: 25 additions & 0 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,27 @@ int32_t FastCompare(Local<Value>,

static CFunction fast_compare(CFunction::Make(FastCompare));

bool IsImmutableImpl(Local<Value> view) {
return view.As<ArrayBufferView>()->Buffer()->IsImmutable();
}
Comment on lines +992 to +994

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool IsImmutableImpl(Local<Value> view) {
return view.As<ArrayBufferView>()->Buffer()->IsImmutable();
}
bool IsImmutableImpl(Local<Value> value) {
Local<ArrayBufferView> abv = value.As<ArrayBufferView>();
if (!abv->HasBuffer()) {
return false;
}
return abv->Buffer()->IsImmutable();
}

Buffer() has side-effects if a typed array was created with on-heap storage.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should likely also at least DCHECK that view->IsArrayBufferView()

@jasnell jasnell Sep 29, 2026 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, given how this is used, I think you actually need a tri-state return.

For the IsImmutableImpl, return an std::optional<bool>.

std::nullopt if view is not an ArrayBufferView.

false if view is not immutable.

true if view is immutable.

Then at the callsites, std::nullopt should map to null

(that's a bit pedantic tho so I'll leave it up to you... at a minimum the DCHECK)


void SlowIsImmutable(const FunctionCallbackInfo<Value>& args) {
CHECK(args[0]->IsArrayBufferView());
args.GetReturnValue().Set(IsImmutableImpl(args[0]));
}

bool FastIsImmutable(Local<Value>,
Local<Value> view,
// NOLINTNEXTLINE(runtime/references)
FastApiCallbackOptions& options) {
TRACK_V8_FAST_API_CALL("buffer.isImmutable");
HandleScope scope(options.isolate);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this really need a handle scope?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not


return IsImmutableImpl(view);
}

static CFunction fast_is_immutable(CFunction::Make(FastIsImmutable));

// Computes the offset for starting an indexOf or lastIndexOf search.
// Returns either a valid offset in [0...<length - 1>], ie inside the Buffer,
// or -1 to signal that there is no possible match.
Expand Down Expand Up @@ -1927,6 +1948,8 @@ void Initialize(Local<Object> target,
&fast_byte_length_utf8);
SetFastMethod(context, target, "copy", SlowCopy, &fast_copy);
SetFastMethodNoSideEffect(context, target, "compare", Compare, &fast_compare);
SetFastMethodNoSideEffect(
context, target, "isImmutable", SlowIsImmutable, &fast_is_immutable);
SetMethodNoSideEffect(context, target, "compareOffset", CompareOffset);
SetMethod(context, target, "fill", Fill);
SetMethodNoSideEffect(context, target, "indexOfBuffer", IndexOfBuffer);
Expand Down Expand Up @@ -2014,6 +2037,8 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(fast_copy);
registry->Register(Compare);
registry->Register(fast_compare);
registry->Register(SlowIsImmutable);
registry->Register(fast_is_immutable);
registry->Register(CompareOffset);
registry->Register(Fill);
registry->Register(IndexOfBuffer);
Expand Down
65 changes: 65 additions & 0 deletions test/parallel/test-buffer-from-immutable.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Flags: --js-immutable-arraybuffer
'use strict';

require('../common');
const assert = require('assert');

function immutable(bytes) {
const buffer = Uint8Array.from(bytes).buffer.transferToImmutable();
assert.strictEqual(buffer.immutable, true);
return new Uint8Array(buffer);
}

function check(actual, expected) {
assert(Buffer.isBuffer(actual));
assert.deepStrictEqual([...actual], expected);
assert.strictEqual(actual.buffer.immutable, false);
actual.fill(0);
}

{
const source = immutable([1, 2, 3, 4]);
check(Buffer.from(source), [1, 2, 3, 4]);
check(Buffer.from(source.subarray(1, 3)), [2, 3]);
check(Buffer.from(Buffer.from(source.buffer)), [1, 2, 3, 4]);
assert.deepStrictEqual([...source], [1, 2, 3, 4]);
}

{
// Larger than half the pool size, so the copy is not pooled.
const bytes = Array.from({ length: Buffer.poolSize }, (_, i) => i & 0xff);
const source = immutable(bytes);
check(Buffer.from(source), bytes);
check(Buffer.from(source.subarray(1)), bytes.slice(1));
}

{
const source = immutable([1, 2, 3, 4]);
check(Buffer.copyBytesFrom(source), [1, 2, 3, 4]);
check(Buffer.copyBytesFrom(source, 1, 2), [2, 3]);
const wide = new Uint16Array(Uint16Array.from([0x0102, 0x0304]).buffer.transferToImmutable());
check(Buffer.copyBytesFrom(wide, 1), [...new Uint8Array(Uint16Array.from([0x0304]).buffer)]);
}

{
const a = immutable([1, 2]);
const b = Buffer.from([3, 4]);
const c = immutable([5, 6]);
check(Buffer.concat([a, b, c]), [1, 2, 3, 4, 5, 6]);
check(Buffer.concat([a, b, c], 6), [1, 2, 3, 4, 5, 6]);
// The last element is cut short.
check(Buffer.concat([a, b, c], 5), [1, 2, 3, 4, 5]);
// The result is longer than the sum of the elements and gets zero-filled.
check(Buffer.concat([a, c], 5), [1, 2, 5, 6, 0]);
check(Buffer.concat([a.subarray(1), c.subarray(0, 1)]), [2, 5]);
assert.deepStrictEqual([...a], [1, 2]);
assert.deepStrictEqual([...c], [5, 6]);
}

{
// The other direction is unchanged: the copy must not write into a view
// backed by an immutable ArrayBuffer.
const target = immutable([9, 9, 9, 9]);
assert.strictEqual(Buffer.from([1, 2, 3, 4]).copy(target), 0);
assert.deepStrictEqual([...target], [9, 9, 9, 9]);
}
Loading
Loading