Skip to content

Commit 5ca703f

Browse files
committed
buffer: support copy/concat for >= 2^32 bytes
1 parent b76bcb3 commit 5ca703f

File tree

5 files changed

+68
-18
lines changed

5 files changed

+68
-18
lines changed

doc/api/buffer.md

+5
Original file line numberDiff line numberDiff line change
@@ -1752,6 +1752,11 @@ added: v0.1.90
17521752
Copies data from a region of `buf` to a region in `target`, even if the `target`
17531753
memory region overlaps with `buf`.
17541754

1755+
If `sourceEnd` exceeds `buf.bytesLength`, only the bytes up to
1756+
`buf.bytesLength` will be copied.
1757+
1758+
The source slice is truncated if it does not fit into the target slice.
1759+
17551760
[`TypedArray.prototype.set()`][] performs the same operation, and is available
17561761
for all TypedArrays, including Node.js `Buffer`s, although it takes
17571762
different function arguments.

lib/buffer.js

+38-15
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ const {
100100
inspect: utilInspect,
101101
} = require('internal/util/inspect');
102102

103+
const assert = require('internal/assert');
104+
103105
const {
104106
codes: {
105107
ERR_BUFFER_OUT_OF_BOUNDS,
@@ -205,6 +207,7 @@ function toInteger(n, defaultVal) {
205207
return defaultVal;
206208
}
207209

210+
208211
function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
209212
if (!ArrayBufferIsView(source))
210213
throw new ERR_INVALID_ARG_TYPE('source', ['Buffer', 'Uint8Array'], source);
@@ -235,27 +238,45 @@ function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
235238
throw new ERR_OUT_OF_RANGE('sourceEnd', '>= 0', sourceEnd);
236239
}
237240

241+
// Clamp sourceEnd to be inbounds of source buffer.
242+
// This is expected behavior and not to throw.
243+
sourceEnd = MathMin(sourceEnd, source.byteLength);
244+
238245
if (targetStart >= target.byteLength || sourceStart >= sourceEnd)
239246
return 0;
240247

241-
return _copyActual(source, target, targetStart, sourceStart, sourceEnd);
242-
}
243-
244-
function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
245-
if (sourceEnd - sourceStart > target.byteLength - targetStart)
246-
sourceEnd = sourceStart + target.byteLength - targetStart;
248+
// Assert source slice in bounds of source buffer.
249+
// 0 <= sourceStart < sourceEnd <= source.byteLength
250+
assert(0 <= sourceStart && sourceStart < sourceEnd && sourceEnd <= source.byteLength);
247251

248-
let nb = sourceEnd - sourceStart;
249-
const sourceLen = source.byteLength - sourceStart;
250-
if (nb > sourceLen)
251-
nb = sourceLen;
252+
// Assert target slice in bounds of target buffer.
253+
// 0 <= targetStart < target.byteLength
254+
assert(0 <= targetStart && targetStart < target.byteLength);
252255

253-
if (nb <= 0)
254-
return 0;
256+
// Truncate source so that its length doesn't exceed targets.
257+
// This is the expected behavior, not to throw.
258+
const copyLength = MathMin(sourceEnd - sourceStart, target.byteLength - targetStart);
259+
sourceEnd = sourceStart + copyLength;
255260

256-
_copy(source, target, targetStart, sourceStart, nb);
261+
return _copyActual(source, target, targetStart, sourceStart, sourceStart + copyLength);
262+
}
257263

258-
return nb;
264+
// Safely performs native copy from valid source slice to valid target slice.
265+
// - The source slice is not clamped to fit into the target slice. If it won't fit, this throws.
266+
// - If either the source or target slice are out of bounds, this throws.
267+
function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
268+
assert(isUint8Array(source) && isUint8Array(target));
269+
// Enforce: 0 <= sourceStart <= sourceEnd <= source.byteLength
270+
assert(0 <= sourceStart && sourceStart <= sourceEnd && sourceEnd <= source.byteLength);
271+
// Enforce: 0 <= targetStart<= target.byteLength
272+
assert(0 <= targetStart && targetStart <= target.byteLength);
273+
274+
const copyLength = sourceEnd - sourceStart;
275+
const targetCapacity = target.byteLength - targetStart;
276+
assert(copyLength <= targetCapacity);
277+
278+
_copy(source, target, targetStart, sourceStart, copyLength);
279+
return copyLength;
259280
}
260281

