Skip to content

Commit

Permalink
Fix issues surrounding type checking for Uint8Arrays and Buffers
Browse files Browse the repository at this point in the history
  • Loading branch information
chjj committed Feb 3, 2024
1 parent 54885d4 commit 0927999
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 37 deletions.
58 changes: 21 additions & 37 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,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)

Expand Down Expand Up @@ -358,9 +359,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'
)
Expand Down Expand Up @@ -423,37 +422,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. ' +
Expand Down Expand Up @@ -627,7 +617,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
}
Expand All @@ -644,10 +633,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)
Expand Down Expand Up @@ -692,13 +678,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
}
}
Expand Down Expand Up @@ -1714,7 +1697,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
Expand Down Expand Up @@ -1809,7 +1792,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
Expand Down Expand Up @@ -2101,7 +2084,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
Expand Down
179 changes: 179 additions & 0 deletions test/typing.js
Original file line number Diff line number Diff line change
@@ -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()
})

0 comments on commit 0927999

Please sign in to comment.