From 5f6a7875ae5914f65e96512ce2cea0ea8210f738 Mon Sep 17 00:00:00 2001
From: Matt Broadstone <mbroadst@mongodb.com>
Date: Mon, 11 Nov 2019 13:58:20 -0500
Subject: [PATCH] fix(connect): prevent multiple callbacks in error scenarios

We treat `close`/`timeout`/`parseError`/`error` as error cases
during initial handshake, but some of these do not generate errors.
Additionally, in some cases multiple of these events may be emitted
which leaves us in a bad state if a subsequent `error` message is
emitted, since that will crash the node process.

NODE-2277
---
 lib/core/connection/connect.js  | 18 ++++++++--
 test/core/unit/connect_tests.js | 58 +++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/lib/core/connection/connect.js b/lib/core/connection/connect.js
index 9feafc143f..95ac4406d2 100644
--- a/lib/core/connection/connect.js
+++ b/lib/core/connection/connect.js
@@ -315,11 +315,25 @@ function runCommand(conn, ns, command, options, callback) {
     numberToReturn: 1
   });
 
+  const noop = () => {};
+  function _callback(err, result) {
+    callback(err, result);
+    callback = noop;
+  }
+
   function errorHandler(err) {
     conn.resetSocketTimeout();
     CONNECTION_ERROR_EVENTS.forEach(eventName => conn.removeListener(eventName, errorHandler));
     conn.removeListener('message', messageHandler);
-    callback(err, null);
+
+    if (err == null) {
+      err = new MongoError(`runCommand failed for connection to '${conn.address}'`);
+    }
+
+    // ignore all future errors
+    conn.on('error', noop);
+
+    _callback(err, null);
   }
 
   function messageHandler(msg) {
@@ -332,7 +346,7 @@ function runCommand(conn, ns, command, options, callback) {
     conn.removeListener('message', messageHandler);
 
     msg.parse({ promoteValues: true });
-    callback(null, msg.documents[0]);
+    _callback(null, msg.documents[0]);
   }
 
   conn.setSocketTimeout(socketTimeout);
diff --git a/test/core/unit/connect_tests.js b/test/core/unit/connect_tests.js
index 7d4e8c6a8d..ead9c9df86 100644
--- a/test/core/unit/connect_tests.js
+++ b/test/core/unit/connect_tests.js
@@ -3,6 +3,7 @@
 const BSON = require('bson');
 const mock = require('mongodb-mock-server');
 const expect = require('chai').expect;
+const EventEmitter = require('events');
 
 const connect = require('../../../lib/core/connection/connect');
 const MongoCredentials = require('../../../lib/core/auth/mongo_credentials').MongoCredentials;
@@ -98,4 +99,61 @@ describe('Connect Tests', function() {
       done();
     });
   });
+
+  describe('runCommand', function() {
+    class MockConnection extends EventEmitter {
+      constructor(conn) {
+        super();
+        this.options = { bson: new BSON() };
+        this.conn = conn;
+      }
+
+      get address() {
+        return 'mocked';
+      }
+
+      setSocketTimeout() {}
+      resetSocketTimeout() {}
+      destroy() {
+        this.conn.destroy();
+      }
+    }
+
+    it('should treat non-Error generating error-like events as errors', function(done) {
+      class ConnectionFailingWithClose extends MockConnection {
+        write() {
+          this.emit('close');
+        }
+      }
+
+      connect(
+        { host: '127.0.0.1', port: 27017, connectionType: ConnectionFailingWithClose },
+        (err, conn) => {
+          expect(err).to.exist;
+          expect(err.message).to.match(/runCommand failed/);
+          expect(conn).to.not.exist;
+          done();
+        }
+      );
+    });
+
+    it('should not crash the application if multiple error-like events are emitted on `runCommand`', function(done) {
+      class ConnectionFailingWithAllEvents extends MockConnection {
+        write() {
+          this.emit('close');
+          this.emit('timeout');
+          this.emit('error');
+        }
+      }
+
+      connect(
+        { host: '127.0.0.1', port: 27017, connectionType: ConnectionFailingWithAllEvents },
+        (err, conn) => {
+          expect(err).to.exist;
+          expect(conn).to.not.exist;
+          done();
+        }
+      );
+    });
+  });
 });