buffer: add assertions, document copy behavior

Author: duncpro

doc/api/buffer.md

@@ -1752,6 +1752,11 @@ added: v0.1.90
 Copies data from a region of `buf` to a region in `target`, even if the `target`
 memory region overlaps with `buf`.
 
+If `sourceEnd` exceeds `buf.bytesLength`, only the bytes up to
+`buf.bytesLength` will be copied.
+
+The source slice is truncated if it does not fit into the target slice.
+
 [`TypedArray.prototype.set()`][] performs the same operation, and is available
 for all TypedArrays, including Node.js `Buffer`s, although it takes
 different function arguments.

lib/buffer.js

@@ -100,6 +100,8 @@ const {
   inspect: utilInspect,
 } = require('internal/util/inspect');
 
+const assert = require('internal/assert');
+
 const {
   codes: {
     ERR_BUFFER_OUT_OF_BOUNDS,
@@ -205,12 +207,12 @@ function toInteger(n, defaultVal) {
   return defaultVal;
 }
 
+
 function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
   if (!ArrayBufferIsView(source))
     throw new ERR_INVALID_ARG_TYPE('source', ['Buffer', 'Uint8Array'], source);
   if (!ArrayBufferIsView(target))
     throw new ERR_INVALID_ARG_TYPE('target', ['Buffer', 'Uint8Array'], target);
-  // Guaranteed real buffers
 
   if (targetStart === undefined) {
     targetStart = 0;
@@ -219,7 +221,7 @@ function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
     if (targetStart < 0)
       throw new ERR_OUT_OF_RANGE('targetStart', '>= 0', targetStart);
   }
-  // Guaranteed targetStart >= 0
+  // Guaranteed 0 <= targetStart
 
   if (sourceStart === undefined) {
     sourceStart = 0;
@@ -239,26 +241,43 @@ function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
   }
   // Guaranteed 0 <= sourceEnd
 
+  // Clamp sourceEnd to be inbounds of source buffer.
+  // This is expected behavior and not to throw.
+  sourceEnd = MathMin(sourceEnd, source.byteLength);
+
   if (targetStart >= target.byteLength || sourceStart >= sourceEnd)
     return 0;
 
-  // Guaranteed 0 <= sourceStart < sourceEnd (NO GUARANTEE <= source.byteLnegth)
-  // Guaranteed 0 <= targetStart < target.byteLength here
+  // Assert source slice in bounds of source buffer.
+  // 0 <= sourceStart < sourceEnd <= source.byteLength
+  assert(0 <= sourceStart && sourceStart < sourceEnd && sourceEnd <= source.byteLength);
+
+  // Assert target slice in bounds of target buffer.
+  // 0 <= targetStart < target.byteLength
+  assert(0 <= targetStart && targetStart < target.byteLength);
+
+  // Truncate source so that its length doesn't exceed targets.
+  // This is the expected behavior, not to throw.
+  const copyLength = MathMin(sourceEnd - sourceStart, target.byteLength - targetStart);
+  sourceEnd = sourceStart + copyLength;
 
-  return _copyActual(source, target, targetStart, sourceStart, sourceEnd);
+  return _copyActual(source, target, targetStart, sourceStart, sourceStart + copyLength);
 }
 
-// Assumes `source`, `target` are real buffers.
-// Assumes 0 <= sourceStart <= sourceEnd
-// Assumes 0 <= targetStart <= target.byteLength
-//
-// This function clamps sourceEnd such that the source subarray will not exceed the length
-// of the target subarray.
+// Safely performs native copy from valid source slice to valid target slice.
+// - The source slice is not clamped to fit into the target slice. If it won't fit, this throws.
+// - If either the source or target slice are out of bounds, this throws.
 function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
-  sourceEnd = MathMin(sourceEnd, source.byteLength);
-  const sourceLength = sourceEnd - sourceStart;
-  const targetLength = target.byteLength - targetStart;
-  const copyLength = MathMin(sourceLength, targetLength);
+  assert(isUint8Array(source) && isUint8Array(target));
+  // Enforce: 0 <= sourceStart <= sourceEnd <= source.byteLength
+  assert(0 <= sourceStart && sourceStart <= sourceEnd && sourceEnd <= source.byteLength);
+  // Enforce: 0 <= targetStart<= target.byteLength
+  assert(0 <= targetStart && targetStart <= target.byteLength);
+
+  const copyLength = sourceEnd - sourceStart;
+  const targetCapacity = target.byteLength - targetStart;
+  assert(copyLength <= targetCapacity);
+
   _copy(source, target, targetStart, sourceStart, copyLength);
   return copyLength;
 }
@@ -607,8 +626,9 @@ Buffer.concat = function concat(list, length) {
       throw new ERR_INVALID_ARG_TYPE(
         `list[${i}]`, ['Buffer', 'Uint8Array'], list[i]);
     }
-    //                (src, target, t_beg, s_beg, s_end)
-    pos += _copyActual(buf, buffer, pos, 0, buf.length);
+
+    const copyLength = MathMin(buf.length, buffer.length - pos);
+    pos += _copyActual(buf, buffer, pos, 0, copyLength);
   }
 
   // Note: `length` is always equal to `buffer.length` at this point

test/parallel/test-buffer-concat.js

@@ -100,7 +100,8 @@ assert.deepStrictEqual(Buffer.concat([new Uint8Array([0x41, 0x42]),
                                       new Uint8Array([0x43, 0x44])]),
                        Buffer.from('ABCD'));
 
-
+// Test its possible to concat buffers where the total length of the result in bytes
+// is greater than 2^32.
 if (2 ** 32 + 1 <= kMaxLength) {
   const a = Buffer.alloc(2 ** 31, 0);
   const b = Buffer.alloc(2 ** 31, 1);

test/parallel/test-buffer-copy.js

@@ -1,6 +1,7 @@
 'use strict';
 
 require('../common');
+const { kMaxLength } = require('buffer');
 const assert = require('assert');
 
 const b = Buffer.allocUnsafe(1024);
@@ -234,3 +235,12 @@ assert.deepStrictEqual(c, b.slice(0, c.length));
   // No copying took place:
   assert.deepStrictEqual(c.toString(), 'C'.repeat(c.length));
 }
+
+
+// Test its possible to copy buffer with length > 2^32
+if (2 ** 32 + 1 <= kMaxLength) {
+  const src = Buffer.alloc(2 ** 32 + 1, 1);
+  const dst = Buffer.alloc(2 ** 32 + 1, 2);
+  src.copy(dst);
+  assert.deepStrictEqual(src, dst);
+}