From 803d01469d4d5112e7906a45d0769cd34c403ebc Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Thu, 5 Sep 2024 15:23:28 -0600 Subject: [PATCH] push for diff --- src/bson.ts | 11 ++ src/cmap/connection.ts | 3 +- src/cmap/wire_protocol/on_demand/document.ts | 16 +- src/cmap/wire_protocol/responses.ts | 24 ++- src/cursor/abstract_cursor.ts | 26 +++- src/cursor/aggregation_cursor.ts | 2 +- src/cursor/find_cursor.ts | 2 +- .../bson-options/utf8_validation.test.ts | 141 ++++-------------- .../unit/cmap/wire_protocol/responses.test.ts | 72 --------- 9 files changed, 89 insertions(+), 208 deletions(-) diff --git a/src/bson.ts b/src/bson.ts index f7e0fb56be2..260b60aba18 100644 --- a/src/bson.ts +++ b/src/bson.ts @@ -132,3 +132,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 index e1fb3f96631..666e92fb8c2 100644 --- 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 { // 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 index 67f5b3a091d..b92e6012ff7 100644 --- a/src/cmap/wire_protocol/on_demand/document.ts +++ b/src/cmap/wire_protocol/on_demand/document.ts @@ -1,8 +1,9 @@ +import { type DeserializeOptions } from 'bson'; + import { Binary, type BSONElement, BSONError, - type BSONSerializeOptions, BSONType, deserialize, getBigInt64LE, @@ -10,7 +11,6 @@ import { getInt32LE, ObjectId, parseToElementsToArray, - pluckBSONSerializeOptions, Timestamp, toUTF8 } from '../../../bson'; @@ -330,14 +330,14 @@ export class OnDemandDocument { * Deserialize this object, DOES NOT cache result so avoid multiple invocations * @param options - BSON deserialization options */ - public toObject(options?: BSONSerializeOptions): Record { - const exactBSONOptions = { - ...pluckBSONSerializeOptions(options ?? {}), - validation: this.parseBsonSerializationOptions(options), + public toObject( + options?: DeserializeOptions & { validation: NonNullable } + ): Record { + return deserialize(this.bson, { + ...options, index: this.offset, allowObjectSmallerThanBufferSize: true - }; - return deserialize(this.bson, exactBSONOptions); + }); } private parseBsonSerializationOptions(options?: { enableUtf8Validation?: boolean }): { diff --git a/src/cmap/wire_protocol/responses.ts b/src/cmap/wire_protocol/responses.ts index 9837634bfba..7f4b438f2f1 100644 --- a/src/cmap/wire_protocol/responses.ts +++ b/src/cmap/wire_protocol/responses.ts @@ -1,3 +1,5 @@ +import { type DeserializeOptions } from 'bson'; + import { type BSONElement, type BSONSerializeOptions, @@ -5,6 +7,8 @@ import { type Document, Long, parseToElementsToArray, + parseUtf8ValidationOption, + pluckBSONSerializeOptions, type Timestamp } from '../../bson'; import { MongoUnexpectedServerResponseError } from '../../error'; @@ -112,7 +116,8 @@ export class MongoDBResponse extends OnDemandDocument { this.get('recoveryToken', BSONType.object)?.toObject({ promoteValues: false, promoteLongs: false, - promoteBuffers: false + promoteBuffers: false, + validation: { utf8: true } }) ?? null ); } @@ -165,6 +170,14 @@ export class MongoDBResponse extends OnDemandDocument { } return this.clusterTime ?? null; } + + public override toObject(options?: BSONSerializeOptions): Record { + const exactBSONOptions = { + ...pluckBSONSerializeOptions(options ?? {}), + validation: parseUtf8ValidationOption(options) + }; + return super.toObject(exactBSONOptions); + } } /** @internal */ @@ -248,12 +261,15 @@ 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: DeserializeOptions & { validation: NonNullable } + ): any { if (this.iterated >= this.batchSize) { return null; } @@ -305,7 +321,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 index 4525ae19fe4..eab9f55ae61 100644 --- a/src/cursor/abstract_cursor.ts +++ b/src/cursor/abstract_cursor.ts @@ -1,6 +1,13 @@ +import { type DeserializeOptions } from 'bson'; import { Readable, Transform } from 'stream'; -import { type BSONSerializeOptions, type Document, Long, pluckBSONSerializeOptions } from '../bson'; +import { + type BSONSerializeOptions, + type Document, + Long, + parseUtf8ValidationOption, + pluckBSONSerializeOptions +} from '../bson'; import { type CursorResponse } from '../cmap/wire_protocol/responses'; import { MongoAPIError, @@ -157,6 +164,10 @@ export abstract class AbstractCursor< /** @event */ static readonly CLOSE = 'close' as const; + protected deserializationOptions: DeserializeOptions & { + validation: NonNullable; + }; + /** @internal */ protected constructor( client: MongoClient, @@ -211,6 +222,11 @@ export abstract class AbstractCursor< } else { this.cursorSession = this.cursorClient.startSession({ owner: this, explicit: false }); } + + this.deserializationOptions = { + ...this.cursorOptions, + validation: parseUtf8ValidationOption(this.cursorOptions) + }; } /** @@ -304,7 +320,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); } @@ -406,7 +422,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; @@ -425,7 +441,7 @@ 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; @@ -433,7 +449,7 @@ export abstract class AbstractCursor< 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 index c843f6c47e2..622bce14aa1 100644 --- a/src/cursor/aggregation_cursor.ts +++ b/src/cursor/aggregation_cursor.ts @@ -76,7 +76,7 @@ export class AggregationCursor extends AbstractCursor { 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 index e4b3dbc03c2..ef21cea290f 100644 --- a/src/cursor/find_cursor.ts +++ b/src/cursor/find_cursor.ts @@ -143,7 +143,7 @@ export class FindCursor extends AbstractCursor { explain: verbosity ?? true }) ) - ).shift(this.findOptions); + ).shift(this.deserializationOptions); } /** Set the cursor query */ diff --git a/test/integration/node-specific/bson-options/utf8_validation.test.ts b/test/integration/node-specific/bson-options/utf8_validation.test.ts index d6345a884d0..df367f233b8 100644 --- a/test/integration/node-specific/bson-options/utf8_validation.test.ts +++ b/test/integration/node-specific/bson-options/utf8_validation.test.ts @@ -10,121 +10,43 @@ import { deserialize, type MongoClient, MongoServerError, - OnDemandDocument, OpMsgResponse } from '../../../mongodb'; -const EXPECTED_VALIDATION_DISABLED_ARGUMENT = { - utf8: false -}; - -const EXPECTED_VALIDATION_ENABLED_ARGUMENT = { - utf8: { - writeErrors: false - } -}; - describe('class MongoDBResponse', () => { - let bsonSpy: sinon.SinonSpy; - - beforeEach(() => { - // @ts-expect-error private function - bsonSpy = sinon.spy(OnDemandDocument.prototype, 'parseBsonSerializationOptions'); - }); - - afterEach(() => { - bsonSpy?.restore(); - bsonSpy = null; - }); - let client; afterEach(async () => { + sinon.restore(); if (client) await client.close(); }); - describe('enableUtf8Validation option set to false', () => { - const option = { enableUtf8Validation: false }; + context( + 'when the server is given a long multibyte utf sequence and there is a writeError that includes invalid utf8', + () => { + let client: MongoClient; + let error: MongoServerError; + beforeEach(async function () { + client = this.configuration.newClient(); - for (const passOptionTo of ['client', 'db', 'collection', 'operation']) { - it(`should disable validation with option passed to ${passOptionTo}`, async function () { - client = this.configuration.newClient(passOptionTo === 'client' ? option : undefined); + async function generateWriteErrorWithInvalidUtf8() { + // Insert a large string of multibyte UTF-8 characters + const _id = '\u{1F92A}'.repeat(100); - const db = client.db('bson_utf8Validation_db', passOptionTo === 'db' ? option : undefined); - const collection = db.collection( - 'bson_utf8Validation_coll', - passOptionTo === 'collection' ? option : undefined - ); + const test = client.db('parsing').collection<{ _id: string }>('parsing'); + await test.insertOne({ _id }); - await collection.insertOne( - { name: 'John Doe' }, - passOptionTo === 'operation' ? option : {} - ); + const spy = sinon.spy(OpMsgResponse.prototype, 'parse'); - expect(bsonSpy).to.have.been.called; - const result = bsonSpy.lastCall.returnValue; - expect(result).to.deep.equal(EXPECTED_VALIDATION_DISABLED_ARGUMENT); - }); - } - }); + error = await test.insertOne({ _id }).catch(error => error); - describe('enableUtf8Validation option set to true', () => { - // define client and option for tests to use - const option = { enableUtf8Validation: true }; - for (const passOptionTo of ['client', 'db', 'collection', 'operation']) { - it(`should enable validation with option passed to ${passOptionTo}`, async function () { - client = this.configuration.newClient(passOptionTo === 'client' ? option : undefined); - await client.connect(); - - const db = client.db('bson_utf8Validation_db', passOptionTo === 'db' ? option : undefined); - const collection = db.collection( - 'bson_utf8Validation_coll', - passOptionTo === 'collection' ? option : undefined - ); - - await collection.insertOne( - { name: 'John Doe' }, - passOptionTo === 'operation' ? option : {} - ); - - expect(bsonSpy).to.have.been.called; - const result = bsonSpy.lastCall.returnValue; - expect(result).to.deep.equal(EXPECTED_VALIDATION_ENABLED_ARGUMENT); - }); - } - }); + // Check that the server sent us broken BSON (bad UTF) + expect(() => { + BSON.deserialize(spy.returnValues[0], { validation: { utf8: true } }); + }).to.throw(BSON.BSONError, /Invalid UTF/i, 'did not generate error with invalid utf8'); + } - describe('enableUtf8Validation option not set', () => { - const option = {}; - for (const passOptionTo of ['client', 'db', 'collection', 'operation']) { - it(`should default to enabled with option passed to ${passOptionTo}`, async function () { - client = this.configuration.newClient(passOptionTo === 'client' ? option : undefined); - await client.connect(); - - const db = client.db('bson_utf8Validation_db', passOptionTo === 'db' ? option : undefined); - const collection = db.collection( - 'bson_utf8Validation_coll', - passOptionTo === 'collection' ? option : undefined - ); - - await collection.insertOne( - { name: 'John Doe' }, - passOptionTo === 'operation' ? option : {} - ); - - expect(bsonSpy).to.have.been.called; - const result = bsonSpy.lastCall.returnValue; - expect(result).to.deep.equal(EXPECTED_VALIDATION_ENABLED_ARGUMENT); - }); - } - }); - - context( - 'when the server is given a long multibyte utf sequence and there is a writeError', - () => { - let client: MongoClient; - beforeEach(async function () { - client = this.configuration.newClient(); + await generateWriteErrorWithInvalidUtf8(); }); afterEach(async function () { @@ -133,22 +55,7 @@ describe('class MongoDBResponse', () => { await client.close(); }); - it('does not throw a UTF-8 parsing error', async () => { - // Insert a large string of multibyte UTF-8 characters - const _id = '\u{1F92A}'.repeat(100); - - const test = client.db('parsing').collection<{ _id: string }>('parsing'); - await test.insertOne({ _id }); - - const spy = sinon.spy(OpMsgResponse.prototype, 'parse'); - - const error = await test.insertOne({ _id }).catch(error => error); - - // Check that the server sent us broken BSON (bad UTF) - expect(() => { - BSON.deserialize(spy.returnValues[0], { validation: { utf8: true } }); - }).to.throw(BSON.BSONError, /Invalid UTF/i); - + it('does not throw a UTF-8 parsing error', function () { // Assert the driver squashed it expect(error).to.be.instanceOf(MongoServerError); expect(error.message).to.match(/duplicate/i); @@ -176,7 +83,9 @@ describe('utf8 validation with cursors', function () { if (providedBuffer.includes(targetBytes)) { if (providedBuffer.split(targetBytes).length !== 2) { sinon.restore(); - const message = `too many target bytes sequences: received ${providedBuffer.split(targetBytes).length}\n. command: ${inspect(deserialize(args[0]), { depth: Infinity })}`; + const message = `too many target bytes sequences: received ${ + providedBuffer.split(targetBytes).length + }\n. command: ${inspect(deserialize(args[0]), { depth: Infinity })}`; throw new Error(message); } const buffer = Buffer.from(providedBuffer.replace(targetBytes, 'c301'.repeat(8)), 'hex'); diff --git a/test/unit/cmap/wire_protocol/responses.test.ts b/test/unit/cmap/wire_protocol/responses.test.ts index 7fccbfc7fc6..9f2527d8a3c 100644 --- a/test/unit/cmap/wire_protocol/responses.test.ts +++ b/test/unit/cmap/wire_protocol/responses.test.ts @@ -1,9 +1,5 @@ import { expect } from 'chai'; -import * as sinon from 'sinon'; -// to spy on the bson module, we must import it from the driver -// eslint-disable-next-line @typescript-eslint/no-restricted-imports -import * as mdb from '../../../../src/bson'; import { CursorResponse, Int32, @@ -17,74 +13,6 @@ describe('class MongoDBResponse', () => { it('is a subclass of OnDemandDocument', () => { expect(new MongoDBResponse(serialize({ ok: 1 }))).to.be.instanceOf(OnDemandDocument); }); - - context('utf8 validation', () => { - let deseriailzeSpy: sinon.SinonStub>; - beforeEach(function () { - const deserialize = mdb.deserialize; - deseriailzeSpy = sinon.stub>().callsFake(deserialize); - sinon.stub(mdb, 'deserialize').get(() => { - return deseriailzeSpy; - }); - }); - afterEach(function () { - sinon.restore(); - }); - - context('when enableUtf8Validation is not specified', () => { - const options = { enableUtf8Validation: undefined }; - it('calls BSON deserialize with writeErrors validation turned off', () => { - const res = new MongoDBResponse(serialize({})); - res.toObject(options); - - expect(deseriailzeSpy).to.have.been.called; - - const [ - { - args: [_buffer, { validation }] - } - ] = deseriailzeSpy.getCalls(); - - expect(validation).to.deep.equal({ utf8: { writeErrors: false } }); - }); - }); - - context('when enableUtf8Validation is true', () => { - const options = { enableUtf8Validation: true }; - it('calls BSON deserialize with writeErrors validation turned off', () => { - const res = new MongoDBResponse(serialize({})); - res.toObject(options); - - expect(deseriailzeSpy).to.have.been.called; - - const [ - { - args: [_buffer, { validation }] - } - ] = deseriailzeSpy.getCalls(); - - expect(validation).to.deep.equal({ utf8: { writeErrors: false } }); - }); - }); - - context('when enableUtf8Validation is false', () => { - const options = { enableUtf8Validation: false }; - it('calls BSON deserialize with all validation disabled', () => { - const res = new MongoDBResponse(serialize({})); - res.toObject(options); - - expect(deseriailzeSpy).to.have.been.called; - - const [ - { - args: [_buffer, { validation }] - } - ] = deseriailzeSpy.getCalls(); - - expect(validation).to.deep.equal({ utf8: false }); - }); - }); - }); }); describe('class CursorResponse', () => {