diff --git a/lib/command_utils.js b/lib/command_utils.js index ba8289c326..a8f52b2d73 100644 --- a/lib/command_utils.js +++ b/lib/command_utils.js @@ -2,6 +2,7 @@ const Msg = require('./core/connection/msg').Msg; const KillCursor = require('./core/connection/commands').KillCursor; const GetMore = require('./core/connection/commands').GetMore; +const deepCopy = require('./utils').deepCopy; /** Commands that we want to redact because of the sensitive nature of their contents */ const SENSITIVE_COMMANDS = new Set([ @@ -63,17 +64,17 @@ const extractCommand = command => { let extractedCommand; if (command instanceof GetMore) { extractedCommand = { - getMore: command.cursorId, + getMore: deepCopy(command.cursorId), collection: collectionName(command), batchSize: command.numberToReturn }; } else if (command instanceof KillCursor) { extractedCommand = { killCursors: collectionName(command), - cursors: command.cursorIds + cursors: deepCopy(command.cursorIds) }; } else if (command instanceof Msg) { - extractedCommand = command.command; + extractedCommand = deepCopy(command.command); } else if (command.query && command.query.$query) { let result; if (command.ns === 'admin.$cmd') { @@ -84,12 +85,13 @@ const extractCommand = command => { 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]); }); } Object.keys(LEGACY_FIND_OPTIONS_MAP).forEach(key => { - if (typeof command[key] !== 'undefined') result[LEGACY_FIND_OPTIONS_MAP[key]] = command[key]; + if (typeof command[key] !== 'undefined') + result[LEGACY_FIND_OPTIONS_MAP[key]] = deepCopy(command[key]); }); OP_QUERY_KEYS.forEach(key => { @@ -106,7 +108,7 @@ const extractCommand = command => { extractedCommand = result; } } else { - extractedCommand = command.query || command; + extractedCommand = deepCopy(command.query || command); } const commandName = Object.keys(extractedCommand)[0]; diff --git a/lib/utils.js b/lib/utils.js index b0d8776dda..32dcea72ae 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -871,6 +871,82 @@ function emitWarningOnce(message) { } } +function isSuperset(set, subset) { + set = Array.isArray(set) ? new Set(set) : set; + subset = Array.isArray(subset) ? new Set(subset) : subset; + for (const elem of subset) { + if (!set.has(elem)) { + return false; + } + } + return true; +} + +function isRecord(value, requiredKeys) { + const toString = Object.prototype.toString; + const hasOwnProperty = Object.prototype.hasOwnProperty; + const isObject = v => toString.call(v) === '[object Object]'; + if (!isObject(value)) { + return false; + } + + const ctor = value.constructor; + if (ctor && ctor.prototype) { + if (!isObject(ctor.prototype)) { + return false; + } + + // Check to see if some method exists from the Object exists + if (!hasOwnProperty.call(ctor.prototype, 'isPrototypeOf')) { + return false; + } + } + + if (requiredKeys) { + const keys = Object.keys(value); + return isSuperset(keys, requiredKeys); + } + + return true; +} + +/** + * Make a deep copy of an object + * + * NOTE: This is not meant to be the perfect implementation of a deep copy, + * but instead something that is good enough for the purposes of + * command monitoring. + */ +function deepCopy(value) { + if (value == null) { + return value; + } else if (Array.isArray(value)) { + return value.map(item => deepCopy(item)); + } else if (isRecord(value)) { + const res = {}; + for (const key in value) { + res[key] = deepCopy(value[key]); + } + return res; + } + + const ctor = value.constructor; + if (ctor) { + switch (ctor.name.toLowerCase()) { + case 'date': + return new ctor(Number(value)); + case 'map': + return new Map(value); + case 'set': + return new Set(value); + case 'buffer': + return Buffer.from(value); + } + } + + return value; +} + module.exports = { filterOptions, mergeOptions, @@ -907,5 +983,6 @@ module.exports = { hasAtomicOperators, MONGODB_WARNING_CODE, emitWarning, - emitWarningOnce + emitWarningOnce, + deepCopy }; diff --git a/test/functional/apm.test.js b/test/functional/apm.test.js index 5a7350c02f..8199fb440d 100644 --- a/test/functional/apm.test.js +++ b/test/functional/apm.test.js @@ -701,6 +701,55 @@ describe('APM', function() { } }); + // NODE-3358 + describe('Internal state references', function() { + let client; + + beforeEach(function() { + client = this.configuration.newClient( + { writeConcern: { w: 1 } }, + { maxPoolSize: 1, monitorCommands: true } + ); + }); + + afterEach(function(done) { + client.close(done); + }); + + it('should not allow mutation of internal state from commands returned by event monitoring', function() { + const started = []; + const succeeded = []; + const documentToInsert = { a: { b: 1 } }; + client.on('commandStarted', filterForCommands('insert', started)); + client.on('commandSucceeded', filterForCommands('insert', succeeded)); + 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); + }); + }); + }); + it('should correctly receive the APM events for deleteOne', { metadata: { requires: { topology: ['single', 'replicaset'] } }, diff --git a/test/unit/cmap/apm_events.test.js b/test/unit/cmap/apm_events.test.js new file mode 100644 index 0000000000..869a159e28 --- /dev/null +++ b/test/unit/cmap/apm_events.test.js @@ -0,0 +1,67 @@ +'use strict'; + +const Msg = require('../../../lib/core/connection/msg').Msg; +const GetMore = require('../../../lib/core/connection/commands').GetMore; +const Query = require('../../../lib/core/connection/commands').Query; +const KillCursor = require('../../../lib/core/connection/commands').KillCursor; +const CommandStartedEvent = require('../../../lib/core/connection/apm').CommandStartedEvent; +const expect = require('chai').expect; +const Long = require('bson').Long; + +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(`CommandStartedEvent 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]); + } + } + } + }); + } +});