261282
/**
@@ -602,7 +623,9 @@ Buffer.concat = function concat(list, length) {
602623
throw new ERR_INVALID_ARG_TYPE(
603624
`list[${i}]`, ['Buffer', 'Uint8Array'], list[i]);
604625
}
605-
pos += _copyActual(buf, buffer, pos, 0, buf.length);
626+
627+
const copyLength = MathMin(buf.length, buffer.length - pos);
628+
pos += _copyActual(buf, buffer, pos, 0, copyLength);
606629
}
607630

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

src/node_buffer.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -572,9 +572,9 @@ void SlowCopy(const FunctionCallbackInfo<Value>& args) {
572572
ArrayBufferViewContents<char> source(args[0]);
573573
SPREAD_BUFFER_ARG(args[1].As<Object>(), target);
574574

575-
const auto target_start = args[2]->Uint32Value(env->context()).ToChecked();
576-
const auto source_start = args[3]->Uint32Value(env->context()).ToChecked();
577-
const auto to_copy = args[4]->Uint32Value(env->context()).ToChecked();
575+
const auto target_start = args[2]->IntegerValue(env->context()).ToChecked();
576+
const auto source_start = args[3]->IntegerValue(env->context()).ToChecked();
577+
const auto to_copy = args[4]->IntegerValue(env->context()).ToChecked();
578578

579579
memmove(target_data + target_start, source.data() + source_start, to_copy);
580580
args.GetReturnValue().Set(to_copy);

test/parallel/test-buffer-concat.js

+13
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
'use strict';
2323
const common = require('../common');
2424
const assert = require('assert');
25+
const { kMaxLength } = require('buffer');
2526

2627
const zero = [];
2728
const one = [ Buffer.from('asdf') ];
@@ -98,3 +99,15 @@ assert.deepStrictEqual(
9899
assert.deepStrictEqual(Buffer.concat([new Uint8Array([0x41, 0x42]),
99100
new Uint8Array([0x43, 0x44])]),
100101
Buffer.from('ABCD'));
102+
103+
// Ref: https://github.com/nodejs/node/issues/55422#issue-2594486812
104+
if (2 ** 32 + 1 <= kMaxLength) {
105+
const a = Buffer.alloc(2 ** 31, 0);
106+
const b = Buffer.alloc(2 ** 31, 1);
107+
const c = Buffer.alloc(1, 2);
108+
const destin = Buffer.concat([a, b, c]);
109+
110+
assert.deepStrictEqual(destin.subarray(0, 2 ** 31), a);
111+
assert.deepStrictEqual(destin.subarray(2 ** 31, 2 ** 32), b);
112+
assert.deepStrictEqual(destin.subarray(2 ** 32), c);
113+
}

test/parallel/test-buffer-copy.js

+9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
require('../common');
4+
const { kMaxLength } = require('buffer');
45
const assert = require('assert');
56

67
const b = Buffer.allocUnsafe(1024);
@@ -234,3 +235,11 @@ assert.deepStrictEqual(c, b.slice(0, c.length));
234235
// No copying took place:
235236
assert.deepStrictEqual(c.toString(), 'C'.repeat(c.length));
236237
}
238+
239+
// Ref: https://github.com/nodejs/node/issues/55422#issue-2594486812
240+
if (2 ** 32 + 1 <= kMaxLength) {
241+
const src = Buffer.alloc(2 ** 32 + 1, 1);
242+
const dst = Buffer.alloc(2 ** 32 + 1, 2);
243+
src.copy(dst);
244+
assert.deepStrictEqual(src, dst);
245+
}

0 commit comments

Comments
 (0)