Skip to content

Commit

Permalink
fix(connect): prevent multiple callbacks in error scenarios
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mbroadst committed Nov 11, 2019
1 parent ba123a6 commit 5f6a787
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 2 deletions.
18 changes: 16 additions & 2 deletions lib/core/connection/connect.js
Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
58 changes: 58 additions & 0 deletions test/core/unit/connect_tests.js
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
}
);
});
});
});

0 comments on commit 5f6a787

Please sign in to comment.