From f6d3e514554dee4aaa1d2551c3d0771b30944104 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 23 Apr 2021 00:01:51 +0200 Subject: [PATCH] fix: accept Uint8Array where Buffer is accepted Also fixes NODE-3223 (ensureBuffer ignores byteLength/byteOffset). This is the "proper" alternative to https://github.com/mongodb/js-bson/pull/418 and matches what e.g. Node.js APIs do. --- src/ensure_buffer.ts | 12 ++++++------ src/objectid.ts | 10 +++++----- src/parser/serializer.ts | 7 +++---- src/parser/utils.ts | 5 ----- src/uuid.ts | 9 +++------ test/node/ensure_buffer_test.js | 20 ++++++++++++++++++-- 6 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/ensure_buffer.ts b/src/ensure_buffer.ts index 1a36c1d1..62e0dde0 100644 --- a/src/ensure_buffer.ts +++ b/src/ensure_buffer.ts @@ -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. @@ -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)) { diff --git a/src/objectid.ts b/src/objectid.ts index cb1ca162..752c43b3 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -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); @@ -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) { @@ -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') { @@ -315,7 +315,7 @@ export class ObjectId { return true; } - if (Buffer.isBuffer(id) && id.length === 12) { + if (isUint8Array(id) && id.length === 12) { return true; } diff --git a/src/parser/serializer.ts b/src/parser/serializer.ts index 3cf1fb3e..ee9a70d2 100644 --- a/src/parser/serializer.ts +++ b/src/parser/serializer.ts @@ -18,7 +18,6 @@ import type { BSONRegExp } from '../regexp'; import { isBigInt64Array, isBigUInt64Array, - isBuffer, isDate, isMap, isRegExp, @@ -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); @@ -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); @@ -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); diff --git a/src/parser/utils.ts b/src/parser/utils.ts index a083d79d..3f0dcc7c 100644 --- a/src/parser/utils.ts +++ b/src/parser/utils.ts @@ -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]'; diff --git a/src/uuid.ts b/src/uuid.ts index 85288201..1ac61c54 100644 --- a/src/uuid.ts +++ b/src/uuid.ts @@ -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 = { @@ -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); @@ -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; diff --git a/test/node/ensure_buffer_test.js b/test/node/ensure_buffer_test.js index 5857675f..fe271138 100644 --- a/test/node/ensure_buffer_test.js +++ b/test/node/ensure_buffer_test.js @@ -9,7 +9,7 @@ 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; @@ -17,7 +17,10 @@ describe('ensureBuffer tests', 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 () { @@ -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 () {