From 5ac5ac465976d40de0cf3bc3afd9ec9ca04975f2 Mon Sep 17 00:00:00 2001 From: "Christopher Jeffrey (JJ)" Date: Wed, 7 Feb 2024 20:09:59 -0500 Subject: [PATCH] Fix issues surrounding type checking for Uint8Arrays and Buffers (#346) --- index.js | 58 ++++++---------- test/typing.js | 179 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 200 insertions(+), 37 deletions(-) create mode 100644 test/typing.js diff --git a/index.js b/index.js index cbe4c51..32f290f 100644 --- a/index.js +++ b/index.js @@ -317,6 +317,7 @@ function fromArrayBuffer (array, byteOffset, length) { function fromObject (obj) { if (Buffer.isBuffer(obj)) { + // Note: Probably not necessary anymore. const len = checked(obj.length) | 0 const buf = createBuffer(len) @@ -363,9 +364,7 @@ Buffer.isBuffer = function isBuffer (b) { } Buffer.compare = function compare (a, b) { - if (isInstance(a, Uint8Array)) a = Buffer.from(a, a.offset, a.byteLength) - if (isInstance(b, Uint8Array)) b = Buffer.from(b, b.offset, b.byteLength) - if (!Buffer.isBuffer(a) || !Buffer.isBuffer(b)) { + if (!isInstance(a, Uint8Array) || !isInstance(b, Uint8Array)) { throw new TypeError( 'The "buf1", "buf2" arguments must be one of type Buffer or Uint8Array' ) @@ -428,37 +427,28 @@ Buffer.concat = function concat (list, length) { const buffer = Buffer.allocUnsafe(length) let pos = 0 for (i = 0; i < list.length; ++i) { - let buf = list[i] - if (isInstance(buf, Uint8Array)) { - if (pos + buf.length > buffer.length) { - if (!Buffer.isBuffer(buf)) { - buf = Buffer.from(buf.buffer, buf.byteOffset, buf.byteLength) - } - buf.copy(buffer, pos) - } else { - Uint8Array.prototype.set.call( - buffer, - buf, - pos - ) - } - } else if (!Buffer.isBuffer(buf)) { + const buf = list[i] + if (!isInstance(buf, Uint8Array)) { throw new TypeError('"list" argument must be an Array of Buffers') - } else { - buf.copy(buffer, pos) } + if (pos + buf.length > buffer.length) { + buffer.set(buf.subarray(0, buffer.length - pos), pos) + break + } + buffer.set(buf, pos) pos += buf.length } return buffer } function byteLength (string, encoding) { - if (Buffer.isBuffer(string)) { - return string.length - } if (ArrayBuffer.isView(string) || isInstance(string, ArrayBuffer)) { return string.byteLength } + if (typeof SharedArrayBuffer !== 'undefined' && + isInstance(string, SharedArrayBuffer)) { + return string.byteLength + } if (typeof string !== 'string') { throw new TypeError( 'The "string" argument must be one of type string, Buffer, or ArrayBuffer. ' + @@ -632,7 +622,6 @@ Buffer.prototype.toString = function toString () { Buffer.prototype.toLocaleString = Buffer.prototype.toString Buffer.prototype.equals = function equals (b) { - if (!Buffer.isBuffer(b)) throw new TypeError('Argument must be a Buffer') if (this === b) return true return Buffer.compare(this, b) === 0 } @@ -649,10 +638,7 @@ if (customInspectSymbol) { } Buffer.prototype.compare = function compare (target, start, end, thisStart, thisEnd) { - if (isInstance(target, Uint8Array)) { - target = Buffer.from(target, target.offset, target.byteLength) - } - if (!Buffer.isBuffer(target)) { + if (!isInstance(target, Uint8Array)) { throw new TypeError( 'The "target" argument must be one of type Buffer or Uint8Array. ' + 'Received type ' + (typeof target) @@ -697,13 +683,10 @@ Buffer.prototype.compare = function compare (target, start, end, thisStart, this let y = end - start const len = Math.min(x, y) - const thisCopy = this.slice(thisStart, thisEnd) - const targetCopy = target.slice(start, end) - for (let i = 0; i < len; ++i) { - if (thisCopy[i] !== targetCopy[i]) { - x = thisCopy[i] - y = targetCopy[i] + if (this[thisStart + i] !== target[start + i]) { + x = this[thisStart + i] + y = target[start + i] break } } @@ -1719,7 +1702,7 @@ Buffer.prototype.writeDoubleBE = function writeDoubleBE (value, offset, noAssert // copy(targetBuffer, targetStart=0, sourceStart=0, sourceEnd=buffer.length) Buffer.prototype.copy = function copy (target, targetStart, start, end) { - if (!Buffer.isBuffer(target)) throw new TypeError('argument should be a Buffer') + if (!isInstance(target, Uint8Array)) throw new TypeError('argument should be a Buffer') if (!start) start = 0 if (!end && end !== 0) end = this.length if (targetStart >= target.length) targetStart = target.length @@ -1814,7 +1797,7 @@ Buffer.prototype.fill = function fill (val, start, end, encoding) { this[i] = val } } else { - const bytes = Buffer.isBuffer(val) + const bytes = isInstance(val, Uint8Array) ? val : Buffer.from(val, encoding) const len = bytes.length @@ -2100,7 +2083,8 @@ function blitBuffer (src, dst, offset, length) { function isInstance (obj, type) { return obj instanceof type || (obj != null && obj.constructor != null && obj.constructor.name != null && - obj.constructor.name === type.name) + obj.constructor.name === type.name) || + (type === Uint8Array && Buffer.isBuffer(obj)) } function numberIsNaN (obj) { // For IE11 support diff --git a/test/typing.js b/test/typing.js new file mode 100644 index 0000000..b63e587 --- /dev/null +++ b/test/typing.js @@ -0,0 +1,179 @@ +'use strict' + +const Buffer = require('../').Buffer +const test = require('tape') +const vm = require('vm') + +// Get a Uint8Array and Buffer constructor from another context. +const code = ` + 'use strict' + function Buffer (...args) { + const buf = new Uint8Array(...args) + Object.setPrototypeOf(buf, Buffer.prototype) + return buf + } + Object.setPrototypeOf(Buffer.prototype, Uint8Array.prototype) + Object.setPrototypeOf(Buffer, Uint8Array) + Buffer.prototype._isBuffer = true + exports.Uint8Array = Uint8Array + exports.Buffer = Buffer +` + +const context = {} + +// Should work in browserify. +vm.runInNewContext(code, { exports: context }) + +const arrays = [context.Uint8Array, context.Buffer] + +// Extracted from the index.js code for testing purposes. +function isInstance (obj, type) { + return (obj instanceof type) || + (obj != null && + obj.constructor != null && + obj.constructor.name != null && + obj.constructor.name === type.name) || + (type === Uint8Array && Buffer.isBuffer(obj)) +} + +test('Uint8Arrays and Buffers from other contexts', (t) => { + // Our buffer is considered a view. + t.ok(ArrayBuffer.isView(Buffer.alloc(0))) + + for (const ForeignArray of arrays) { + const buf = new ForeignArray(1) + + buf[0] = 1 + + // Prove that ArrayBuffer.isView and isInstance + // return true for objects from other contexts. + t.ok(!(buf instanceof Object)) + t.ok(!(buf instanceof Uint8Array)) + t.ok(!(buf instanceof Buffer)) + t.ok(ArrayBuffer.isView(buf)) + + // Now returns true even for Buffers from other contexts: + t.ok(isInstance(buf, Uint8Array)) + + if (ForeignArray === context.Uint8Array) { + t.ok(!Buffer.isBuffer(buf)) + } else { + t.ok(Buffer.isBuffer(buf)) + } + + // They even behave the same! + const copy = new Uint8Array(buf) + + t.ok(copy instanceof Object) + t.ok(copy instanceof Uint8Array) + t.ok(ArrayBuffer.isView(copy)) + t.equal(copy[0], 1) + } + + t.end() +}) + +test('should instantiate from foreign arrays', (t) => { + for (const ForeignArray of arrays) { + const arr = new ForeignArray(2) + + arr[0] = 1 + arr[1] = 2 + + const buf = Buffer.from(arr) + + t.equal(buf.toString('hex'), '0102') + } + + t.end() +}) + +test('should do comparisons with foreign arrays', (t) => { + const a = Buffer.from([1, 2, 3]) + const b = new context.Uint8Array(a) + const c = new context.Buffer(a) + + t.equal(Buffer.byteLength(a), 3) + t.equal(Buffer.byteLength(b), 3) + t.equal(Buffer.byteLength(c), 3) + t.equal(b[0], 1) + t.equal(c[0], 1) + + t.ok(a.equals(b)) + t.ok(a.equals(c)) + t.ok(a.compare(b) === 0) + t.ok(a.compare(c) === 0) + t.ok(Buffer.compare(a, b) === 0) + t.ok(Buffer.compare(a, c) === 0) + t.ok(Buffer.compare(b, c) === 0) + t.ok(Buffer.compare(c, b) === 0) + + a[0] = 0 + + t.ok(!a.equals(b)) + t.ok(!a.equals(c)) + t.ok(a.compare(b) < 0) + t.ok(a.compare(c) < 0) + t.ok(Buffer.compare(a, b) < 0) + t.ok(Buffer.compare(a, c) < 0) + + b[0] = 0 + + t.ok(Buffer.compare(b, c) < 0) + t.ok(Buffer.compare(c, b) > 0) + + t.end() +}) + +test('should fill with foreign arrays', (t) => { + for (const ForeignArray of arrays) { + const buf = Buffer.alloc(4) + const arr = new ForeignArray(2) + + arr[0] = 1 + arr[1] = 2 + + buf.fill(arr) + + t.equal(buf.toString('hex'), '01020102') + } + + t.end() +}) + +test('should do concatenation with foreign arrays', (t) => { + for (const ForeignArray of arrays) { + const a = new ForeignArray(2) + + a[0] = 1 + a[1] = 2 + + const b = new ForeignArray(a) + + { + const buf = Buffer.concat([a, b]) + t.equal(buf.toString('hex'), '01020102') + } + + { + const buf = Buffer.concat([a, b], 3) + t.equal(buf.toString('hex'), '010201') + } + } + + t.end() +}) + +test('should copy on to foreign arrays', (t) => { + for (const ForeignArray of arrays) { + const a = Buffer.from([1, 2]) + const b = new ForeignArray(2) + + a.copy(b) + + t.equal(b[0], 1) + t.equal(b[1], 2) + } + + t.end() +}) -- 2.34.1