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 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 12 additions & 12 deletions doc/api/buffer.md
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
22 changes: 11 additions & 11 deletions lib/buffer.js
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);
Buffer.prototype.copy.call(obj, b, 0, 0, obj.length);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't need to reference it this way. copy() is native method. Can simply call binding.copy(...).

return b;
}

Expand Down Expand Up @@ -283,8 +284,7 @@ Buffer.isBuffer = function isBuffer(b) {


Buffer.compare = function compare(a, b) {
if (!(a instanceof Buffer) ||
!(b instanceof Buffer)) {
if (!isUint8Array(a) || !isUint8Array(b)) {
throw new TypeError('Arguments must be Buffers');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this error message will need to be changed

}

Expand Down Expand Up @@ -322,9 +322,9 @@ Buffer.concat = function(list, length) {
var pos = 0;
for (i = 0; i < list.length; i++) {
var buf = list[i];
if (!Buffer.isBuffer(buf))
if (!isUint8Array(buf))
throw new TypeError('"list" argument must be an Array of Buffers');
Copy link
Contributor

@mscdex mscdex Dec 13, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto here and elsewhere about error messages

buf.copy(buffer, pos);
Buffer.prototype.copy.call(buf, buffer, pos);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto on using binding.copy().

pos += buf.length;
}

Expand Down Expand Up @@ -506,7 +506,7 @@ Buffer.prototype.toString = function() {


Buffer.prototype.equals = function equals(b) {
if (!(b instanceof Buffer))
if (!isUint8Array(b))
throw new TypeError('Argument must be a Buffer');

if (this === b)
Expand Down Expand Up @@ -535,7 +535,7 @@ Buffer.prototype.compare = function compare(target,
thisStart,
thisEnd) {

if (!(target instanceof Buffer))
if (!isUint8Array(target))
throw new TypeError('Argument must be a Buffer');

if (start === undefined)
Expand Down Expand Up @@ -600,7 +600,7 @@ 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);
Expand Down Expand Up @@ -1033,7 +1033,7 @@ Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) {


function checkInt(buffer, value, offset, ext, max, min) {
if (!(buffer instanceof Buffer))
if (!isUint8Array(buffer))
throw new TypeError('"buffer" argument must be a Buffer instance');
if (value > max || value < min)
throw new TypeError('"value" argument is out of bounds');
Expand Down
5 changes: 3 additions & 2 deletions lib/internal/buffer.js
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))
if (!isUint8Array(source))
throw new TypeError('"source" argument must be a Buffer');
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
3 changes: 2 additions & 1 deletion src/node_util.cc
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
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
4 changes: 4 additions & 0 deletions test/parallel/test-buffer-concat.js
Expand Up @@ -60,3 +60,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
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
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);
}
8 changes: 8 additions & 0 deletions test/parallel/test-icu-transcode.js
Expand Up @@ -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'));
}