Skip to content

Commit e5d3795

Browse files
mertcanaltinaduh95
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 7893640 commit e5d3795

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
Uint8ArrayPrototype,
5657
} = primordials;
@@ -626,25 +627,55 @@ Buffer.concat = function concat(list, length) {
626627
if (length === undefined) {
627628
length = 0;
628629
for (let i = 0; i < list.length; i++) {
629-
if (list[i].length) {
630-
length += list[i].length;
630+
const buf = list[i];
631+
if (!isUint8Array(buf)) {
632+
// TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
633+
// Instead, find the proper error code for this.
634+
throw new ERR_INVALID_ARG_TYPE(
635+
`list[${i}]`, ['Buffer', 'Uint8Array'], buf);
631636
}
637+
length += TypedArrayPrototypeGetByteLength(buf);
632638
}
633-
} else {
634-
validateOffset(length, 'length');
639+
640+
const buffer = allocate(length);
641+
let pos = 0;
642+
for (let i = 0; i < list.length; i++) {
643+
const buf = list[i];
644+
const bufLength = TypedArrayPrototypeGetByteLength(buf);
645+
TypedArrayPrototypeSet(buffer, buf, pos);
646+
pos += bufLength;
647+
}
648+
649+
if (pos < length) {
650+
TypedArrayPrototypeFill(buffer, 0, pos, length);
651+
}
652+
return buffer;
635653
}
636654

637-
const buffer = Buffer.allocUnsafe(length);
638-
let pos = 0;
655+
validateOffset(length, 'length');
639656
for (let i = 0; i < list.length; i++) {
640-
const buf = list[i];
641-
if (!isUint8Array(buf)) {
657+
if (!isUint8Array(list[i])) {
642658
// TODO(BridgeAR): This should not be of type ERR_INVALID_ARG_TYPE.
643659
// Instead, find the proper error code for this.
644660
throw new ERR_INVALID_ARG_TYPE(
645661
`list[${i}]`, ['Buffer', 'Uint8Array'], list[i]);
646662
}
647-
pos += _copyActual(buf, buffer, pos, 0, buf.length, true);
663+
}
664+
665+
const buffer = allocate(length);
666+
let pos = 0;
667+
for (let i = 0; i < list.length; i++) {
668+
const buf = list[i];
669+
const bufLength = TypedArrayPrototypeGetByteLength(buf);
670+
if (pos + bufLength > length) {
671+
TypedArrayPrototypeSet(buffer,
672+
TypedArrayPrototypeSubarray(buf, 0, length - pos),
673+
pos);
674+
pos = length;
675+
break;
676+
}
677+
TypedArrayPrototypeSet(buffer, buf, pos);
678+
pos += bufLength;
648679
}
649680

650681
// 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)