diff --git a/src/cmap/command_monitoring_events.ts b/src/cmap/command_monitoring_events.ts index 042fc5ea55..e770f7fedc 100644 --- a/src/cmap/command_monitoring_events.ts +++ b/src/cmap/command_monitoring_events.ts @@ -181,7 +181,7 @@ const OP_QUERY_KEYS = [ function extractCommand(command: WriteProtocolMessageType): Document { if (command instanceof GetMore) { return { - getMore: command.cursorId, + getMore: deepCopy(command.cursorId), collection: collectionName(command), batchSize: command.numberToReturn }; @@ -190,15 +190,15 @@ function extractCommand(command: WriteProtocolMessageType): Document { if (command instanceof KillCursor) { return { killCursors: collectionName(command), - cursors: command.cursorIds + cursors: deepCopy(command.cursorIds) }; } if (command instanceof Msg) { - return command.command; + return deepCopy(command.command); } - if (command.query && command.query.$query) { + if (command.query?.$query) { let result: Document; if (command.ns === 'admin.$cmd') { // up-convert legacy command @@ -208,7 +208,7 @@ function extractCommand(command: WriteProtocolMessageType): Document { result = { find: collectionName(command) }; Object.keys(LEGACY_FIND_QUERY_MAP).forEach(key => { if (typeof command.query[key] !== 'undefined') { - result[LEGACY_FIND_QUERY_MAP[key]] = command.query[key]; + result[LEGACY_FIND_QUERY_MAP[key]] = deepCopy(command.query[key]); } }); } @@ -216,7 +216,7 @@ function extractCommand(command: WriteProtocolMessageType): Document { Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => { const legacyKey = key as keyof typeof LEGACY_FIND_OPTIONS_MAP; if (typeof command[legacyKey] !== 'undefined') { - result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = command[legacyKey]; + result[LEGACY_FIND_OPTIONS_MAP[legacyKey]] = deepCopy(command[legacyKey]); } }); @@ -234,11 +234,23 @@ function extractCommand(command: WriteProtocolMessageType): Document { if (command.query.$explain) { return { explain: result }; } - return result; } - return command.query ? command.query : command; + const clonedQuery: Record = {}; + const clonedCommand: Record = {}; + if (command.query) { + for (const k in command.query) { + clonedQuery[k] = deepCopy(command.query[k]); + } + clonedCommand.query = clonedQuery; + } + + for (const k in command) { + if (k === 'query') continue; + clonedCommand[k] = deepCopy(((command as unknown) as Record)[k]); + } + return command.query ? clonedQuery : clonedCommand; } function extractReply(command: WriteProtocolMessageType, reply?: Document) { diff --git a/test/functional/apm.test.js b/test/functional/apm.test.js index 2d42305b92..957ba615f3 100644 --- a/test/functional/apm.test.js +++ b/test/functional/apm.test.js @@ -658,6 +658,43 @@ describe('APM', function () { } }); + // NODE-1502 + it('should not allow mutation of internal state from commands returned by event monitoring', function () { + const started = []; + const succeeded = []; + const client = this.configuration.newClient( + { writeConcern: { w: 1 } }, + { maxPoolSize: 1, monitorCommands: true } + ); + client.on('commandStarted', filterForCommands('insert', started)); + client.on('commandSucceeded', filterForCommands('insert', succeeded)); + let documentToInsert = { a: { b: 1 } }; + return client + .connect() + .then(client => { + const db = client.db(this.configuration.db); + return db.collection('apm_test').insertOne(documentToInsert); + }) + .then(r => { + expect(r).to.have.property('insertedId').that.is.an('object'); + expect(started).to.have.lengthOf(1); + // Check if contents of returned document are equal to document inserted (by value) + expect(documentToInsert).to.deep.equal(started[0].command.documents[0]); + // Check if the returned document is a clone of the original. This confirms that the + // reference is not the same. + expect(documentToInsert !== started[0].command.documents[0]).to.equal(true); + expect(documentToInsert.a !== started[0].command.documents[0].a).to.equal(true); + + started[0].command.documents[0].a.b = 2; + expect(documentToInsert.a.b).to.equal(1); + + expect(started[0].commandName).to.equal('insert'); + expect(started[0].command.insert).to.equal('apm_test'); + expect(succeeded).to.have.lengthOf(1); + return client.close(); + }); + }); + describe('spec tests', function () { before(function () { return setupDatabase(this.configuration); diff --git a/test/functional/multiple_db.test.js b/test/functional/multiple_db.test.js index 740668d72c..739d937641 100644 --- a/test/functional/multiple_db.test.js +++ b/test/functional/multiple_db.test.js @@ -85,7 +85,9 @@ describe('Multiple Databases', function () { { returnDocument: ReturnDocument.AFTER }, function (err) { expect(err).to.not.exist; - client.close(done); + collection.drop(() => { + client.close(done); + }); } ); }); diff --git a/test/unit/cmap/command_monitoring_events.test.js b/test/unit/cmap/command_monitoring_events.test.js new file mode 100644 index 0000000000..7303bbe262 --- /dev/null +++ b/test/unit/cmap/command_monitoring_events.test.js @@ -0,0 +1,64 @@ +'use strict'; + +const { Msg, Query, GetMore, KillCursor } = require('../../../src/cmap/commands'); +const { CommandStartedEvent } = require('../../../src/cmap/command_monitoring_events'); +const { expect } = require('chai'); +const { Long } = require('bson'); + +describe('Command Monitoring Events - unit/cmap', function () { + const commands = [ + new Query('admin.$cmd', { a: { b: 10 }, $query: { b: 10 } }, {}), + new Query('hello', { a: { b: 10 }, $query: { b: 10 } }, {}), + new Msg('admin.$cmd', { b: { c: 20 } }, {}), + new Msg('hello', { b: { c: 20 } }, {}), + new GetMore('admin.$cmd', Long.fromNumber(10)), + new GetMore('hello', Long.fromNumber(10)), + new KillCursor('admin.$cmd', [Long.fromNumber(100), Long.fromNumber(200)]), + new KillCursor('hello', [Long.fromNumber(100), Long.fromNumber(200)]), + { ns: 'admin.$cmd', query: { $query: { a: 16 } } }, + { ns: 'hello there', f1: { h: { a: 52, b: { c: 10, d: [1, 2, 3, 5] } } } } + ]; + + for (const command of commands) { + it(`should make a deep copy of object of type: ${command.constructor.name}`, () => { + const ev = new CommandStartedEvent({ id: 'someId', address: 'someHost' }, command); + if (command instanceof Query) { + if (command.ns === 'admin.$cmd') { + expect(ev.command !== command.query.$query).to.equal(true); + for (const k in command.query.$query) { + expect(ev.command[k]).to.deep.equal(command.query.$query[k]); + } + } else { + expect(ev.command.filter !== command.query.$query).to.equal(true); + for (const k in command.query.$query) { + expect(ev.command.filter[k]).to.deep.equal(command.query.$query[k]); + } + } + } else if (command instanceof Msg) { + expect(ev.command !== command.command).to.equal(true); + expect(ev.command).to.deep.equal(command.command); + } else if (command instanceof GetMore) { + // NOTE: BSON Longs pass strict equality when their internal values are equal + // i.e. + // let l1 = Long(10); + // let l2 = Long(10); + // l1 === l2 // returns true + // expect(ev.command.getMore !== command.cursorId).to.equal(true); + expect(ev.command.getMore).to.deep.equal(command.cursorId); + + ev.command.getMore = Long.fromNumber(50128); + expect(command.cursorId).to.not.deep.equal(ev.command.getMore); + } else if (command instanceof KillCursor) { + expect(ev.command.cursors !== command.cursorIds).to.equal(true); + expect(ev.command.cursors).to.deep.equal(command.cursorIds); + } else if (typeof command === 'object') { + if (command.ns === 'admin.$cmd') { + expect(ev.command !== command.query.$query).to.equal(true); + for (const k in command.query.$query) { + expect(ev.command[k]).to.deep.equal(command.query.$query[k]); + } + } + } + }); + } +});