Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buffer: allow Uint8Array input to methods #10236

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions doc/api/buffer.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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'`
Expand All @@ -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`.

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
43 changes: 25 additions & 18 deletions lib/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -247,13 +248,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;
}

Expand Down Expand Up @@ -283,9 +284,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) {
Expand All @@ -302,10 +302,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();
Expand All @@ -322,9 +325,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;
}

Expand Down Expand Up @@ -491,6 +494,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;
Expand All @@ -506,8 +512,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;
Expand Down Expand Up @@ -535,8 +541,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;
Expand Down Expand Up @@ -600,13 +606,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');
}


Expand Down Expand Up @@ -1033,8 +1040,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)
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/buffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
18 changes: 9 additions & 9 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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>();
Expand All @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand Down
5 changes: 5 additions & 0 deletions test/parallel/test-buffer-compare.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion test/parallel/test-buffer-concat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
});
}

Expand All @@ -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'));
1 change: 1 addition & 0 deletions test/parallel/test-buffer-equals.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
8 changes: 8 additions & 0 deletions test/parallel/test-buffer-indexof.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
10 changes: 9 additions & 1 deletion test/parallel/test-icu-transcode.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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'));
}