fix(NODE-6355): respect utf8 validation option when iterating cursors (#4220)

baileympearson · web-flow · commit 886cefb4197e · 2024-09-06T16:21:45.000-04:00
diff --git a/.eslintrc.json b/.eslintrc.json
@@ -124,7 +124,8 @@
     "@typescript-eslint/no-unused-vars": [
       "error",
       {
-        "argsIgnorePattern": "^_"
+        "argsIgnorePattern": "^_",
+        "varsIgnorePattern": "^_"
       }
     ]
   },
@@ -275,7 +276,15 @@
       ],
       "parser": "@typescript-eslint/parser",
       "rules": {
-        "unused-imports/no-unused-imports": "error"
+        "unused-imports/no-unused-imports": "error",
+        "@typescript-eslint/consistent-type-imports": [
+          "error",
+          {
+            "prefer": "type-imports",
+            "disallowTypeAnnotations": false,
+            "fixStyle": "separate-type-imports"
+          }
+        ]
       }
     }
   ]
diff --git a/src/bson.ts b/src/bson.ts
@@ -133,3 +133,14 @@ export function resolveBSONOptions(
       options?.enableUtf8Validation ?? parentOptions?.enableUtf8Validation ?? true
   };
 }
+
+/** @internal */
+export function parseUtf8ValidationOption(options?: { enableUtf8Validation?: boolean }): {
+  utf8: { writeErrors: false } | false;
+} {
+  const enableUtf8Validation = options?.enableUtf8Validation;
+  if (enableUtf8Validation === false) {
+    return { utf8: false };
+  }
+  return { utf8: { writeErrors: false } };
+}
diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts
@@ -1,3 +1,4 @@
+import { type DeserializeOptions } from 'bson';
 import { type Readable, Transform, type TransformCallback } from 'stream';
 import { clearTimeout, setTimeout } from 'timers';
 
