Skip to content

Commit 8bd68db

Browse files
mertcanaltinjuanarbol
authored andcommitted
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 cc723fa commit 8bd68db

2 files changed

Lines changed: 50 additions & 9 deletions

File tree

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
Uint8ArrayPrototype,
5657
} = primordials;
@@ -588,25 +589,55 @@ Buffer.concat = function concat(list, length) {
588589
if (length === undefined) {
589590
length = 0;
590591
for (let i = 0; i < list.length; i++) {
591-
if (list[i].length) {
592-
length += list[i].length;
592+
const buf = list[i];
593+
if (!isUint8Array(buf)) {
594+
// TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
595+
// Instead, find the proper error code for this.
596+
throw new ERR_INVALID_ARG_TYPE(
597+
`list[${i}]`, ['Buffer', 'Uint8Array'], buf);
593598
}
599+
length += TypedArrayPrototypeGetByteLength(buf);
594600
}
595-
} else {
596-
validateOffset(length, 'length');
601+
602+
const buffer = allocate(length);
603+
let pos = 0;
604+
for (let i = 0; i < list.length; i++) {
605+
const buf = list[i];
606+
const bufLength = TypedArrayPrototypeGetByteLength(buf);
607+
TypedArrayPrototypeSet(buffer, buf, pos);
608+
pos += bufLength;
609+
}
610+
611+
if (pos < length) {
612+
TypedArrayPrototypeFill(buffer, 0, pos, length);
613+
}
614+
return buffer;
597615
}
598616

599-
const buffer = Buffer.allocUnsafe(length);
600-
let pos = 0;
617+
validateOffset(length, 'length');
601618
for (let i = 0; i < list.length; i++) {
602-
const buf = list[i];
603-
if (!isUint8Array(buf)) {
619+
if (!isUint8Array(list[i])) {
604620
// TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
605621
// Instead, find the proper error code for this.
606622
throw new ERR_INVALID_ARG_TYPE(
607623
`list[${i}]`, ['Buffer', 'Uint8Array'], list[i]);
608624
}
609-
pos += _copyActual(buf, buffer, pos, 0, buf.length, true);
625+
}
626+
627+
const buffer = allocate(length);
628+
let pos = 0;
629+
for (let i = 0; i < list.length; i++) {
630+
const buf = list[i];
631+
const bufLength = TypedArrayPrototypeGetByteLength(buf);
632+
if (pos + bufLength > length) {
633+
TypedArrayPrototypeSet(buffer,
634+
TypedArrayPrototypeSubarray(buf, 0, length - pos),
635+
pos);
636+
pos = length;
637+
break;
638+
}
639+
TypedArrayPrototypeSet(buffer, buf, pos);
640+
pos += bufLength;
610641
}
611642

612643
// 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
@@ -106,3 +106,13 @@ assert.deepStrictEqual(
106106
assert.deepStrictEqual(Buffer.concat([new Uint8Array([0x41, 0x42]),
107107
new Uint8Array([0x43, 0x44])]),
108108
Buffer.from('ABCD'));
109+
110+
// Spoofed length getter should not cause uninitialized memory exposure
111+
{
112+
const u8_1 = new Uint8Array([1, 2, 3, 4]);
113+
const u8_2 = new Uint8Array([5, 6, 7, 8]);
114+
Object.defineProperty(u8_1, 'length', { get() { return 100; } });
115+
const buf = Buffer.concat([u8_1, u8_2]);
116+
assert.strictEqual(buf.length, 8);
117+
assert.deepStrictEqual(buf, Buffer.from([1, 2, 3, 4, 5, 6, 7, 8]));
118+
}

0 commit comments

Comments
 (0)