Skip to content
Open
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
2 changes: 1 addition & 1 deletion benchmark/buffers/buffer-bytelength-string.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const common = require('../common');
const bench = common.createBenchmark(main, {
type: ['one_byte', 'two_bytes', 'three_bytes',
'four_bytes', 'latin1'],
encoding: ['utf8', 'base64'],
encoding: ['utf8', 'base64', 'latin1', 'hex'],
repeat: [1, 2, 16, 256], // x16
n: [4e6],
});
Expand Down
31 changes: 31 additions & 0 deletions benchmark/buffers/buffer-copy-bytes-from.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use strict';

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

const bench = common.createBenchmark(main, {
type: ['Uint8Array', 'Uint16Array', 'Uint32Array', 'Float64Array'],
len: [64, 256, 2048],
partial: ['none', 'offset', 'offset-length'],
n: [6e5],
});

function main({ n, len, type, partial }) {
const TypedArrayCtor = globalThis[type];
const src = new TypedArrayCtor(len);
for (let i = 0; i < len; i++) src[i] = i;

let offset;
let length;
if (partial === 'offset') {
offset = len >>> 2;
} else if (partial === 'offset-length') {
offset = len >>> 2;
length = len >>> 1;
}

bench.start();
for (let i = 0; i < n; i++) {
Buffer.copyBytesFrom(src, offset, length);
}
bench.end(n);
}
1 change: 1 addition & 0 deletions benchmark/buffers/buffer-fill.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const bench = common.createBenchmark(main, {
'fill("t")',
'fill("test")',
'fill("t", "utf8")',
'fill("t", "ascii")',
'fill("t", 0, "utf8")',
'fill("t", 0)',
'fill(Buffer.alloc(1), 0)',
Expand Down
2 changes: 1 addition & 1 deletion benchmark/buffers/buffer-indexof.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const searchStrings = [

const bench = common.createBenchmark(main, {
search: searchStrings,
encoding: ['undefined', 'utf8', 'ucs2'],
encoding: ['undefined', 'utf8', 'ascii', 'latin1', 'ucs2'],
type: ['buffer', 'string'],
n: [5e4],
}, {
Expand Down
2 changes: 1 addition & 1 deletion benchmark/buffers/buffer-tostring.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const common = require('../common.js');

const bench = common.createBenchmark(main, {
encoding: ['', 'utf8', 'ascii', 'latin1', 'hex', 'UCS-2'],
encoding: ['', 'utf8', 'ascii', 'latin1', 'hex', 'base64', 'base64url', 'UCS-2'],
args: [0, 1, 3],
len: [1, 64, 1024],
n: [1e6],
Expand Down
87 changes: 55 additions & 32 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ const {
TypedArrayPrototypeGetByteOffset,
TypedArrayPrototypeGetLength,
TypedArrayPrototypeSet,
TypedArrayPrototypeSlice,
TypedArrayPrototypeSubarray,
Uint8Array,
} = primordials;
Expand Down Expand Up @@ -383,28 +382,42 @@ Buffer.copyBytesFrom = function copyBytesFrom(view, offset, length) {
return new FastBuffer();
}

const byteLength = TypedArrayPrototypeGetByteLength(view);

if (offset !== undefined || length !== undefined) {
if (offset !== undefined) {
validateInteger(offset, 'offset', 0);
if (offset >= viewLength) return new FastBuffer();
Comment on lines +385 to 390
Copy link
Contributor

Choose a reason for hiding this comment

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

In case we return here, we have called TypedArrayPrototypeGetByteLength for nothing. Can we move it further down?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call, let me flatten the structure currently it's quite nested to accomedate this

} else {
offset = 0;
}

let end;
if (length !== undefined) {
validateInteger(length, 'length', 0);
end = offset + length;
// The old code used TypedArrayPrototypeSlice which clamps internally.
end = MathMin(offset + length, viewLength);
} else {
end = viewLength;
}

view = TypedArrayPrototypeSlice(view, offset, end);
if (end <= offset) return new FastBuffer();

const elementSize = byteLength / viewLength;
const srcByteOffset = TypedArrayPrototypeGetByteOffset(view) +
offset * elementSize;
const srcByteLength = (end - offset) * elementSize;

return fromArrayLike(new Uint8Array(
TypedArrayPrototypeGetBuffer(view),
srcByteOffset,
srcByteLength));
}

return fromArrayLike(new Uint8Array(
TypedArrayPrototypeGetBuffer(view),
TypedArrayPrototypeGetByteOffset(view),
TypedArrayPrototypeGetByteLength(view)));
byteLength));
};

// Identical to the built-in %TypedArray%.of(), but avoids using the deprecated
Expand Down Expand Up @@ -551,14 +564,15 @@ function fromArrayBuffer(obj, byteOffset, length) {
}

function fromArrayLike(obj) {
if (obj.length <= 0)
const len = obj.length;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

Suggested change
const len = obj.length;
const { length } = obj;

if (len <= 0)
return new FastBuffer();
if (obj.length < (Buffer.poolSize >>> 1)) {
if (obj.length > (poolSize - poolOffset))
if (len < (Buffer.poolSize >>> 1)) {
if (len > (poolSize - poolOffset))
createPool();
const b = new FastBuffer(allocPool, poolOffset, obj.length);
const b = new FastBuffer(allocPool, poolOffset, len);
TypedArrayPrototypeSet(b, obj, 0);
poolOffset += obj.length;
poolOffset += len;
alignPool();
return b;
}
Expand Down Expand Up @@ -732,11 +746,7 @@ const encodingOps = {
write: asciiWrite,
slice: asciiSlice,
indexOf: (buf, val, byteOffset, dir) =>
indexOfBuffer(buf,
fromStringFast(val, encodingOps.ascii),
byteOffset,
encodingsMap.ascii,
dir),
indexOfString(buf, val, byteOffset, encodingsMap.ascii, dir),
},
base64: {
encoding: 'base64',
Expand Down Expand Up @@ -1118,7 +1128,9 @@ function _fill(buf, value, offset, end, encoding) {
value = 0;
} else if (value.length === 1) {
// Fast path: If `value` fits into a single byte, use that numeric value.
if (normalizedEncoding === 'utf8') {
// ASCII shares this branch with utf8 since code < 128 covers the full
// ASCII range; anything outside falls through to C++ bindingFill.
if (normalizedEncoding === 'utf8' || normalizedEncoding === 'ascii') {
Copy link
Member

Choose a reason for hiding this comment

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

Currently, ascii behaves exactly like latin1
Unsure if by design or accidentally

Copy link
Contributor Author

@thisalihassan thisalihassan Feb 18, 2026

Choose a reason for hiding this comment

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

yes, this is safe by design I am just extending the existing single byte numeric optimization to cover ASCII, since the guard already constrains it to the valid ASCII range.

const code = StringPrototypeCharCodeAt(value, 0);
if (code < 128) {
value = code;
Expand Down Expand Up @@ -1168,29 +1180,30 @@ function _fill(buf, value, offset, end, encoding) {
}

Buffer.prototype.write = function write(string, offset, length, encoding) {
const len = this.length;
// Buffer#write(string);
if (offset === undefined) {
return utf8Write(this, string, 0, this.length);
return utf8Write(this, string, 0, len);
}
// Buffer#write(string, encoding)
if (length === undefined && typeof offset === 'string') {
encoding = offset;
length = this.length;
length = len;
offset = 0;

// Buffer#write(string, offset[, length][, encoding])
} else {
validateOffset(offset, 'offset', 0, this.length);
validateOffset(offset, 'offset', 0, len);

const remaining = this.length - offset;
const remaining = len - offset;

if (length === undefined) {
length = remaining;
} else if (typeof length === 'string') {
encoding = length;
length = remaining;
} else {
validateOffset(length, 'length', 0, this.length);
validateOffset(length, 'length', 0, len);
if (length > remaining)
length = remaining;
}
Expand All @@ -1208,9 +1221,10 @@ Buffer.prototype.write = function write(string, offset, length, encoding) {
};

Buffer.prototype.toJSON = function toJSON() {
if (this.length > 0) {
const data = new Array(this.length);
for (let i = 0; i < this.length; ++i)
const len = this.length;
if (len > 0) {
const data = new Array(len);
for (let i = 0; i < len; ++i)
data[i] = this[i];
return { type: 'Buffer', data };
}
Expand Down Expand Up @@ -1253,45 +1267,53 @@ function swap(b, n, m) {
}

Buffer.prototype.swap16 = function swap16() {
// For Buffer.length < 128, it's generally faster to
// For Buffer.length <= 32, it's generally faster to
// do the swap in javascript. For larger buffers,
// dropping down to the native code is faster.
if (!isUint8Array(this))
throw new ERR_INVALID_ARG_TYPE('this', ['Buffer', 'Uint8Array'], this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw new ERR_INVALID_ARG_TYPE('this', ['Buffer', 'Uint8Array'], this);
throw new ERR_INVALID_THIS('Uint8Array');

Copy link
Contributor Author

@thisalihassan thisalihassan Mar 11, 2026

Choose a reason for hiding this comment

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

@aduh95 I believe ERR_INVALID_ARG_TYPE gives better error message, but if you think ERR_INVALID_THIS follows the spec better happy to align with the change

=== swap16 (ERR_INVALID_ARG_TYPE) ===
code: ERR_INVALID_ARG_TYPE
message: The "this" argument must be an instance of Buffer or Uint8Array. Received an instance of Float32Array

=== swap32 (ERR_INVALID_THIS) ===
code: ERR_INVALID_THIS
message: Value of "this" must be of type Uint8Array

Copy link
Contributor

Choose a reason for hiding this comment

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

Overriding the this value is not really supported on Node.js APIs, so we don't really care about the error message. ERR_INVALID_ARG_TYPE is not suited, as this is not an argument. My preference would be to go with #61871 (comment), if not to use ERR_INVALID_THIS

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh then I think TypedArrayPrototypeGetLength might be a bit confusing as it doesn't block Float32Array
Buffer.prototype.swap32.call(new Float32Array(4));

it's a TypedArray check, not a Uint8Array check

what do you think @aduh95? check #61861_comment_by_ChaLker

const len = this.length;
Comment on lines +1273 to 1275
Copy link
Contributor

@aduh95 aduh95 Mar 11, 2026

Choose a reason for hiding this comment

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

Alternatively, we can use the primordials to get the brand check for free

Suggested change
if (!isUint8Array(this))
throw new ERR_INVALID_ARG_TYPE('this', ['Buffer', 'Uint8Array'], this);
const len = this.length;
const len = TypedArrayPrototypeGetLength(this);

Or do we want TypedArrayPrototypeGetByteLength here?

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, we can use the primordials to get the brand check for free

This would result in an unhelpful error.

TypeError: Method get TypedArray.prototype.length called on incompatible receiver

Copy link
Contributor

Choose a reason for hiding this comment

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

How is that unhelpful? The full error message would be TypeError: Method get TypedArray.prototype.length called on incompatible receiver [object Array] btw

Copy link
Contributor Author

@thisalihassan thisalihassan Mar 11, 2026

Choose a reason for hiding this comment

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

Yup here's all three error messages:

=== swap16 (ERR_INVALID_ARG_TYPE) ===
code: ERR_INVALID_ARG_TYPE
message: The "this" argument must be an instance of Buffer or Uint8Array. Received an instance of Float32Array

=== swap32 (ERR_INVALID_THIS) ===
code: ERR_INVALID_THIS
message: Value of "this" must be of type Uint8Array

=== swap64 ===
code: undefined
message: Method get TypedArray.prototype.length called on incompatible receiver [object Array]

if (len % 2 !== 0)
throw new ERR_INVALID_BUFFER_SIZE('16-bits');
if (len < 128) {
if (len <= 32) {
for (let i = 0; i < len; i += 2)
swap(this, i, i + 1);
return this;
}
return _swap16(this);
_swap16(this);
Copy link
Member

@ChALkeR ChALkeR Mar 5, 2026

Choose a reason for hiding this comment

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

what would happen if it's called on not a TypeArrayView now?

Copy link
Member

Choose a reason for hiding this comment

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

i.e. Buffer.prototype.swap16.apply(new Array(128))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I will add a JS side guard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would that be good approach @ChALkeR ?

Copy link
Member

Choose a reason for hiding this comment

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

It changes the method to not being callable on a typedarray that is not an uint8array, but the previous behavior was inconsistent in that case anyway as the js part swapped elements and the src part swapped bytes

So I think explicitly blocking non-uint8arr should be fine and not a semver-major?
(perhaps cc @nodejs/lts just in case)

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently node -e 'Buffer.prototype.swap16.apply(new Float32Array(128))' does not throw. If we add a check for Uint8Array, that's a potential breaking change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aduh95 actually the point is it should always be thrown error on TypedArray because the current behavior silently corrupts data on the C++ path and on the JS loop path it swaps float elements instead of bytes. So a clean error is strictly better than silent data corruption. this is what @ChALkeR pointed

const f = new Float32Array(128);
f[0] = 1.5;                          // bytes: 00 00 C0 3F
Buffer.prototype.swap16.call(f);
console.log(f[0]);                    // -2.984375 (bytes: 00 00 3F C0)

return this;
};

Buffer.prototype.swap32 = function swap32() {
// For Buffer.length < 192, it's generally faster to
// For Buffer.length <= 32, it's generally faster to
// do the swap in javascript. For larger buffers,
// dropping down to the native code is faster.
if (!isUint8Array(this))
throw new ERR_INVALID_ARG_TYPE('this', ['Buffer', 'Uint8Array'], this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw new ERR_INVALID_ARG_TYPE('this', ['Buffer', 'Uint8Array'], this);
throw new ERR_INVALID_THIS('Uint8Array');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above

const len = this.length;
if (len % 4 !== 0)
throw new ERR_INVALID_BUFFER_SIZE('32-bits');
if (len < 192) {
if (len <= 32) {
for (let i = 0; i < len; i += 4) {
swap(this, i, i + 3);
swap(this, i + 1, i + 2);
}
return this;
}
return _swap32(this);
_swap32(this);
return this;
};

Buffer.prototype.swap64 = function swap64() {
// For Buffer.length < 192, it's generally faster to
// For Buffer.length < 48, it's generally faster to
// do the swap in javascript. For larger buffers,
// dropping down to the native code is faster.
if (!isUint8Array(this))
throw new ERR_INVALID_ARG_TYPE('this', ['Buffer', 'Uint8Array'], this);
const len = this.length;
if (len % 8 !== 0)
throw new ERR_INVALID_BUFFER_SIZE('64-bits');
if (len < 192) {
if (len < 48) {
for (let i = 0; i < len; i += 8) {
swap(this, i, i + 7);
swap(this, i + 1, i + 6);
Expand All @@ -1300,7 +1322,8 @@ Buffer.prototype.swap64 = function swap64() {
}
return this;
}
return _swap64(this);
_swap64(this);
return this;
};

Buffer.prototype.toLocaleString = Buffer.prototype.toString;
Expand Down
57 changes: 44 additions & 13 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,7 @@ void IndexOfString(const FunctionCallbackInfo<Value>& args) {
needle_length,
offset,
is_forward);
} else if (enc == LATIN1) {
} else if (enc == ASCII || enc == LATIN1) {
uint8_t* needle_data = node::UncheckedMalloc<uint8_t>(needle_length);
if (needle_data == nullptr) {
return args.GetReturnValue().Set(-1);
Expand Down Expand Up @@ -1200,31 +1200,59 @@ int32_t FastIndexOfNumber(Local<Value>,
static CFunction fast_index_of_number(CFunction::Make(FastIndexOfNumber));

void Swap16(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
DCHECK(args[0]->IsArrayBufferView());
SPREAD_BUFFER_ARG(args[0], ts_obj);
CHECK(nbytes::SwapBytes16(ts_obj_data, ts_obj_length));
args.GetReturnValue().Set(args[0]);
}

void FastSwap16(Local<Value> receiver,
Local<Value> buffer_obj,
// NOLINTNEXTLINE(runtime/references)
FastApiCallbackOptions& options) {
TRACK_V8_FAST_API_CALL("buffer.swap16");
HandleScope scope(options.isolate);
SPREAD_BUFFER_ARG(buffer_obj, ts_obj);
CHECK(nbytes::SwapBytes16(ts_obj_data, ts_obj_length));
}
Comment on lines +1208 to +1216
Copy link
Member

Choose a reason for hiding this comment

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

These fast callbacks are non-identical to the conventional callbacks they shadow.

  • The existing callbacks validate their argument and throws to JS if invalid, whereas your fast callbacks hard-crash the process. It might be better to validate in the JS layer, then use the same unwrapping logic on both sides.
  • Your fast callback cannot have a different return convention to the conventional callback. You will need to remove the return value from the conventional callback.

Copy link
Member

@ChALkeR ChALkeR Mar 5, 2026

Choose a reason for hiding this comment

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

Was removing the validation instead of moving it to js intentional?

Also fast paths can keep the validation and fall back to slow i think, so we can still validate on the src side?

Copy link
Member

Choose a reason for hiding this comment

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

Also fast paths can keep the validation and fall back to slow i think, so we can still validate on the src side?

This is outdated, the fast API doesn't use fallback any more (since Node.js v23.x).

However, any validation in a JS wrapper should be shadowed by a CHECK or DCHECK in the C++ binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPREAD_BUFFER_ARG already contains CHECK((val)->IsArrayBufferView()) on all Slow & Fast swap methods

Comment on lines +1208 to +1216
Copy link
Member

Choose a reason for hiding this comment

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

Fast callbacks should include debug tracking and call tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for sharing these I will update the code


static CFunction fast_swap16(CFunction::Make(FastSwap16));

void Swap32(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
DCHECK(args[0]->IsArrayBufferView());
SPREAD_BUFFER_ARG(args[0], ts_obj);
CHECK(nbytes::SwapBytes32(ts_obj_data, ts_obj_length));
args.GetReturnValue().Set(args[0]);
}

void FastSwap32(Local<Value> receiver,
Local<Value> buffer_obj,
// NOLINTNEXTLINE(runtime/references)
FastApiCallbackOptions& options) {
TRACK_V8_FAST_API_CALL("buffer.swap32");
HandleScope scope(options.isolate);
SPREAD_BUFFER_ARG(buffer_obj, ts_obj);
CHECK(nbytes::SwapBytes32(ts_obj_data, ts_obj_length));
}

static CFunction fast_swap32(CFunction::Make(FastSwap32));

void Swap64(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
DCHECK(args[0]->IsArrayBufferView());
SPREAD_BUFFER_ARG(args[0], ts_obj);
CHECK(nbytes::SwapBytes64(ts_obj_data, ts_obj_length));
args.GetReturnValue().Set(args[0]);
}

void FastSwap64(Local<Value> receiver,
Local<Value> buffer_obj,
// NOLINTNEXTLINE(runtime/references)
FastApiCallbackOptions& options) {
TRACK_V8_FAST_API_CALL("buffer.swap64");
HandleScope scope(options.isolate);
SPREAD_BUFFER_ARG(buffer_obj, ts_obj);
CHECK(nbytes::SwapBytes64(ts_obj_data, ts_obj_length));
}

static CFunction fast_swap64(CFunction::Make(FastSwap64));

static void IsUtf8(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
CHECK_EQ(args.Length(), 1);
Expand Down Expand Up @@ -1622,9 +1650,9 @@ void Initialize(Local<Object> target,
SetMethodNoSideEffect(
context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer);

SetMethod(context, target, "swap16", Swap16);
SetMethod(context, target, "swap32", Swap32);
SetMethod(context, target, "swap64", Swap64);
SetFastMethod(context, target, "swap16", Swap16, &fast_swap16);
SetFastMethod(context, target, "swap32", Swap32, &fast_swap32);
SetFastMethod(context, target, "swap64", Swap64, &fast_swap64);

SetMethodNoSideEffect(context, target, "isUtf8", IsUtf8);
SetMethodNoSideEffect(context, target, "isAscii", IsAscii);
Expand Down Expand Up @@ -1693,8 +1721,11 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(IndexOfString);

registry->Register(Swap16);
registry->Register(fast_swap16);
registry->Register(Swap32);
registry->Register(fast_swap32);
registry->Register(Swap64);
registry->Register(fast_swap64);

registry->Register(IsUtf8);
registry->Register(IsAscii);
Expand Down
Loading
Loading