Skip to content

Commit

Permalink
fix: accept Uint8Array where Buffer is accepted
Browse files Browse the repository at this point in the history
Also fixes NODE-3223 (ensureBuffer ignores byteLength/byteOffset).
This is the "proper" alternative to
#418 and matches what e.g.
Node.js APIs do.
  • Loading branch information
addaleax committed Apr 26, 2021
1 parent 3ffc44f commit f6d3e51
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 28 deletions.
12 changes: 6 additions & 6 deletions src/ensure_buffer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Buffer } from 'buffer';
import { isBuffer, isAnyArrayBuffer } from './parser/utils';
import { isAnyArrayBuffer } from './parser/utils';

/**
* Makes sure that, if a Uint8Array is passed in, it is wrapped in a Buffer.
Expand All @@ -10,12 +10,12 @@ import { isBuffer, isAnyArrayBuffer } from './parser/utils';
* @throws TypeError If anything other than a Buffer or Uint8Array is passed in
*/
export function ensureBuffer(potentialBuffer: Buffer | ArrayBufferView | ArrayBuffer): Buffer {
if (isBuffer(potentialBuffer)) {
return potentialBuffer;
}

if (ArrayBuffer.isView(potentialBuffer)) {
return Buffer.from(potentialBuffer.buffer);
return Buffer.from(
potentialBuffer.buffer,
potentialBuffer.byteOffset,
potentialBuffer.byteLength
);
}

if (isAnyArrayBuffer(potentialBuffer)) {
Expand Down
10 changes: 5 additions & 5 deletions src/objectid.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Buffer } from 'buffer';
import { ensureBuffer } from './ensure_buffer';
import { deprecate, randomBytes } from './parser/utils';
import { deprecate, isUint8Array, randomBytes } from './parser/utils';

// constants
const PROCESS_UNIQUE = randomBytes(5);
Expand Down Expand Up @@ -229,9 +229,9 @@ export class ObjectId {
typeof otherId === 'string' &&
ObjectId.isValid(otherId) &&
otherId.length === 12 &&
Buffer.isBuffer(this.id)
isUint8Array(this.id)
) {
return otherId === this.id.toString('binary');
return otherId === Buffer.prototype.toString.call(this.id, 'latin1');
}

if (typeof otherId === 'string' && ObjectId.isValid(otherId) && otherId.length === 24) {
Expand Down Expand Up @@ -300,7 +300,7 @@ export class ObjectId {
*
* @param id - ObjectId instance to validate.
*/
static isValid(id: number | string | ObjectId | Buffer | ObjectIdLike): boolean {
static isValid(id: number | string | ObjectId | Uint8Array | ObjectIdLike): boolean {
if (id == null) return false;

if (typeof id === 'number') {
Expand All @@ -315,7 +315,7 @@ export class ObjectId {
return true;
}

if (Buffer.isBuffer(id) && id.length === 12) {
if (isUint8Array(id) && id.length === 12) {
return true;
}

Expand Down
7 changes: 3 additions & 4 deletions src/parser/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import type { BSONRegExp } from '../regexp';
import {
isBigInt64Array,
isBigUInt64Array,
isBuffer,
isDate,
isMap,
isRegExp,
Expand Down Expand Up @@ -784,7 +783,7 @@ export function serializeInto(
index = serializeNull(buffer, key, value, index, true);
} else if (value['_bsontype'] === 'ObjectId' || value['_bsontype'] === 'ObjectID') {
index = serializeObjectId(buffer, key, value, index, true);
} else if (isBuffer(value) || isUint8Array(value)) {
} else if (isUint8Array(value)) {
index = serializeBuffer(buffer, key, value, index, true);
} else if (value instanceof RegExp || isRegExp(value)) {
index = serializeRegExp(buffer, key, value, index, true);
Expand Down Expand Up @@ -890,7 +889,7 @@ export function serializeInto(
index = serializeNull(buffer, key, value, index);
} else if (value['_bsontype'] === 'ObjectId' || value['_bsontype'] === 'ObjectID') {
index = serializeObjectId(buffer, key, value, index);
} else if (isBuffer(value) || isUint8Array(value)) {
} else if (isUint8Array(value)) {
index = serializeBuffer(buffer, key, value, index);
} else if (value instanceof RegExp || isRegExp(value)) {
index = serializeRegExp(buffer, key, value, index);
Expand Down Expand Up @@ -996,7 +995,7 @@ export function serializeInto(
index = serializeNull(buffer, key, value, index);
} else if (value['_bsontype'] === 'ObjectId' || value['_bsontype'] === 'ObjectID') {
index = serializeObjectId(buffer, key, value, index);
} else if (isBuffer(value) || isUint8Array(value)) {
} else if (isUint8Array(value)) {
index = serializeBuffer(buffer, key, value, index);
} else if (value instanceof RegExp || isRegExp(value)) {
index = serializeRegExp(buffer, key, value, index);
Expand Down
5 changes: 0 additions & 5 deletions src/parser/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,6 @@ export function haveBuffer(): boolean {
return typeof global !== 'undefined' && typeof global.Buffer !== 'undefined';
}

/** Callable in any environment to check if value is a Buffer */
export function isBuffer(value: unknown): value is Buffer {
return typeof value === 'object' && value?.constructor?.name === 'Buffer';
}

// To ensure that 0.4 of node works correctly
export function isDate(d: unknown): d is Date {
return isObjectLike(d) && Object.prototype.toString.call(d) === '[object Date]';
Expand Down
9 changes: 3 additions & 6 deletions src/uuid.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Buffer } from 'buffer';
import { ensureBuffer } from './ensure_buffer';
import { Binary } from './binary';
import { bufferToUuidHexString, uuidHexStringToBuffer, uuidValidateString } from './uuid_utils';
import { randomBytes } from './parser/utils';
import { isUint8Array, randomBytes } from './parser/utils';

/** @public */
export type UUIDExtended = {
Expand Down Expand Up @@ -40,10 +40,7 @@ export class UUID {
} else if (input instanceof UUID) {
this[kId] = Buffer.from(input.id);
this.__id = input.__id;
} else if (
(Buffer.isBuffer(input) || ArrayBuffer.isView(input)) &&
input.byteLength === BYTE_LENGTH
) {
} else if (ArrayBuffer.isView(input) && input.byteLength === BYTE_LENGTH) {
this.id = ensureBuffer(input);
} else if (typeof input === 'string') {
this.id = uuidHexStringToBuffer(input);
Expand Down Expand Up @@ -167,7 +164,7 @@ export class UUID {
return uuidValidateString(input);
}

if (Buffer.isBuffer(input)) {
if (isUint8Array(input)) {
// check for length & uuid version (https://tools.ietf.org/html/rfc4122#section-4.1.3)
if (input.length !== BYTE_LENGTH) {
return false;
Expand Down
20 changes: 18 additions & 2 deletions test/node/ensure_buffer_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@ describe('ensureBuffer tests', function () {
expect(ensureBuffer).to.be.a('function');
});

it('should return the exact same buffer if a buffer is passed in', function () {
it('should return a view over the exact same memory when a Buffer is passed in', function () {
const bufferIn = Buffer.alloc(10);
let bufferOut;

expect(function () {
bufferOut = ensureBuffer(bufferIn);
}).to.not.throw(Error);

expect(bufferOut).to.equal(bufferIn);
expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.buffer).to.equal(bufferIn.buffer);
expect(bufferOut.byteLength).to.equal(bufferIn.byteLength);
expect(bufferOut.byteOffset).to.equal(bufferIn.byteOffset);
});

it('should wrap a Uint8Array with a buffer', function () {
Expand Down Expand Up @@ -60,6 +63,19 @@ describe('ensureBuffer tests', function () {
expect(bufferOut.buffer).to.equal(arrayBufferIn);
});

it('should account for the input view byteLength and byteOffset', function () {
const input = new Uint8Array(new Uint8Array([1, 2, 3, 4, 5]).buffer, 1, 3);
let bufferOut;

expect(function () {
bufferOut = ensureBuffer(input);
}).to.not.throw(Error);

expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.byteLength).to.equal(3);
expect(bufferOut.byteOffset).to.equal(1);
});

[0, 12, -1, '', 'foo', null, undefined, ['list'], {}, /x/].forEach(function (item) {
it(`should throw if input is ${typeof item}: ${item}`, function () {
expect(function () {
Expand Down

0 comments on commit f6d3e51

Please sign in to comment.