buffer: allow Uint8Array input to methods

Allow all methods on `buffer` and `Buffer` to take `Uint8Array` arguments where it makes sense. On the native side, there is effectively no difference, and as a bonus the `isUint8Array` check is faster than `instanceof Buffer`. PR-URL: #10236 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
nodejs · Dec 21, 2016 · beca3244e2792bc4257f480b138c1b603de3781c · beca324 · xcstream · Jan 10, 2017
1 parent 6cb33c0
commit beca3244e2792bc4257f480b138c1b603de3781c
diff --git a/doc/api/buffer.md b/doc/api/buffer.md
@@ -635,8 +635,8 @@ actual byte length is returned.
 added: v0.11.13
 -->
 
-* `buf1` {Buffer}
-* `buf2` {Buffer}
+* `buf1` {Buffer|Uint8Array}
+* `buf2` {Buffer|Uint8Array}
 * Returns: {Integer}
 
 Compares `buf1` to `buf2` typically for the purpose of sorting arrays of
@@ -660,7 +660,7 @@ console.log(arr.sort(Buffer.compare));
 added: v0.7.11
 -->
 
-* `list` {Array} List of `Buffer` instances to concat
+* `list` {Array} List of `Buffer` or [`Uint8Array`] instances to concat
 * `totalLength` {Integer} Total length of the `Buffer` instances in `list`
   when concatenated
 * Returns: {Buffer}
@@ -882,7 +882,7 @@ console.log(buf.toString('ascii'));
 added: v0.11.13
 -->
 
-* `target` {Buffer} A `Buffer` to compare to
+* `target` {Buffer|Uint8Array} A `Buffer` or [`Uint8Array`] to compare to
 * `targetStart` {Integer} The offset within `target` at which to begin
   comparison. **Default:** `0`
 * `targetEnd` {Integer} The offset with `target` at which to end comparison
