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

fix: accept Uint8Array where Buffer is accepted #432

Merged
merged 1 commit into from
Apr 27, 2021
Merged
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
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
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know the bug was here before anyway, but I should absolutely have caught this during #429, sorry.

}

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