Skip to content

Commit

Permalink
fix(NODE-6355): respect utf8 validation option when iterating cursors (
Browse files Browse the repository at this point in the history
  • Loading branch information
baileympearson authored Sep 6, 2024
1 parent 25c84a4 commit 886cefb
Show file tree
Hide file tree
Showing 14 changed files with 303 additions and 199 deletions.
13 changes: 11 additions & 2 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@
"@typescript-eslint/no-unused-vars": [
"error",
{
"argsIgnorePattern": "^_"
"argsIgnorePattern": "^_",
"varsIgnorePattern": "^_"
}
]
},
Expand Down Expand Up @@ -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"
}
]
}
}
]
Expand Down
11 changes: 11 additions & 0 deletions src/bson.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 } };
}
3 changes: 2 additions & 1 deletion src/cmap/connection.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { type DeserializeOptions } from 'bson';
import { type Readable, Transform, type TransformCallback } from 'stream';
import { clearTimeout, setTimeout } from 'timers';

Expand Down Expand Up @@ -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
: {
Expand Down
17 changes: 13 additions & 4 deletions src/cmap/wire_protocol/on_demand/document.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { type DeserializeOptions } from 'bson';

import {
Binary,
BSON,
type BSONElement,
BSONError,
type BSONSerializeOptions,
BSONType,
deserialize,
getBigInt64LE,
getFloat64LE,
getInt32LE,
Expand Down Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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
Expand Down
31 changes: 15 additions & 16 deletions src/cmap/wire_protocol/responses.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
);
}
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down
19 changes: 15 additions & 4 deletions src/cursor/abstract_cursor.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -153,6 +154,9 @@ export abstract class AbstractCursor<
/** @event */
static readonly CLOSE = 'close' as const;

/** @internal */
protected deserializationOptions: OnDemandDocumentDeserializeOptions;

/** @internal */
protected constructor(
client: MongoClient,
Expand Down Expand Up @@ -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
}
};
}

/**
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/cursor/aggregation_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/cursor/find_cursor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
6 changes: 5 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions test/integration/change-streams/change_stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 886cefb

Please sign in to comment.