fix(NODE-3358): Command monitoring objects hold internal state references (#2858)

Co-authored-by: Daria Pardue <[email protected]>

Author: W-A-James

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];

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
 };

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'] } },
 

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]);
+          }
+        }
+      }
+    });
+  }
+});