feat(NODE-1921)!: validate serializer root input

docs/upgrade-to-v5.md

@@ -179,9 +179,31 @@ EJSON.parse("...",  { strict: false }); /* migrate to */ EJSON.parse("...",  { r
 // stringify
 EJSON.stringify({}, { strict: true  }); /* migrate to */ EJSON.stringify({}, { relaxed: false });
 EJSON.stringify({}, { strict: false }); /* migrate to */ EJSON.stringify({}, { relaxed: true });
+```
 
 ### The BSON default export has been removed.
 
 * If you import BSON commonjs style `const BSON = require('bson')` then the `BSON.default` property is no longer present.
 * If you import BSON esmodule style `import BSON from 'bson'` then this code will crash upon loading. **TODO: This is not the case right now but it will be after NODE-4713.**
   * This error will throw: `SyntaxError: The requested module 'bson' does not provide an export named 'default'`.
+
+### `BSON.serialize()` validation
+
+The BSON format does not support encoding arrays as the **root** object.
+However, in javascript arrays are just objects where the keys are numeric (and a magic `length` property), so round tripping an array (ex. `[1, 2]`) though BSON would return `{ '0': 1, '1': 2 }`.
+
+`BSON.serialize()` now validates input types, the input to serialize must be an object or a `Map`, arrays will now cause an error.
+
+```typescript
+BSON.serialize([1, 2, 3])
+// BSONError: serialize does not support an array as the root input
+```
+
+if the functionality of turning arrays into an object with numeric keys is useful see the following example:
+
+```typescript
+// Migration example:
+const result = BSON.serialize(Object.fromEntries([1, true, 'blue'].entries()))
+BSON.deserialize(result)
+// { '0': 1, '1': true, '2': 'blue' }
+```

src/bson.ts

@@ -109,7 +109,7 @@ export function serialize(object: Document, options: SerializeOptions = {}): Uin
     0,
     serializeFunctions,
     ignoreUndefined,
-    []
+    null
   );
 
   // Create the final buffer
@@ -152,7 +152,8 @@ export function serializeWithBufferAndIndex(
     0,
     0,
     serializeFunctions,
-    ignoreUndefined
+    ignoreUndefined,
+    null
   );
 
   finalBuffer.set(buffer.subarray(0, serializationIndex), startIndex);

src/parser/serializer.ts

@@ -278,18 +278,18 @@ function serializeObject(
   key: string,
   value: Document,
   index: number,
-  checkKeys = false,
-  depth = 0,
-  serializeFunctions = false,
-  ignoreUndefined = true,
-  path: Document[] = []
+  checkKeys: boolean,
+  depth: number,
+  serializeFunctions: boolean,
+  ignoreUndefined: boolean,
+  path: Set<Document>
 ) {
-  for (let i = 0; i < path.length; i++) {
-    if (path[i] === value) throw new BSONError('cyclic dependency detected');
+  if (path.has(value)) {
+    throw new BSONError('Cannot convert circular structure to BSON');
   }
 
-  // Push value to stack
-  path.push(value);
+  path.add(value);
+
   // Write the type
   buffer[index++] = Array.isArray(value) ? constants.BSON_DATA_ARRAY : constants.BSON_DATA_OBJECT;
   // Number of written bytes
@@ -307,8 +307,9 @@ function serializeObject(
     ignoreUndefined,
     path
   );
-  // Pop stack
-  path.pop();
+
+  path.delete(value);
+
   return endIndex;
 }
 
@@ -425,7 +426,8 @@ function serializeCode(
   checkKeys = false,
   depth = 0,
   serializeFunctions = false,
-  ignoreUndefined = true
+  ignoreUndefined = true,
+  path: Set<Document>
 ) {
   if (value.scope && typeof value.scope === 'object') {
     // Write the type
@@ -456,7 +458,6 @@ function serializeCode(
     // Write the
     index = index + codeSize + 4;
 
-    //
     // Serialize the scope value
     const endIndex = serializeInto(
       buffer,
@@ -465,7 +466,8 @@ function serializeCode(
       index,
       depth + 1,
       serializeFunctions,
-      ignoreUndefined
+      ignoreUndefined,
+      path
     );
     index = endIndex - 1;
 
@@ -570,7 +572,8 @@ function serializeDBRef(
   value: DBRef,
   index: number,
   depth: number,
-  serializeFunctions: boolean
+  serializeFunctions: boolean,
+  path: Set<Document>
 ) {
   // Write the type
   buffer[index++] = constants.BSON_DATA_OBJECT;
@@ -592,7 +595,16 @@ function serializeDBRef(
   }
 
   output = Object.assign(output, value.fields);
-  const endIndex = serializeInto(buffer, output, false, index, depth + 1, serializeFunctions);
+  const endIndex = serializeInto(
+    buffer,
+    output,
+    false,
+    index,
+    depth + 1,
+    serializeFunctions,
+    true,
+    path
+  );
 
   // Calculate object size
   const size = endIndex - startIndex;
@@ -608,18 +620,41 @@ function serializeDBRef(
 export function serializeInto(
   buffer: Uint8Array,
   object: Document,
-  checkKeys = false,
-  startingIndex = 0,
-  depth = 0,
-  serializeFunctions = false,
-  ignoreUndefined = true,
-  path: Document[] = []
+  checkKeys: boolean,
+  startingIndex: number,
+  depth: number,
+  serializeFunctions: boolean,
+  ignoreUndefined: boolean,
+  path: Set<Document> | null
 ): number {
-  startingIndex = startingIndex || 0;
-  path = path || [];
+  if (path == null) {
+    // We are at the root input
+    if (object == null) {
+      // ONLY the root should turn into an empty document
+      // BSON Empty document has a size of 5 (LE)
+      buffer[0] = 0x05;
+      buffer[1] = 0x00;
+      buffer[2] = 0x00;
+      buffer[3] = 0x00;
+      // All documents end with null terminator
+      buffer[4] = 0x00;
+      return 5;
+    }
+
+    if (Array.isArray(object)) {
+      throw new BSONError('serialize does not support an array as the root input');
+    }
+    if (typeof object !== 'object') {
+      throw new BSONError('serialize does not support non-object as the root input');
+    } else if ('_bsontype' in object && typeof object._bsontype === 'string') {
+      throw new BSONError(`BSON types cannot be serialized as a document`);
+    }
+
+    path = new Set();
+  }
 
   // Push the object to the path
-  path.push(object);
+  path.add(object);
 
   // Start place to serialize into
   let index = startingIndex + 4;
@@ -689,14 +724,15 @@ export function serializeInto(
           checkKeys,
           depth,
           serializeFunctions,
-          ignoreUndefined
+          ignoreUndefined,
+          path
         );
       } else if (value['_bsontype'] === 'Binary') {
         index = serializeBinary(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Symbol') {
         index = serializeSymbol(buffer, key, value, index);
       } else if (value['_bsontype'] === 'DBRef') {
-        index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions);
+        index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions, path);
       } else if (value['_bsontype'] === 'BSONRegExp') {
         index = serializeBSONRegExp(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Int32') {
@@ -787,7 +823,8 @@ export function serializeInto(
           checkKeys,
           depth,
           serializeFunctions,
-          ignoreUndefined
+          ignoreUndefined,
+          path
         );
       } else if (typeof value === 'function' && serializeFunctions) {
         index = serializeFunction(buffer, key, value, index, checkKeys, depth);
@@ -796,7 +833,7 @@ export function serializeInto(
       } else if (value['_bsontype'] === 'Symbol') {
         index = serializeSymbol(buffer, key, value, index);
       } else if (value['_bsontype'] === 'DBRef') {
-        index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions);
+        index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions, path);
       } else if (value['_bsontype'] === 'BSONRegExp') {
         index = serializeBSONRegExp(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Int32') {
@@ -891,7 +928,8 @@ export function serializeInto(
           checkKeys,
           depth,
           serializeFunctions,
-          ignoreUndefined
+          ignoreUndefined,
+          path
         );
       } else if (typeof value === 'function' && serializeFunctions) {
         index = serializeFunction(buffer, key, value, index, checkKeys, depth);
@@ -900,7 +938,7 @@ export function serializeInto(
       } else if (value['_bsontype'] === 'Symbol') {
         index = serializeSymbol(buffer, key, value, index);
       } else if (value['_bsontype'] === 'DBRef') {
-        index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions);
+        index = serializeDBRef(buffer, key, value, index, depth, serializeFunctions, path);
       } else if (value['_bsontype'] === 'BSONRegExp') {
         index = serializeBSONRegExp(buffer, key, value, index);
       } else if (value['_bsontype'] === 'Int32') {
@@ -914,7 +952,7 @@ export function serializeInto(
   }
 
   // Remove the path
-  path.pop();
+  path.delete(object);
 
   // Final padding byte for object
   buffer[index++] = 0x00;

test/node/bson_test.js

@@ -1849,7 +1849,9 @@ describe('BSON', function () {
 
     // Array
     const array = [new ObjectIdv400(), new OldObjectID(), new ObjectId()];
-    const deserializedArrayAsMap = BSON.deserialize(BSON.serialize(array));
+    const deserializedArrayAsMap = BSON.deserialize(
+      BSON.serialize(Object.fromEntries(array.entries()))
+    );
     const deserializedArray = Object.keys(deserializedArrayAsMap).map(
       x => deserializedArrayAsMap[x]
     );

test/node/circular_reference.test.ts

@@ -0,0 +1,110 @@
+import { expect } from 'chai';
+import * as BSON from '../register-bson';
+import type { Code, Document } from '../..';
+import { inspect, types } from 'node:util';
+import { DBRef } from '../register-bson';
+
+const EJSON = BSON.EJSON;
+
+function setOn(object: Document | unknown[] | Map<string, unknown>, value: unknown) {
+  if (Array.isArray(object)) {
+    object[Math.floor(Math.random() * object.length)] = value;
+  } else if (types.isMap(object)) {
+    // @ts-expect-error: "readonly" map case does not apply
+    object.set('a', value);
+  } else {
+    object.a = value;
+  }
+}
+
+function* generateTests() {
+  for (const makeRoot of [() => new Map(), () => ({})]) {
+    for (const makeNestedType of [() => new Map(), () => ({}), () => []]) {
+      const root = makeRoot();
+      const nested = makeNestedType();
+      setOn(root, nested);
+      setOn(nested, root);
+
+      yield {
+        title: `root that is a ${types.isMap(root) ? 'map' : 'object'} with a nested ${
+          types.isMap(nested) ? 'map' : Array.isArray(nested) ? 'array' : 'object'
+        } with a circular reference to the root throws`,
+        input: root
+      };
+    }
+  }
+}
+
+describe('Cyclic reference detection', () => {
+  context('fuzz BSON circular references', () => {
+    for (const test of generateTests()) {
+      it(test.title, () => {
+        expect(() => BSON.serialize(test.input), inspect(test.input)).to.throw(/circular/);
+      });
+    }
+  });
+
+  context('in Code with scope', () => {
+    it('throws if code.scope is circular', () => {
+      const root: { code: Code | null } = { code: null };
+      root.code = new BSON.Code('function() {}', { a: root });
+      expect(() => BSON.serialize(root)).to.throw(/circular/);
+    });
+  });
+
+  context('in DBRef with fields', () => {
+    it('throws if dbref.fields is circular', () => {
+      const root: { dbref: DBRef | null } = { dbref: null };
+      root.dbref = new BSON.DBRef('test', new BSON.ObjectId(), 'test', { a: root });
+      expect(() => BSON.serialize(root)).to.throw(/circular/);
+    });
+  });
+
+  context('EJSON circular references', () => {
+    it('should throw a helpful error message for input with circular references', function () {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const obj: any = {
+        some: {
+          property: {
+            array: []
+          }
+        }
+      };
+      obj.some.property.array.push(obj.some);
+      expect(() => EJSON.serialize(obj)).to.throw(`\
+Converting circular structure to EJSON:
+    (root) -> some -> property -> array -> index 0
+                \\-----------------------------/`);
+    });
+
+    it('should throw a helpful error message for input with circular references, one-level nested', function () {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const obj: any = {};
+      obj.obj = obj;
+      expect(() => EJSON.serialize(obj)).to.throw(`\
+Converting circular structure to EJSON:
+    (root) -> obj
+       \\-------/`);
+    });
+
+    it('should throw a helpful error message for input with circular references, one-level nested inside base object', function () {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const obj: any = {};
+      obj.obj = obj;
+      expect(() => EJSON.serialize({ foo: obj })).to.throw(`\
+Converting circular structure to EJSON:
+    (root) -> foo -> obj
+               \\------/`);
+    });
+
+    it('should throw a helpful error message for input with circular references, pointing back to base object', function () {
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      const obj: any = { foo: {} };
+      obj.foo.obj = obj;
+      expect(() => EJSON.serialize(obj)).to.throw(`\
+Converting circular structure to EJSON:
+    (root) -> foo -> obj
+       \\--------------/`);
+    });
+  });
+});