fix(NODE-3662): error checking to make sure that ObjectId results in object with correct properties

src/objectid.ts

@@ -43,57 +43,57 @@ 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');
+    if (inputId instanceof ObjectId) {
+      this[kId] = inputId.id;
+      this.__id = inputId.__id;
+    } else {
+      if (typeof inputId === 'object' && inputId && 'id' in inputId) {
+        if ('toHexString' in inputId && typeof inputId.toHexString === 'function') {
+          this[kId] = Buffer.from(inputId.toHexString(), 'hex');
+        } else if (typeof inputId.id === 'string') {
+          this[kId] = Buffer.from(inputId.id);
+        } else {
+          try {
+            this[kId] = ensureBuffer(inputId.id);
+          } catch {
+            throw new BSONTypeError(
+              'Argument passed in must have an id that is of type string or Buffer'
+            );
+          }
+        }
+      } else if (inputId == null || typeof inputId === 'number') {
+        // The most common use case (blank id, new objectId instance)
+        // Generate a new id
+        this[kId] = ObjectId.generate(typeof inputId === 'number' ? inputId : undefined);
+      } else if (ArrayBuffer.isView(inputId) && inputId.byteLength === 12) {
+        this[kId] = ensureBuffer(inputId);
+      } else if (typeof inputId === 'string') {
+        if (inputId.length === 12) {
+          const bytes = Buffer.from(inputId);
+          if (bytes.byteLength === 12) {
+            this[kId] = bytes;
+          }
+        } else if (inputId.length === 24 && checkForHexRegExp.test(inputId)) {
+          this[kId] = Buffer.from(inputId, 'hex');
+        } else {
+          throw new BSONTypeError(
+            'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters'
+          );
+        }
       } else {
-        this[kId] = typeof id.id === 'string' ? Buffer.from(id.id) : id.id;
+        throw new BSONTypeError('Argument passed in does not match the accepted types');
       }
-    }
-
-    // The most common use case (blank id, new objectId instance)
-    if (id == null || typeof id === 'number') {
-      // 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);
-        if (bytes.byteLength === 12) {
-          this[kId] = bytes;
-        }
-      } else if (id.length === 24 && checkForHexRegExp.test(id)) {
-        this[kId] = Buffer.from(id, 'hex');
-      } else {
-        throw new BSONTypeError(
-          'Argument passed in must be a Buffer or string of 12 bytes or a string of 24 hex characters'
-        );
-      }
-    }
-
-    if (ObjectId.cacheHexString) {
-      this.__id = this.id.toString('hex');
-    }
   }
 
   /**

test/node/object_id_tests.js

@@ -9,6 +9,7 @@ describe('ObjectId', function () {
   /**
    * @ignore
    */
+
   it('should correctly handle objectId timestamps', function (done) {
     // var test_number = {id: ObjectI()};
     var a = ObjectId.createFromTime(1);
@@ -24,6 +25,100 @@ describe('ObjectId', function () {
     done();
   });
 
+  it('should correctly create ObjectId from ObjectId', function () {
+    var tmp = new ObjectId();
+    expect(() => new ObjectId(tmp).toHexString()).to.not.throw();
+  });
+
+  it('should throw error if empty array is passed in', function () {
+    expect(() => new ObjectId([])).to.throw(TypeError);
+  });
+
+  it('should throw error if nonempty array is passed in', function () {
+    expect(() => new ObjectId(['abcdefŽhijkl'])).to.throw(TypeError);
+  });
+
+  it('should throw error if empty object is passed in', function () {
+    expect(() => new ObjectId({})).to.throw(TypeError);
+  });
+
+  it('should throw error if object without an id property is passed in', function () {
+    var tmp = new ObjectId();
+    var objectIdLike = {
+      toHexString: function () {
+        return tmp.toHexString();
+      }
+    };
+
+    expect(() => new ObjectId(objectIdLike)).to.throw(TypeError);
+  });
+
+  it('should throw error if object with non-Buffer non-string id is passed in', function () {
+    var objectNumId = {
+      id: 5
+    };
+    var objectNullId = {
+      id: null
+    };
+
+    expect(() => new ObjectId(objectNumId)).to.throw(TypeError);
+    expect(() => new ObjectId(objectNullId)).to.throw(TypeError);
+  });
+
+  it('should correctly create ObjectId with objectIdLike properties', function () {
+    var tmp = new ObjectId();
+    var objectIdLike = {
+      id: tmp.id,
+      toHexString: function () {
+        return tmp.toHexString();
+      }
+    };
+
+    expect(() => new ObjectId(objectIdLike).toHexString()).to.not.throw(TypeError);
+  });
+
+  it('should correctly create ObjectId from number or null', function () {
+    expect(() => new ObjectId(42).toHexString()).to.not.throw();
+    expect(() => new ObjectId(0x2a).toHexString()).to.not.throw();
+    expect(() => new ObjectId(NaN).toHexString()).to.not.throw();
+    expect(() => new ObjectId(null).toHexString()).to.not.throw();
+  });
+
+  it('should correctly create ObjectId with Buffer or string id', function () {
+    var objectStringId = {
+      id: 'thisisastringid'
+    };
+    var objectBufferId = {
+      id: Buffer.from('AAAAAAAAAAAAAAAAAAAAAAAA', 'hex')
+    };
+    var objectBufferFromArray = {
+      id: Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12])
+    };
+
+    expect(() => new ObjectId(objectStringId).toHexString()).to.not.throw(TypeError);
+    expect(() => new ObjectId(objectBufferId).toHexString()).to.not.throw(TypeError);
+    expect(() => new ObjectId(objectBufferFromArray).toHexString()).to.not.throw(TypeError);
+  });
+
+  it('should throw error if non-12 byte non-24 hex string passed in', function () {
+    expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFG')).to.throw();
+    expect(() => new ObjectId('thisismorethan12chars')).to.throw();
+    expect(() => new ObjectId('101010')).to.throw();
+    expect(() => new ObjectId('')).to.throw();
+  });
+
+  it('should correctly create ObjectId from 12 byte or 24 hex string', function () {
+    expect(() => new ObjectId('AAAAAAAAAAAAAAAAAAAAAAAA').toHexString()).to.not.throw();
+    expect(() => new ObjectId('FFFFFFFFFFFFFFFFFFFFFFFF').toHexString()).to.not.throw();
+    expect(() => new ObjectId('abcdefghijkl').toHexString()).to.not.throw();
+  });
+
+  it('should correctly create ObjectId from 12 byte sequence', function () {
+    var a = '111111111111';
+    expect(() => new ObjectId(a).toHexString()).to.not.throw();
+    expect(Buffer.from(new ObjectId(a).id).equals(Buffer.from('111111111111', 'latin1')));
+  });
+
   /**
    * @ignore
    */