From 5f99b1bfa74bcf75700174f8d4a8b974f9753e7f Mon Sep 17 00:00:00 2001 From: gjchong25 Date: Tue, 2 Nov 2021 14:33:18 -0500 Subject: [PATCH] fix(NODE-3662): error checking to make sure that ObjectId results in object with correct properties (#467) --- src/objectid.ts | 70 +++++----- test/node/object_id_tests.js | 247 ++++++++++++++++++++++++++++++----- 2 files changed, 248 insertions(+), 69 deletions(-) diff --git a/src/objectid.ts b/src/objectid.ts index a13165e9..dcd8cd6a 100644 --- a/src/objectid.ts +++ b/src/objectid.ts @@ -31,7 +31,7 @@ export class ObjectId { _bsontype!: 'ObjectId'; /** @internal */ - static index = ~~(Math.random() * 0xffffff); + static index = Math.floor(Math.random() * 0xffffff); static cacheHexString: boolean; @@ -43,54 +43,54 @@ export class ObjectId { /** * Create an ObjectId type * - * @param id - Can be a 24 character hex string, 12 byte binary Buffer, or a number. + * @param inputId - Can be a 24 character hex string, 12 byte binary Buffer, or a number. */ - constructor(id?: string | Buffer | number | ObjectIdLike | ObjectId) { - if (!(this instanceof ObjectId)) return new ObjectId(id); + constructor(inputId?: string | Buffer | number | ObjectIdLike | ObjectId) { + if (!(this instanceof ObjectId)) return new ObjectId(inputId); - // Duck-typing to support ObjectId from different npm packages - if (id instanceof ObjectId) { - this[kId] = id.id; - this.__id = id.__id; - } - - if (typeof id === 'object' && id && 'id' in id) { - if ('toHexString' in id && typeof id.toHexString === 'function') { - this[kId] = Buffer.from(id.toHexString(), 'hex'); + // workingId is set based on type of input and whether valid id exists for the input + let workingId; + if (typeof inputId === 'object' && inputId && 'id' in inputId) { + if (typeof inputId.id !== 'string' && !ArrayBuffer.isView(inputId.id)) { + throw new BSONTypeError( + 'Argument passed in must have an id that is of type string or Buffer' + ); + } + if ('toHexString' in inputId && typeof inputId.toHexString === 'function') { + workingId = Buffer.from(inputId.toHexString(), 'hex'); } else { - this[kId] = typeof id.id === 'string' ? Buffer.from(id.id) : id.id; + workingId = inputId.id; } + } else { + workingId = inputId; } - // The most common use case (blank id, new objectId instance) - if (id == null || typeof id === 'number') { + // the following cases use workingId to construct an ObjectId + if (workingId == null || typeof workingId === 'number') { + // The most common use case (blank id, new objectId instance) // Generate a new id - this[kId] = ObjectId.generate(typeof id === 'number' ? id : undefined); - // If we are caching the hex string - if (ObjectId.cacheHexString) { - this.__id = this.id.toString('hex'); - } - } - - if (ArrayBuffer.isView(id) && id.byteLength === 12) { - this[kId] = ensureBuffer(id); - } - - if (typeof id === 'string') { - if (id.length === 12) { - const bytes = Buffer.from(id); + this[kId] = ObjectId.generate(typeof workingId === 'number' ? workingId : undefined); + } else if (ArrayBuffer.isView(workingId) && workingId.byteLength === 12) { + this[kId] = ensureBuffer(workingId); + } else if (typeof workingId === 'string') { + if (workingId.length === 12) { + const bytes = Buffer.from(workingId); if (bytes.byteLength === 12) { this[kId] = bytes; + } else { + throw new BSONTypeError('Argument passed in must be a string of 12 bytes'); } - } else if (id.length === 24 && checkForHexRegExp.test(id)) { - this[kId] = Buffer.from(id, 'hex'); + } else if (workingId.length === 24 && checkForHexRegExp.test(workingId)) { + this[kId] = Buffer.from(workingId, 'hex'); } else { throw new BSONTypeError( - 'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters' + 'Argument passed in must be a string of 12 bytes or a string of 24 hex characters' ); } + } else { + throw new BSONTypeError('Argument passed in does not match the accepted types'); } - + // If we are caching the hex string if (ObjectId.cacheHexString) { this.__id = this.id.toString('hex'); } @@ -156,7 +156,7 @@ export class ObjectId { */ static generate(time?: number): Buffer { if ('number' !== typeof time) { - time = ~~(Date.now() / 1000); + time = Math.floor(Date.now() / 1000); } const inc = ObjectId.getInc(); diff --git a/test/node/object_id_tests.js b/test/node/object_id_tests.js index 601768c0..4d18b69f 100644 --- a/test/node/object_id_tests.js +++ b/test/node/object_id_tests.js @@ -6,16 +6,12 @@ const util = require('util'); const ObjectId = BSON.ObjectId; describe('ObjectId', function () { - /** - * @ignore - */ it('should correctly handle objectId timestamps', function (done) { - // var test_number = {id: ObjectI()}; - var a = ObjectId.createFromTime(1); + const a = ObjectId.createFromTime(1); expect(Buffer.from([0, 0, 0, 1])).to.deep.equal(a.id.slice(0, 4)); expect(1000).to.equal(a.getTimestamp().getTime()); - var b = new ObjectId(); + const b = new ObjectId(); b.generationTime = 1; expect(Buffer.from([0, 0, 0, 1])).to.deep.equal(b.id.slice(0, 4)); expect(1).to.equal(b.generationTime); @@ -24,13 +20,192 @@ describe('ObjectId', function () { done(); }); - /** - * @ignore - */ + it('should correctly create ObjectId from ObjectId', function () { + const noArgObjID = new ObjectId(); + expect(new ObjectId(noArgObjID).id).to.deep.equal(Buffer.from(noArgObjID.id, 'hex')); + }); + + const invalidInputs = [ + { input: [], description: 'empty array' }, + { input: ['abcdefŽhijkl'], description: 'nonempty array' }, + { input: {}, description: 'empty object' } + ]; + + for (const { input, description } of invalidInputs) { + it(`should throw error if ${description} is passed in`, function () { + expect(() => new ObjectId(input)).to.throw(TypeError); + }); + } + + it('should throw error if object without an id property is passed in', function () { + const noArgObjID = new ObjectId(); + const objectIdLike = { + toHexString: function () { + return noArgObjID.toHexString(); + } + }; + + expect(() => new ObjectId(objectIdLike)).to.throw(TypeError); + }); + + it('should correctly create ObjectId from object with valid string id', function () { + const objectValidString24Hex = { + id: 'aaaaaaaaaaaaaaaaaaaaaaaa' + }; + const objectValidString12Bytes = { + id: 'abcdefghijkl' + }; + const buf24Hex = Buffer.from('aaaaaaaaaaaaaaaaaaaaaaaa', 'hex'); + const buf12Bytes = Buffer.from('abcdefghijkl'); + expect(new ObjectId(objectValidString24Hex).id).to.deep.equal(buf24Hex); + expect(new ObjectId(objectValidString12Bytes).id).to.deep.equal(buf12Bytes); + }); + + it('should correctly create ObjectId from object with valid string id and toHexString method', function () { + function new24HexToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + const buf24hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + const objectValidString24Hex = { + id: 'aaaaaaaaaaaaaaaaaaaaaaaa', + toHexString: new24HexToHexString + }; + const objectValidString12Bytes = { + id: 'abcdefghijkl', + toHexString: new24HexToHexString + }; + expect(new ObjectId(objectValidString24Hex).id).to.deep.equal(buf24hex); + expect(new ObjectId(objectValidString12Bytes).id).to.deep.equal(buf24hex); + }); + + it('should correctly create ObjectId from object with valid Buffer id', function () { + const validBuffer24Hex = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); + const validBuffer12Array = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + const objectBufferId = { + id: validBuffer24Hex + }; + const objectBufferFromArray = { + id: validBuffer12Array + }; + expect(new ObjectId(objectBufferId).id).to.deep.equals(validBuffer24Hex); + expect(new ObjectId(objectBufferFromArray).id).to.deep.equals(validBuffer12Array); + }); + + it('should correctly create ObjectId from object with valid Buffer id and toHexString method', function () { + const validBuffer24Hex = Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex'); + const validBuffer12Array = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + const bufferNew24Hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + const objectBufferHex = { + id: validBuffer24Hex, + toHexString: newToHexString + }; + const objectBufferArray = { + id: validBuffer12Array, + toHexString: newToHexString + }; + expect(new ObjectId(objectBufferHex).id).to.deep.equal(bufferNew24Hex); + expect(new ObjectId(objectBufferArray).id).to.deep.equal(bufferNew24Hex); + }); + + it('should throw error if object with non-Buffer non-string id is passed in', function () { + const objectNumId = { + id: 5 + }; + const objectNullId = { + id: null + }; + expect(() => new ObjectId(objectNumId)).to.throw(TypeError); + expect(() => new ObjectId(objectNullId)).to.throw(TypeError); + }); + + it('should throw an error if object with invalid string id is passed in', function () { + const objectInvalid24HexStr = { + id: 'FFFFFFFFFFFFFFFFFFFFFFFG' + }; + expect(() => new ObjectId(objectInvalid24HexStr)).to.throw(TypeError); + }); + + it('should correctly create ObjectId from object with invalid string id and toHexString method', function () { + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + const objectInvalid24HexStr = { + id: 'FFFFFFFFFFFFFFFFFFFFFFFG', + toHexString: newToHexString + }; + const bufferNew24Hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + expect(new ObjectId(objectInvalid24HexStr).id).to.deep.equal(bufferNew24Hex); + }); + + it('should throw an error if object with invalid Buffer id is passed in', function () { + const objectInvalidBuffer = { + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]) + }; + expect(() => new ObjectId(objectInvalidBuffer)).to.throw(TypeError); + }); + + it('should correctly create ObjectId from object with invalid Buffer id and toHexString method', function () { + function newToHexString() { + return 'BBBBBBBBBBBBBBBBBBBBBBBB'; + } + const objectInvalidBuffer = { + id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]), + toHexString: newToHexString + }; + const bufferNew24Hex = Buffer.from('BBBBBBBBBBBBBBBBBBBBBBBB', 'hex'); + expect(new ObjectId(objectInvalidBuffer).id).to.deep.equal(bufferNew24Hex); + }); + + const numericIO = [ + { input: 42, output: 42, description: '42' }, + { input: 0x2a, output: 0x2a, description: '0x2a' }, + { input: 4.2, output: 4, description: '4.2' }, + { input: NaN, output: 0, description: 'NaN' } + ]; + + for (const { input, output } of numericIO) { + it(`should correctly create ObjectId from ${input} and result in ${output}`, function () { + const objId = new ObjectId(input); + expect(objId).to.have.property('id'); + expect(objId.id).to.be.instanceOf(Buffer); + expect(objId.id.readUInt32BE(0)).to.equal(output); + }); + } + + it('should correctly create ObjectId undefined or null', function () { + const objNull = new ObjectId(null); + const objNoArg = new ObjectId(); + const objUndef = new ObjectId(undefined); + expect(objNull.id).to.be.instanceOf(Buffer); + expect(objNoArg.id).to.be.instanceOf(Buffer); + expect(objUndef.id).to.be.instanceOf(Buffer); + }); + + it('should throw error if non-12 byte non-24 hex string passed in', function () { + expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(TypeError); + expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(TypeError); + expect(() => new ObjectId('tooshort')).to.throw(TypeError); + expect(() => new ObjectId('101010')).to.throw(TypeError); + expect(() => new ObjectId('')).to.throw(TypeError); + }); + + it('should correctly create ObjectId from 24 hex string', function () { + const validStr24Hex = 'FFFFFFFFFFFFFFFFFFFFFFFF'; + expect(new ObjectId(validStr24Hex).id).to.deep.equal(Buffer.from(validStr24Hex, 'hex')); + }); + + it('should correctly create ObjectId from 12 byte sequence', function () { + const byteSequence12 = '111111111111'; + expect(new ObjectId(byteSequence12).id).to.deep.equal(Buffer.from(byteSequence12, 'latin1')); + }); + it('should correctly create ObjectId from uppercase hexstring', function (done) { - var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var b = new ObjectId(a); - var c = b.equals(a); // => false + let a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + let b = new ObjectId(a); + let c = b.equals(a); // => false expect(true).to.equal(c); a = 'aaaaaaaaaaaaaaaaaaaaaaaa'; @@ -42,14 +217,11 @@ describe('ObjectId', function () { done(); }); - /** - * @ignore - */ - it('should correctly create ObjectId from Buffer', function (done) { + it('should correctly create ObjectId from valid Buffer', function (done) { if (!Buffer.from) return done(); - var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var b = new ObjectId(Buffer.from(a, 'hex')); - var c = b.equals(a); // => false + let a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + let b = new ObjectId(Buffer.from(a, 'hex')); + let c = b.equals(a); // => false expect(true).to.equal(c); a = 'aaaaaaaaaaaaaaaaaaaaaaaa'; @@ -60,24 +232,23 @@ describe('ObjectId', function () { done(); }); - /** - * @ignore - */ + it('should throw an error if invalid Buffer passed in', function () { + const a = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + expect(() => new ObjectId(a)).to.throw(TypeError); + }); + it('should correctly allow for node.js inspect to work with ObjectId', function (done) { - var a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; - var b = new ObjectId(a); + const a = 'AAAAAAAAAAAAAAAAAAAAAAAA'; + const b = new ObjectId(a); expect(util.inspect(b)).to.equal('new ObjectId("aaaaaaaaaaaaaaaaaaaaaaaa")'); done(); }); - /** - * @ignore - */ it('should isValid check input Buffer length', function (done) { - var buffTooShort = Buffer.from([]); - var buffTooLong = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); - var buff12Bytes = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); + const buffTooShort = Buffer.from([]); + const buffTooLong = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13]); + const buff12Bytes = Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]); expect(ObjectId.isValid(buffTooShort)).to.be.false; expect(ObjectId.isValid(buffTooLong)).to.be.false; @@ -85,13 +256,21 @@ describe('ObjectId', function () { done(); }); - it('should throw if a 12-char string is passed in with character codes greater than 256', function () { - expect(() => new ObjectId('abcdefghijkl').toHexString()).to.not.throw(); - expect(() => new ObjectId('abcdefŽhijkl').toHexString()).to.throw(TypeError); + it('should throw if a 12-char length but non-12 byte string is passed in', function () { + const characterCodesLargerThan256 = 'abcdefŽhijkl'; + const length12Not12Bytes = '🐶🐶🐶🐶🐶🐶'; + expect(() => new ObjectId(characterCodesLargerThan256).toHexString()).to.throw( + TypeError, + 'Argument passed in must be a string of 12 bytes' + ); + expect(() => new ObjectId(length12Not12Bytes).id).to.throw( + TypeError, + 'Argument passed in must be a string of 12 bytes' + ); }); it('should correctly interpret timestamps beyond 2038', function () { - var farFuture = new Date('2040-01-01T00:00:00.000Z').getTime(); + const farFuture = new Date('2040-01-01T00:00:00.000Z').getTime(); expect( new BSON.ObjectId(BSON.ObjectId.generate(farFuture / 1000)).getTimestamp().getTime() ).to.equal(farFuture);