Skip to content

Commit

Permalink
fix(NODE-3724): Fix BSONTypeError and BSONError to correctly handle i…
Browse files Browse the repository at this point in the history
…nstanceof checks (#471)
  • Loading branch information
gjchong25 committed Nov 3, 2021
1 parent 0aa8967 commit d8f334b
Show file tree
Hide file tree
Showing 11 changed files with 93 additions and 56 deletions.
2 changes: 1 addition & 1 deletion src/ensure_buffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { isAnyArrayBuffer } from './parser/utils';
* @param potentialBuffer - The potential buffer
* @returns Buffer the input if potentialBuffer is a buffer, or a buffer that
* wraps a passed in Uint8Array
* @throws TypeError If anything other than a Buffer or Uint8Array is passed in
* @throws BSONTypeError If anything other than a Buffer or Uint8Array is passed in
*/
export function ensureBuffer(potentialBuffer: Buffer | ArrayBufferView | ArrayBuffer): Buffer {
if (ArrayBuffer.isView(potentialBuffer)) {
Expand Down
10 changes: 10 additions & 0 deletions src/error.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,22 @@
/** @public */
export class BSONError extends Error {
constructor(message: string) {
super(message);
Object.setPrototypeOf(this, BSONError.prototype);
}

get name(): string {
return 'BSONError';
}
}

/** @public */
export class BSONTypeError extends TypeError {
constructor(message: string) {
super(message);
Object.setPrototypeOf(this, BSONTypeError.prototype);
}

get name(): string {
return 'BSONTypeError';
}
Expand Down
8 changes: 6 additions & 2 deletions test/binary_parser.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
'use strict';

const BSON = require('./register-bson');
const BSONError = BSON.BSONError;

/**
* Binary Parser.
* Jonas Raoni Soares Silva
Expand All @@ -20,7 +24,7 @@ function BinaryParser(bigEndian, allowExceptions) {

BinaryParser.warn = function warn(msg) {
if (this.allowExceptions) {
throw new Error(msg);
throw new BSONError(msg);
}

return 1;
Expand Down Expand Up @@ -419,7 +423,7 @@ BinaryParserBuffer.prototype.hasNeededBits = function hasNeededBits(neededBits)

BinaryParserBuffer.prototype.checkBuffer = function checkBuffer(neededBits) {
if (!this.hasNeededBits(neededBits)) {
throw new Error('checkBuffer::missing bytes');
throw new BSONError('checkBuffer::missing bytes');
}
};

Expand Down
9 changes: 5 additions & 4 deletions test/node/bigint_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
'use strict';

const BSON = require('../register-bson');
const BSONTypeError = BSON.BSONTypeError;

describe('BSON BigInt Support', function () {
before(function () {
Expand All @@ -13,7 +14,7 @@ describe('BSON BigInt Support', function () {
});
it('Should serialize an int that fits in int32', function () {
const testDoc = { b: BigInt(32) };
expect(() => BSON.serialize(testDoc)).to.throw(TypeError);
expect(() => BSON.serialize(testDoc)).to.throw(BSONTypeError);

// const serializedDoc = BSON.serialize(testDoc);
// // prettier-ignore
Expand All @@ -25,7 +26,7 @@ describe('BSON BigInt Support', function () {

it('Should serialize an int that fits in int64', function () {
const testDoc = { b: BigInt(0x1ffffffff) };
expect(() => BSON.serialize(testDoc)).to.throw(TypeError);
expect(() => BSON.serialize(testDoc)).to.throw(BSONTypeError);

// const serializedDoc = BSON.serialize(testDoc);
// // prettier-ignore
Expand All @@ -37,7 +38,7 @@ describe('BSON BigInt Support', function () {

it('Should serialize an int that fits in decimal128', function () {
const testDoc = { b: BigInt('9223372036854776001') }; // int64 max + 1
expect(() => BSON.serialize(testDoc)).to.throw(TypeError);
expect(() => BSON.serialize(testDoc)).to.throw(BSONTypeError);

// const serializedDoc = BSON.serialize(testDoc);
// // prettier-ignore
Expand All @@ -52,7 +53,7 @@ describe('BSON BigInt Support', function () {
const testDoc = {
b: BigInt('9'.repeat(35))
}; // decimal 128 can only encode 34 digits of precision
expect(() => BSON.serialize(testDoc)).to.throw(TypeError);
expect(() => BSON.serialize(testDoc)).to.throw(BSONTypeError);
// expect(() => BSON.serialize(testDoc)).to.throw();
});

Expand Down
9 changes: 5 additions & 4 deletions test/node/bson_corpus_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

const Buffer = require('buffer').Buffer;
const BSON = require('../register-bson');
const BSONError = BSON.BSONError;
const EJSON = BSON.EJSON;

const deserializeOptions = {
Expand Down Expand Up @@ -89,19 +90,19 @@ const parseErrorForRootDocument = scenario => {
}

if (/Null/.test(parseError.description)) {
expect(caughtError).to.be.instanceOf(Error);
expect(caughtError).to.be.instanceOf(BSONError);
expect(caughtError.message).to.match(/null bytes/);
} else if (/Bad/.test(parseError.description)) {
// There is a number of failing tests that start with 'Bad'
// so this check is essentially making the test optional for now
// This should assert that e is an Error and something about the message
// This should assert that e is a BSONError and something about the message
// TODO(NODE-3637): remove special logic and use expect().to.throw() and add errors to lib
expect(caughtError).to.satisfy(e => {
if (e instanceof Error) return true;
if (e instanceof BSONError) return true;
else this.skip();
});
} else {
expect(caughtError).to.be.instanceOf(Error);
expect(caughtError).to.be.instanceOf(BSONError);
}
});
}
Expand Down
3 changes: 2 additions & 1 deletion test/node/bson_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const Int32 = BSON.Int32;
const Double = BSON.Double;
const MinKey = BSON.MinKey;
const MaxKey = BSON.MaxKey;
const BSONError = BSON.BSONError;
const { BinaryParser } = require('../binary_parser');
const vm = require('vm');
const { assertBuffersEqual } = require('./tools/utils');
Expand All @@ -33,7 +34,7 @@ var ISODate = function (string) {

const match = string.match(ISO_REGEX);
if (!match) {
throw new Error(`Invalid ISO 8601 date given: ${string}`);
throw new BSONError(`Invalid ISO 8601 date given: ${string}`);
}

var date = new Date();
Expand Down
14 changes: 8 additions & 6 deletions test/node/ensure_buffer_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

const { Buffer } = require('buffer');
const { ensureBuffer } = require('../register-bson');
const BSON = require('../register-bson');
const BSONTypeError = BSON.BSONTypeError;

describe('ensureBuffer tests', function () {
it('should be a function', function () {
Expand All @@ -15,7 +17,7 @@ describe('ensureBuffer tests', function () {

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

expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.buffer).to.equal(bufferIn.buffer);
Expand All @@ -29,7 +31,7 @@ describe('ensureBuffer tests', function () {

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

expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.buffer).to.equal(arrayIn.buffer);
Expand All @@ -41,7 +43,7 @@ describe('ensureBuffer tests', function () {

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

expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.buffer).to.equal(arrayBufferIn);
Expand All @@ -57,7 +59,7 @@ describe('ensureBuffer tests', function () {

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

expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.buffer).to.equal(arrayBufferIn);
Expand All @@ -69,7 +71,7 @@ describe('ensureBuffer tests', function () {

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

expect(bufferOut).to.be.an.instanceOf(Buffer);
expect(bufferOut.byteLength).to.equal(3);
Expand All @@ -80,7 +82,7 @@ describe('ensureBuffer tests', function () {
it(`should throw if input is ${typeof item}: ${item}`, function () {
expect(function () {
ensureBuffer(item);
}).to.throw(TypeError);
}).to.throw(BSONTypeError);
});
});

Expand Down
37 changes: 37 additions & 0 deletions test/node/error_test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
'use strict';

const BSON = require('../register-bson');
const BSONTypeError = BSON.BSONTypeError;
const BSONError = BSON.BSONError;

describe('BSONTypeError', function () {
it('should evaluate true on instanceof BSONTypeError and TypeError', function () {
const bsonTypeErr = new BSONTypeError();
expect(bsonTypeErr instanceof BSONTypeError).to.be.true;
expect(bsonTypeErr instanceof TypeError).to.be.true;
expect(bsonTypeErr).to.be.instanceOf(BSONTypeError);
expect(bsonTypeErr).to.be.instanceOf(TypeError);
});

it('should correctly set BSONTypeError name and message properties', function () {
const bsonTypeErr = new BSONTypeError('This is a BSONTypeError message');
expect(bsonTypeErr.name).equals('BSONTypeError');
expect(bsonTypeErr.message).equals('This is a BSONTypeError message');
});
});

describe('BSONError', function () {
it('should evaluate true on instanceof BSONError and Error', function () {
const bsonErr = new BSONError();
expect(bsonErr instanceof BSONError).to.be.true;
expect(bsonErr instanceof Error).to.be.true;
expect(bsonErr).to.be.instanceOf(BSONError);
expect(bsonErr).to.be.instanceOf(Error);
});

it('should correctly set BSONError name and message properties', function () {
const bsonErr = new BSONError('This is a BSONError message');
expect(bsonErr.name).equals('BSONError');
expect(bsonErr.message).equals('This is a BSONError message');
});
});
30 changes: 15 additions & 15 deletions test/node/object_id_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

const Buffer = require('buffer').Buffer;
const BSON = require('../register-bson');
const BSONTypeError = BSON.BSONTypeError;
const util = require('util');
const ObjectId = BSON.ObjectId;

Expand Down Expand Up @@ -33,7 +34,7 @@ describe('ObjectId', function () {

for (const { input, description } of invalidInputs) {
it(`should throw error if ${description} is passed in`, function () {
expect(() => new ObjectId(input)).to.throw(TypeError);
expect(() => new ObjectId(input)).to.throw(BSONTypeError);
});
}

Expand All @@ -44,8 +45,7 @@ describe('ObjectId', function () {
return noArgObjID.toHexString();
}
};

expect(() => new ObjectId(objectIdLike)).to.throw(TypeError);
expect(() => new ObjectId(objectIdLike)).to.throw(BSONTypeError);
});

it('should correctly create ObjectId from object with valid string id', function () {
Expand Down Expand Up @@ -117,15 +117,15 @@ describe('ObjectId', function () {
const objectNullId = {
id: null
};
expect(() => new ObjectId(objectNumId)).to.throw(TypeError);
expect(() => new ObjectId(objectNullId)).to.throw(TypeError);
expect(() => new ObjectId(objectNumId)).to.throw(BSONTypeError);
expect(() => new ObjectId(objectNullId)).to.throw(BSONTypeError);
});

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);
expect(() => new ObjectId(objectInvalid24HexStr)).to.throw(BSONTypeError);
});

it('should correctly create ObjectId from object with invalid string id and toHexString method', function () {
Expand All @@ -144,7 +144,7 @@ describe('ObjectId', 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);
expect(() => new ObjectId(objectInvalidBuffer)).to.throw(BSONTypeError);
});

it('should correctly create ObjectId from object with invalid Buffer id and toHexString method', function () {
Expand Down Expand Up @@ -185,11 +185,11 @@ describe('ObjectId', function () {
});

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);
expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw(BSONTypeError);
expect(() => new ObjectId('thisstringisdefinitelytoolong')).to.throw(BSONTypeError);
expect(() => new ObjectId('tooshort')).to.throw(BSONTypeError);
expect(() => new ObjectId('101010')).to.throw(BSONTypeError);
expect(() => new ObjectId('')).to.throw(BSONTypeError);
});

it('should correctly create ObjectId from 24 hex string', function () {
Expand Down Expand Up @@ -234,7 +234,7 @@ describe('ObjectId', function () {

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);
expect(() => new ObjectId(a)).to.throw(BSONTypeError);
});

it('should correctly allow for node.js inspect to work with ObjectId', function (done) {
Expand All @@ -260,11 +260,11 @@ describe('ObjectId', function () {
const characterCodesLargerThan256 = 'abcdefŽhijkl';
const length12Not12Bytes = '🐶🐶🐶🐶🐶🐶';
expect(() => new ObjectId(characterCodesLargerThan256).toHexString()).to.throw(
TypeError,
BSONTypeError,
'Argument passed in must be a string of 12 bytes'
);
expect(() => new ObjectId(length12Not12Bytes).id).to.throw(
TypeError,
BSONTypeError,
'Argument passed in must be a string of 12 bytes'
);
});
Expand Down
6 changes: 4 additions & 2 deletions test/node/uuid_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ const { Buffer } = require('buffer');
const { Binary, UUID } = require('../register-bson');
const { inspect } = require('util');
const { validate: uuidStringValidate, version: uuidStringVersion } = require('uuid');
const BSON = require('../register-bson');
const BSONTypeError = BSON.BSONTypeError;

// Test values
const UPPERCASE_DASH_SEPARATED_UUID_STRING = 'AAAAAAAA-AAAA-4AAA-AAAA-AAAAAAAAAAAA';
Expand Down Expand Up @@ -76,7 +78,7 @@ describe('UUID', () => {
*/
it('should throw if passed invalid 36-char uuid hex string', () => {
expect(() => new UUID(LOWERCASE_DASH_SEPARATED_UUID_STRING)).to.not.throw();
expect(() => new UUID('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa')).to.throw(TypeError);
expect(() => new UUID('aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa')).to.throw(BSONTypeError);
// Note: The version is missing here ^
});

Expand All @@ -85,7 +87,7 @@ describe('UUID', () => {
*/
it('should throw if passed unsupported argument', () => {
expect(() => new UUID(LOWERCASE_DASH_SEPARATED_UUID_STRING)).to.not.throw();
expect(() => new UUID({})).to.throw(TypeError);
expect(() => new UUID({})).to.throw(BSONTypeError);
});

/**
Expand Down

0 comments on commit d8f334b

Please sign in to comment.