@@ -487,7 +488,7 @@ export class Connection extends TypedEventEmitter<ConnectionEvents> {
 
     // If `documentsReturnedIn` not set or raw is not enabled, use input bson options
     // Otherwise, support raw flag. Raw only works for cursors that hardcode firstBatch/nextBatch fields
-    const bsonOptions =
+    const bsonOptions: DeserializeOptions =
       options.documentsReturnedIn == null || !options.raw
         ? options
         : {
diff --git a/src/cmap/wire_protocol/on_demand/document.ts b/src/cmap/wire_protocol/on_demand/document.ts
@@ -1,10 +1,11 @@
+import { type DeserializeOptions } from 'bson';
+
 import {
   Binary,
-  BSON,
   type BSONElement,
   BSONError,
-  type BSONSerializeOptions,
   BSONType,
+  deserialize,
   getBigInt64LE,
   getFloat64LE,
   getInt32LE,
@@ -43,6 +44,14 @@ export type JSTypeOf = {
 /** @internal */
 type CachedBSONElement = { element: BSONElement; value: any | undefined };
 
+/**
+ * @internal
+ *
+ * Options for `OnDemandDocument.toObject()`. Validation is required to ensure
+ * that callers provide utf8 validation options. */
+export type OnDemandDocumentDeserializeOptions = Omit<DeserializeOptions, 'validation'> &
+  Required<Pick<DeserializeOptions, 'validation'>>;
+
 /** @internal */
 export class OnDemandDocument {
   /**
@@ -329,8 +338,8 @@ export class OnDemandDocument {
    * Deserialize this object, DOES NOT cache result so avoid multiple invocations
    * @param options - BSON deserialization options
    */
-  public toObject(options?: BSONSerializeOptions): Record<string, any> {
-    return BSON.deserialize(this.bson, {
+  public toObject(options?: OnDemandDocumentDeserializeOptions): Record<string, any> {
+    return deserialize(this.bson, {
       ...options,
       index: this.offset,
       allowObjectSmallerThanBufferSize: true
diff --git a/src/cmap/wire_protocol/responses.ts b/src/cmap/wire_protocol/responses.ts
@@ -1,17 +1,24 @@
+import { type DeserializeOptions } from 'bson';
+
 import {
   type BSONElement,
   type BSONSerializeOptions,
   BSONType,
   type Document,
   Long,
   parseToElementsToArray,
+  parseUtf8ValidationOption,
   pluckBSONSerializeOptions,
   type Timestamp
 } from '../../bson';
 import { MongoUnexpectedServerResponseError } from '../../error';
 import { type ClusterTime } from '../../sdam/common';
 import { decorateDecryptionResult, ns } from '../../utils';
-import { type JSTypeOf, OnDemandDocument } from './on_demand/document';
+import {
+  type JSTypeOf,
+  OnDemandDocument,
+  type OnDemandDocumentDeserializeOptions
+} from './on_demand/document';
 
 // eslint-disable-next-line no-restricted-syntax
 const enum BSONElementOffset {
@@ -113,7 +120,8 @@ export class MongoDBResponse extends OnDemandDocument {
       this.get('recoveryToken', BSONType.object)?.toObject({
         promoteValues: false,
         promoteLongs: false,
-        promoteBuffers: false
+        promoteBuffers: false,
+        validation: { utf8: true }
       }) ?? null
     );
   }
@@ -170,20 +178,10 @@ export class MongoDBResponse extends OnDemandDocument {
   public override toObject(options?: BSONSerializeOptions): Record<string, any> {
     const exactBSONOptions = {
       ...pluckBSONSerializeOptions(options ?? {}),
-      validation: this.parseBsonSerializationOptions(options)
+      validation: parseUtf8ValidationOption(options)
     };
     return super.toObject(exactBSONOptions);
   }
-
-  private parseBsonSerializationOptions(options?: { enableUtf8Validation?: boolean }): {
-    utf8: { writeErrors: false } | false;
-  } {
-    const enableUtf8Validation = options?.enableUtf8Validation;
-    if (enableUtf8Validation === false) {
-      return { utf8: false };
-    }
-    return { utf8: { writeErrors: false } };
-  }
 }
 
 /** @internal */
@@ -267,12 +265,13 @@ export class CursorResponse extends MongoDBResponse {
       this.cursor.get('postBatchResumeToken', BSONType.object)?.toObject({
         promoteValues: false,
         promoteLongs: false,
-        promoteBuffers: false
+        promoteBuffers: false,
+        validation: { utf8: true }
       }) ?? null
     );
   }
 
-  public shift(options?: BSONSerializeOptions): any {
+  public shift(options: OnDemandDocumentDeserializeOptions): any {
     if (this.iterated >= this.batchSize) {
       return null;
     }
@@ -324,7 +323,7 @@ export class ExplainedCursorResponse extends CursorResponse {
     return this._length;
   }
 
-  override shift(options?: BSONSerializeOptions | undefined) {
+  override shift(options?: DeserializeOptions) {
     if (this._length === 0) return null;
     this._length -= 1;
     return this.toObject(options);
diff --git a/src/cursor/abstract_cursor.ts b/src/cursor/abstract_cursor.ts
@@ -1,6 +1,7 @@
 import { Readable, Transform } from 'stream';
 
 import { type BSONSerializeOptions, type Document, Long, pluckBSONSerializeOptions } from '../bson';
+import { type OnDemandDocumentDeserializeOptions } from '../cmap/wire_protocol/on_demand/document';
 import { type CursorResponse } from '../cmap/wire_protocol/responses';
 import {
   MongoAPIError,
@@ -153,6 +154,9 @@ export abstract class AbstractCursor<
   /** @event */
   static readonly CLOSE = 'close' as const;
 
+  /** @internal */
+  protected deserializationOptions: OnDemandDocumentDeserializeOptions;
+
   /** @internal */
   protected constructor(
     client: MongoClient,
@@ -207,6 +211,13 @@ export abstract class AbstractCursor<
     } else {
       this.cursorSession = this.cursorClient.startSession({ owner: this, explicit: false });
     }
+
+    this.deserializationOptions = {
+      ...this.cursorOptions,
+      validation: {
+        utf8: options?.enableUtf8Validation === false ? false : true
+      }
+    };
   }
 
   /**
@@ -289,7 +300,7 @@ export abstract class AbstractCursor<
     );
 
     for (let count = 0; count < documentsToRead; count++) {
-      const document = this.documents?.shift(this.cursorOptions);
+      const document = this.documents?.shift(this.deserializationOptions);
       if (document != null) {
         bufferedDocs.push(document);
       }
@@ -390,7 +401,7 @@ export abstract class AbstractCursor<
     }
 
     do {
-      const doc = this.documents?.shift(this.cursorOptions);
+      const doc = this.documents?.shift(this.deserializationOptions);
       if (doc != null) {
         if (this.transform != null) return await this.transformDocument(doc);
         return doc;
@@ -409,15 +420,15 @@ export abstract class AbstractCursor<
       throw new MongoCursorExhaustedError();
     }
 
-    let doc = this.documents?.shift(this.cursorOptions);
+    let doc = this.documents?.shift(this.deserializationOptions);
     if (doc != null) {
       if (this.transform != null) return await this.transformDocument(doc);
       return doc;
     }
 
     await this.fetchBatch();
 
-    doc = this.documents?.shift(this.cursorOptions);
+    doc = this.documents?.shift(this.deserializationOptions);
     if (doc != null) {
       if (this.transform != null) return await this.transformDocument(doc);
       return doc;
diff --git a/src/cursor/aggregation_cursor.ts b/src/cursor/aggregation_cursor.ts
@@ -74,7 +74,7 @@ export class AggregationCursor<TSchema = any> extends AbstractCursor<TSchema> {
           explain: verbosity ?? true
         })
       )
-    ).shift(this.aggregateOptions);
+    ).shift(this.deserializationOptions);
   }
 
   /** Add a stage to the aggregation pipeline
diff --git a/src/cursor/find_cursor.ts b/src/cursor/find_cursor.ts
@@ -143,7 +143,7 @@ export class FindCursor<TSchema = any> extends AbstractCursor<TSchema> {
           explain: verbosity ?? true
         })
       )
-    ).shift(this.findOptions);
+    ).shift(this.deserializationOptions);
   }
 
   /** Set the cursor query */
diff --git a/src/index.ts b/src/index.ts
@@ -299,7 +299,11 @@ export type { ClientMetadata, ClientMetadataOptions } from './cmap/handshake/cli
 export type { ConnectionPoolMetrics } from './cmap/metrics';
 export type { StreamDescription, StreamDescriptionOptions } from './cmap/stream_description';
 export type { CompressorName } from './cmap/wire_protocol/compression';
-export type { JSTypeOf, OnDemandDocument } from './cmap/wire_protocol/on_demand/document';
+export type {
+  JSTypeOf,
+  OnDemandDocument,
+  OnDemandDocumentDeserializeOptions
+} from './cmap/wire_protocol/on_demand/document';
 export type {
   CursorResponse,
   MongoDBResponse,
diff --git a/test/integration/change-streams/change_stream.test.ts b/test/integration/change-streams/change_stream.test.ts
@@ -875,7 +875,7 @@ describe('Change Streams', function () {
         await lastWrite().catch(() => null);
 
         let counter = 0;
-        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+
         for await (const _ of changes) {
           counter += 1;
           if (counter === 2) {
@@ -1027,7 +1027,6 @@ describe('Change Streams', function () {
             changeStream = collection.watch();
 
             const loop = (async function () {
-              // eslint-disable-next-line @typescript-eslint/no-unused-vars
               for await (const _change of changeStream) {
                 return 'loop entered'; // loop should never be entered
               }
diff --git a/test/integration/node-specific/bson-options/utf8_validation.test.ts b/test/integration/node-specific/bson-options/utf8_validation.test.ts
diff --git a/test/integration/server-selection/server_selection.prose.operation_count.test.ts b/test/integration/server-selection/server_selection.prose.operation_count.test.ts
diff --git a/test/unit/cmap/wire_protocol/responses.test.ts b/test/unit/cmap/wire_protocol/responses.test.ts
diff --git a/test/unit/mongo_logger.test.ts b/test/unit/mongo_logger.test.ts

Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,8 @@`
`124`	`124`	`"@typescript-eslint/no-unused-vars": [`
`125`	`125`	`"error",`
`126`	`126`	`{`
`127`		`- "argsIgnorePattern": "^_"`
	`127`	`+ "argsIgnorePattern": "^_",`
	`128`	`+ "varsIgnorePattern": "^_"`
`128`	`129`	`}`
`129`	`130`	`]`
`130`	`131`	`},`
`@@ -275,7 +276,15 @@`
`275`	`276`	`],`
`276`	`277`	`"parser": "@typescript-eslint/parser",`
`277`	`278`	`"rules": {`
`278`		`- "unused-imports/no-unused-imports": "error"`
	`279`	`+ "unused-imports/no-unused-imports": "error",`
	`280`	`+ "@typescript-eslint/consistent-type-imports": [`
	`281`	`+ "error",`
	`282`	`+ {`
	`283`	`+ "prefer": "type-imports",`
	`284`	`+ "disallowTypeAnnotations": false,`
	`285`	`+ "fixStyle": "separate-type-imports"`
	`286`	`+ }`
	`287`	`+ ]`
`279`	`288`	`}`
`280`	`289`	`}`
`281`	`290`	`]`
Original file line number	Diff line number	Diff line change
`@@ -74,7 +74,7 @@ export class AggregationCursor<TSchema = any> extends AbstractCursor<TSchema> {`
`74`	`74`	`explain: verbosity ?? true`
`75`	`75`	`})`
`76`	`76`	`)`
`77`		`- ).shift(this.aggregateOptions);`
	`77`	`+ ).shift(this.deserializationOptions);`
`78`	`78`	`}`
`79`	`79`
`80`	`80`	`/** Add a stage to the aggregation pipeline`
Original file line number	Diff line number	Diff line change
`@@ -143,7 +143,7 @@ export class FindCursor<TSchema = any> extends AbstractCursor<TSchema> {`
`143`	`143`	`explain: verbosity ?? true`
`144`	`144`	`})`
`145`	`145`	`)`
`146`		`- ).shift(this.findOptions);`
	`146`	`+ ).shift(this.deserializationOptions);`
`147`	`147`	`}`
`148`	`148`
`149`	`149`	`/** Set the cursor query */`