Skip to content

Commit cc6c188

Browse files
authored
buffer: optimize buffer.concat performance
PR-URL: #61721 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk> Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent 89f4b6c commit cc6c188

File tree

2 files changed

+50
-9
lines changed

2 files changed

+50
-9
lines changed

lib/buffer.js

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ const {
5151
TypedArrayPrototypeGetLength,
5252
TypedArrayPrototypeSet,
5353
TypedArrayPrototypeSlice,
54+
TypedArrayPrototypeSubarray,
5455
Uint8Array,
5556
} = primordials;
5657

@@ -614,25 +615,55 @@ Buffer.concat = function concat(list, length) {
614615
if (length === undefined) {
615616
length = 0;
616617
for (let i = 0; i < list.length; i++) {
617-
if (list[i].length) {
618-
length += list[i].length;
618+
const buf = list[i];
619+
if (!isUint8Array(buf)) {
620+
// TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
621+
// Instead, find the proper error code for this.
622+
throw new ERR_INVALID_ARG_TYPE(
623+
`list[${i}]`, ['Buffer', 'Uint8Array'], buf);
619624
}
625+
length += TypedArrayPrototypeGetByteLength(buf);
620626
}
621-
} else {
622-
validateOffset(length, 'length');
627+
628+
const buffer = allocate(length);
629+
let pos = 0;
630+
for (let i = 0; i < list.length; i++) {
631+
const buf = list[i];
632+
const bufLength = TypedArrayPrototypeGetByteLength(buf);
633+
TypedArrayPrototypeSet(buffer, buf, pos);
634+
pos += bufLength;
635+
}
636+
637+
if (pos < length) {
638+
TypedArrayPrototypeFill(buffer, 0, pos, length);
639+
}
640+
return buffer;
623641
}
624642

625-
const buffer = Buffer.allocUnsafe(length);
626-
let pos = 0;
643+
validateOffset(length, 'length');
627644
for (let i = 0; i < list.length; i++) {
628-
const buf = list[i];
629-
if (!isUint8Array(buf)) {
645+
if (!isUint8Array(list[i])) {
630646
// TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
631647
// Instead, find the proper error code for this.
632648
throw new ERR_INVALID_ARG_TYPE(
633649
`list[${i}]`, ['Buffer', 'Uint8Array'], list[i]);
634650
}
635-
pos += _copyActual(buf, buffer, pos, 0, buf.length, true);
651+
}
652+
653+
const buffer = allocate(length);
654+
let pos = 0;
655+
for (let i = 0; i < list.length; i++) {
656+
const buf = list[i];
657+
const bufLength = TypedArrayPrototypeGetByteLength(buf);
658+
if (pos + bufLength > length) {
659+
TypedArrayPrototypeSet(buffer,
660+
TypedArrayPrototypeSubarray(buf, 0, length - pos),
661+
pos);
662+
pos = length;
663+
break;
664+
}
665+
TypedArrayPrototypeSet(buffer, buf, pos);
666+
pos += bufLength;
636667
}
637668

638669
// Note: `length` is always equal to `buffer.length` at this point

test/parallel/test-buffer-concat.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,3 +123,13 @@ assert.deepStrictEqual(
123123
assert.deepStrictEqual(Buffer.concat([new Uint8Array([0x41, 0x42]),
124124
new Uint8Array([0x43, 0x44])]),
125125
Buffer.from('ABCD'));
126+
127+
// Spoofed length getter should not cause uninitialized memory exposure
128+
{
129+
const u8_1 = new Uint8Array([1, 2, 3, 4]);
130+
const u8_2 = new Uint8Array([5, 6, 7, 8]);
131+
Object.defineProperty(u8_1, 'length', { get() { return 100; } });
132+
const buf = Buffer.concat([u8_1, u8_2]);
133+
assert.strictEqual(buf.length, 8);
134+
assert.deepStrictEqual(buf, Buffer.from([1, 2, 3, 4, 5, 6, 7, 8]));
135+
}

0 commit comments

Comments
 (0)