Skip to content

Commit

Permalink
fix(ObjectId): will now throw if an invalid character is passed
Browse files Browse the repository at this point in the history
If a non-hex string of length 12 is passed into the ObjectId
constructor, we generate a hexString based off of the character
codes of the passed in id. This requires that the passed id consist
entirely of characters with values < 256 in utf-16. Previously,
we let this silently fail and generated invalid hex string. Now,
we throw a TypeError

Fixes NODE-1737

BREAKING CHANGE:
Where code was previously silently erroring, users may now
experience TypeErrors
  • Loading branch information
daprahamian committed Nov 5, 2018
1 parent c6701fc commit 6f30b4e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
15 changes: 14 additions & 1 deletion lib/objectid.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ function convertToHex(bytes) {
return bytes.toString('hex');
}

function makeObjectIdError(invalidString, index) {
const invalidCharacter = invalidString[index];
return new TypeError(
`ObjectId string "${invalidString}" contains invalid character "${invalidCharacter}" with character code (${invalidString.charCodeAt(
index
)}). All character codes for a non-hex string must be less than 256.`
);
}

/**
* A class representation of the BSON ObjectId type.
*/
Expand Down Expand Up @@ -111,7 +120,11 @@ class ObjectId {
}

for (let i = 0; i < this.id.length; i++) {
hexString += hexTable[this.id.charCodeAt(i)];
const hexChar = hexTable[this.id.charCodeAt(i)];
if (typeof hexChar !== 'string') {
throw makeObjectIdError(this.id, i);
}
hexString += hexChar;
}

if (ObjectId.cacheHexString) this.__id = hexString;
Expand Down
5 changes: 5 additions & 0 deletions test/node/object_id_tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,9 @@ describe('ObjectId', function() {
expect(ObjectId.isValid(buff12Bytes)).to.be.true;
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);
});
});

0 comments on commit 6f30b4e

Please sign in to comment.