@@ -1037,7 +1037,7 @@ for (const pair of buf.entries()) {
 added: v0.11.13
 -->
 
-* `otherBuffer` {Buffer} A `Buffer` to compare to
+* `otherBuffer` {Buffer} A `Buffer` or [`Uint8Array`] to compare to
 * Returns: {Boolean}
 
 Returns `true` if both `buf` and `otherBuffer` have exactly the same bytes,
@@ -1099,7 +1099,7 @@ console.log(Buffer.allocUnsafe(3).fill('\u0222'));
 added: v1.5.0
 -->
 
-* `value` {String | Buffer | Integer} What to search for
+* `value` {String | Buffer | Uint8Array | Integer} What to search for
 * `byteOffset` {Integer} Where to begin searching in `buf`. **Default:** `0`
 * `encoding` {String} If `value` is a string, this is its encoding.
   **Default:** `'utf8'`
@@ -1110,8 +1110,8 @@ If `value` is:
 
   * a string, `value` is interpreted according to the character encoding in
     `encoding`.
-  * a `Buffer`, `value` will be used in its entirety. To compare a partial
-  `Buffer` use [`buf.slice()`].
+  * a `Buffer` or [`Uint8Array`], `value` will be used in its entirety.
+    To compare a partial `Buffer`, use [`buf.slice()`].
   * a number, `value` will be interpreted as an unsigned 8-bit integer
   value between `0` and `255`.
 
@@ -1221,7 +1221,7 @@ for (const key of buf.keys()) {
 added: v6.0.0
 -->
 
-* `value` {String | Buffer | Integer} What to search for
+* `value` {String | Buffer | Uint8Array | Integer} What to search for
 * `byteOffset` {Integer} Where to begin searching in `buf`.
   **Default:** [`buf.length`]` - 1`
 * `encoding` {String} If `value` is a string, this is its encoding.
@@ -2313,12 +2313,12 @@ Note that this is a property on the `buffer` module returned by
 added: v7.1.0
 -->
 
-* `source` {Buffer} A `Buffer` instance
+* `source` {Buffer|Uint8Array} A `Buffer` or `Uint8Array` instance
 * `fromEnc` {String} The current encoding
 * `toEnc` {String} To target encoding
 
-Re-encodes the given `Buffer` instance from one character encoding to another.
-Returns a new `Buffer` instance.
+Re-encodes the given `Buffer` or `Uint8Array` instance from one character
+encoding to another. Returns a new `Buffer` instance.
 
 Throws if the `fromEnc` or `toEnc` specify invalid character encodings or if
 conversion from `fromEnc` to `toEnc` is not permitted.

diff --git a/lib/buffer.js b/lib/buffer.js
@@ -2,7 +2,8 @@
 'use strict';
 
 const binding = process.binding('buffer');
-const { isArrayBuffer, isSharedArrayBuffer } = process.binding('util');
+const { isArrayBuffer, isSharedArrayBuffer, isUint8Array } =
+    process.binding('util');
 const bindingObj = {};
 const internalUtil = require('internal/util');
 
@@ -251,13 +252,13 @@ function fromArrayBuffer(obj, byteOffset, length) {
 }
 
 function fromObject(obj) {
-  if (obj instanceof Buffer) {
+  if (isUint8Array(obj)) {
     const b = allocate(obj.length);
 
     if (b.length === 0)
       return b;
 
-    obj.copy(b, 0, 0, obj.length);
+    binding.copy(obj, b, 0, 0, obj.length);
     return b;
   }
 
@@ -287,9 +288,8 @@ Buffer.isBuffer = function isBuffer(b) {
 
 
 Buffer.compare = function compare(a, b) {
-  if (!(a instanceof Buffer) ||
-      !(b instanceof Buffer)) {
-    throw new TypeError('Arguments must be Buffers');
+  if (!isUint8Array(a) || !isUint8Array(b)) {
+    throw new TypeError('Arguments must be Buffers or Uint8Arrays');
   }
 
   if (a === b) {
@@ -306,10 +306,13 @@ Buffer.isEncoding = function(encoding) {
 };
 Buffer[internalUtil.kIsEncodingSymbol] = Buffer.isEncoding;
 
+const kConcatErrMsg = '"list" argument must be an Array ' +
+                      'of Buffer or Uint8Array instances';
+
 Buffer.concat = function(list, length) {
   var i;
   if (!Array.isArray(list))
-    throw new TypeError('"list" argument must be an Array of Buffers');
+    throw new TypeError(kConcatErrMsg);
 
   if (list.length === 0)
     return new FastBuffer();
@@ -326,9 +329,9 @@ Buffer.concat = function(list, length) {
   var pos = 0;
   for (i = 0; i < list.length; i++) {
     var buf = list[i];
-    if (!Buffer.isBuffer(buf))
-      throw new TypeError('"list" argument must be an Array of Buffers');
-    buf.copy(buffer, pos);
+    if (!isUint8Array(buf))
+      throw new TypeError(kConcatErrMsg);
+    binding.copy(buf, buffer, pos);
     pos += buf.length;
   }
 
@@ -495,6 +498,9 @@ function slowToString(encoding, start, end) {
   }
 }
 
+Buffer.prototype.copy = function(target, targetStart, sourceStart, sourceEnd) {
+  return binding.copy(this, target, targetStart, sourceStart, sourceEnd);
+};
 
 Buffer.prototype.toString = function() {
   let result;
@@ -510,8 +516,8 @@ Buffer.prototype.toString = function() {
 
 
 Buffer.prototype.equals = function equals(b) {
-  if (!(b instanceof Buffer))
-    throw new TypeError('Argument must be a Buffer');
+  if (!isUint8Array(b))
+    throw new TypeError('Argument must be a Buffer or Uint8Array');
 
   if (this === b)
     return true;
@@ -539,8 +545,8 @@ Buffer.prototype.compare = function compare(target,
                                             thisStart,
                                             thisEnd) {
 
-  if (!(target instanceof Buffer))
-    throw new TypeError('Argument must be a Buffer');
+  if (!isUint8Array(target))
+    throw new TypeError('Argument must be a Buffer or Uint8Array');
 
   if (start === undefined)
     start = 0;
@@ -604,13 +610,14 @@ function bidirectionalIndexOf(buffer, val, byteOffset, encoding, dir) {
       return binding.indexOfString(buffer, val, byteOffset, encoding, dir);
     }
     return slowIndexOf(buffer, val, byteOffset, encoding, dir);
-  } else if (val instanceof Buffer) {
+  } else if (isUint8Array(val)) {
     return binding.indexOfBuffer(buffer, val, byteOffset, encoding, dir);
   } else if (typeof val === 'number') {
     return binding.indexOfNumber(buffer, val, byteOffset, dir);
   }
 
-  throw new TypeError('"val" argument must be string, number or Buffer');
+  throw new TypeError('"val" argument must be string, number, Buffer ' +
+                      'or Uint8Array');
 }
 
 
@@ -1037,8 +1044,8 @@ Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) {
 
 
 function checkInt(buffer, value, offset, ext, max, min) {
-  if (!(buffer instanceof Buffer))
-    throw new TypeError('"buffer" argument must be a Buffer instance');
+  if (!isUint8Array(buffer))
+    throw new TypeError('"buffer" argument must be a Buffer or Uint8Array');
   if (value > max || value < min)
     throw new TypeError('"value" argument is out of bounds');
   if (offset + ext > buffer.length)

diff --git a/lib/internal/buffer.js b/lib/internal/buffer.js
@@ -8,18 +8,19 @@ const normalizeEncoding = require('internal/util').normalizeEncoding;
 const Buffer = require('buffer').Buffer;
 
 const icu = process.binding('icu');
+const { isUint8Array } = process.binding('util');
 
 // Transcodes the Buffer from one encoding to another, returning a new
 // Buffer instance.
 exports.transcode = function transcode(source, fromEncoding, toEncoding) {
-  if (!Buffer.isBuffer(source))
-    throw new TypeError('"source" argument must be a Buffer');
+  if (!isUint8Array(source))
+    throw new TypeError('"source" argument must be a Buffer or Uint8Array');
   if (source.length === 0) return Buffer.alloc(0);
 
   fromEncoding = normalizeEncoding(fromEncoding) || fromEncoding;
   toEncoding = normalizeEncoding(toEncoding) || toEncoding;
   const result = icu.transcode(source, fromEncoding, toEncoding);
-  if (Buffer.isBuffer(result))
+  if (typeof result !== 'number')
     return result;
 
   const code = icu.icuErrName(result);

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -519,23 +519,24 @@ void Base64Slice(const FunctionCallbackInfo<Value>& args) {
 }
 
 
-// bytesCopied = buffer.copy(target[, targetStart][, sourceStart][, sourceEnd]);
+// bytesCopied = copy(buffer, target[, targetStart][, sourceStart][, sourceEnd])
 void Copy(const FunctionCallbackInfo<Value> &args) {
   Environment* env = Environment::GetCurrent(args);
 
-  THROW_AND_RETURN_UNLESS_BUFFER(env, args.This());
   THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
-  Local<Object> target_obj = args[0].As<Object>();
-  SPREAD_BUFFER_ARG(args.This(), ts_obj);
+  THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
+  Local<Object> buffer_obj = args[0].As<Object>();
+  Local<Object> target_obj = args[1].As<Object>();
+  SPREAD_BUFFER_ARG(buffer_obj, ts_obj);
   SPREAD_BUFFER_ARG(target_obj, target);
 
   size_t target_start;
   size_t source_start;
   size_t source_end;
 
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[1], 0, &target_start));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[2], 0, &source_start));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[3], ts_obj_length, &source_end));
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[2], 0, &target_start));
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[3], 0, &source_start));
+  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(args[4], ts_obj_length, &source_end));
 
   // Copy 0 bytes; we're done
   if (target_start >= target_length || source_start >= source_end)
@@ -1203,8 +1204,6 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
   env->SetMethod(proto, "ucs2Write", Ucs2Write);
   env->SetMethod(proto, "utf8Write", Utf8Write);
 
-  env->SetMethod(proto, "copy", Copy);
-
   if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) {
     CHECK(args[1]->IsObject());
     auto binding_object = args[1].As<Object>();
@@ -1227,6 +1226,7 @@ void Initialize(Local<Object> target,
   env->SetMethod(target, "createFromString", CreateFromString);
 
   env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
+  env->SetMethod(target, "copy", Copy);
   env->SetMethod(target, "compare", Compare);
   env->SetMethod(target, "compareOffset", CompareOffset);
   env->SetMethod(target, "fill", Fill);

diff --git a/src/node_util.cc b/src/node_util.cc
@@ -29,7 +29,8 @@ using v8::Value;
   V(isSet, IsSet)                                                             \
   V(isSetIterator, IsSetIterator)                                             \
   V(isSharedArrayBuffer, IsSharedArrayBuffer)                                 \
-  V(isTypedArray, IsTypedArray)
+  V(isTypedArray, IsTypedArray)                                               \
+  V(isUint8Array, IsUint8Array)
 
 
 #define V(_, ucname) \

diff --git a/test/parallel/test-buffer-compare.js b/test/parallel/test-buffer-compare.js
@@ -6,10 +6,12 @@ const assert = require('assert');
 const b = Buffer.alloc(1, 'a');
 const c = Buffer.alloc(1, 'c');
 const d = Buffer.alloc(2, 'aa');
+const e = new Uint8Array([ 0x61, 0x61 ]); // ASCII 'aa', same as d
 
 assert.strictEqual(b.compare(c), -1);
 assert.strictEqual(c.compare(d), 1);
 assert.strictEqual(d.compare(b), 1);
+assert.strictEqual(d.compare(e), 0);
 assert.strictEqual(b.compare(d), -1);
 assert.strictEqual(b.compare(b), 0);
 
@@ -18,6 +20,9 @@ assert.strictEqual(Buffer.compare(c, d), 1);
 assert.strictEqual(Buffer.compare(d, b), 1);
 assert.strictEqual(Buffer.compare(b, d), -1);
 assert.strictEqual(Buffer.compare(c, c), 0);
+assert.strictEqual(Buffer.compare(e, e), 0);
+assert.strictEqual(Buffer.compare(d, e), 0);
+assert.strictEqual(Buffer.compare(d, b), 1);
 
 assert.strictEqual(Buffer.compare(Buffer.alloc(0), Buffer.alloc(0)), 0);
 assert.strictEqual(Buffer.compare(Buffer.alloc(0), Buffer.alloc(1)), -1);

diff --git a/test/parallel/test-buffer-concat.js b/test/parallel/test-buffer-concat.js
@@ -35,7 +35,8 @@ function assertWrongList(value) {
     Buffer.concat(value);
   }, function(err) {
     return err instanceof TypeError &&
-           err.message === '"list" argument must be an Array of Buffers';
+           err.message === '"list" argument must be an Array of Buffer ' +
+                           'or Uint8Array instances';
   });
 }
 
@@ -60,3 +61,7 @@ assert.deepStrictEqual(Buffer.concat([empty], 4096), Buffer.alloc(4096));
 assert.deepStrictEqual(
     Buffer.concat([random10], 40),
     Buffer.concat([random10, Buffer.alloc(30)]));
+
+assert.deepStrictEqual(Buffer.concat([new Uint8Array([0x41, 0x42]),
+                                      new Uint8Array([0x43, 0x44])]),
+                       Buffer.from('ABCD'));
diff --git a/test/parallel/test-buffer-equals.js b/test/parallel/test-buffer-equals.js
@@ -12,5 +12,6 @@ assert.ok(b.equals(c));
 assert.ok(!c.equals(d));
 assert.ok(!d.equals(e));
 assert.ok(d.equals(d));
+assert.ok(d.equals(new Uint8Array([0x61, 0x62, 0x63, 0x64, 0x65])));
 
 assert.throws(() => Buffer.alloc(1).equals('abc'));
diff --git a/test/parallel/test-buffer-indexof.js b/test/parallel/test-buffer-indexof.js
@@ -524,3 +524,11 @@ assert.equal(0, reallyLong.lastIndexOf(pattern));
   assert.strictEqual(buf.indexOf(0xff), -1);
   assert.strictEqual(buf.indexOf(0xffff), -1);
 }
+
+// Test that Uint8Array arguments are okay.
+{
+  const needle = new Uint8Array([ 0x66, 0x6f, 0x6f ]);
+  const haystack = Buffer.from('a foo b foo');
+  assert.strictEqual(haystack.indexOf(needle), 2);
+  assert.strictEqual(haystack.lastIndexOf(needle), haystack.length - 3);
+}
diff --git a/test/parallel/test-icu-transcode.js b/test/parallel/test-icu-transcode.js
@@ -40,7 +40,7 @@ for (const test in tests) {
 
 assert.throws(
   () => buffer.transcode(null, 'utf8', 'ascii'),
-  /^TypeError: "source" argument must be a Buffer$/
+  /^TypeError: "source" argument must be a Buffer or Uint8Array$/
 );
 
 assert.throws(
@@ -62,3 +62,11 @@ assert.deepStrictEqual(
 assert.deepStrictEqual(
     buffer.transcode(Buffer.from('hä', 'latin1'), 'latin1', 'utf16le'),
     Buffer.from('hä', 'utf16le'));
+
+// Test that Uint8Array arguments are okay.
+{
+  const uint8array = new Uint8Array([...Buffer.from('hä', 'latin1')]);
+  assert.deepStrictEqual(
+      buffer.transcode(uint8array, 'latin1', 'utf16le'),
+      Buffer.from('hä', 'utf16le'));
+}