From bd931e5ee9980bc855285b5028862d7731506bef Mon Sep 17 00:00:00 2001 From: Dan Aprahamian Date: Mon, 18 Dec 2017 17:00:02 -0500 Subject: [PATCH] feat(connection): attempt both ipv6 and ipv4 when no family entered If the user does not specify an IP family, we will attempt to connect first via IPv6, and fall back to IPv4 if that fails Fixes NODE-1222 --- lib/connection/connection.js | 187 +++++++++++++--------- test/tests/functional/connection_tests.js | 127 +++++++++++++++ 2 files changed, 236 insertions(+), 78 deletions(-) create mode 100644 test/tests/functional/connection_tests.js diff --git a/lib/connection/connection.js b/lib/connection/connection.js index b2c57ff2b..b51ecde5b 100644 --- a/lib/connection/connection.js +++ b/lib/connection/connection.js @@ -528,100 +528,131 @@ function merge(options1, options2) { } } -/** - * Connect - * @method - */ -Connection.prototype.connect = function(_options) { - var self = this; - _options = _options || {}; - // Set the connections - if (connectionAccounting) addConnection(this.id, this); - // Check if we are overriding the promoteLongs - if (typeof _options.promoteLongs === 'boolean') { - self.responseOptions.promoteLongs = _options.promoteLongs; - self.responseOptions.promoteValues = _options.promoteValues; - self.responseOptions.promoteBuffers = _options.promoteBuffers; +function makeSSLConnection(self, _options) { + let sslOptions = { + socket: self.connection, + rejectUnauthorized: self.rejectUnauthorized + }; + + // Merge in options + merge(sslOptions, this.options); + merge(sslOptions, _options); + + // Set options for ssl + if (self.ca) sslOptions.ca = self.ca; + if (self.crl) sslOptions.crl = self.crl; + if (self.cert) sslOptions.cert = self.cert; + if (self.key) sslOptions.key = self.key; + if (self.passphrase) sslOptions.passphrase = self.passphrase; + + // Override checkServerIdentity behavior + if (self.checkServerIdentity === false) { + // Skip the identiy check by retuning undefined as per node documents + // https://nodejs.org/api/tls.html#tls_tls_connect_options_callback + sslOptions.checkServerIdentity = function() { + return undefined; + }; + } else if (typeof self.checkServerIdentity === 'function') { + sslOptions.checkServerIdentity = self.checkServerIdentity; + } + + // Set default sni servername to be the same as host + if (sslOptions.servername == null) { + sslOptions.servername = self.host; } + // Attempt SSL connection + const connection = tls.connect(self.port, self.host, sslOptions, function() { + // Error on auth or skip + if (connection.authorizationError && self.rejectUnauthorized) { + return self.emit('error', connection.authorizationError, self, { ssl: true }); + } + + // Set socket timeout instead of connection timeout + connection.setTimeout(self.socketTimeout); + // We are done emit connect + self.emit('connect', self); + }); + connection.setTimeout(self.connectionTimeout); + + return connection; +} + +function makeUnsecureConnection(self, family) { // Create new connection instance - var connection_options; + let connection_options; if (self.domainSocket) { connection_options = { path: self.host }; } else { connection_options = { port: self.port, host: self.host }; - if (self.family !== void 0) { - connection_options.family = self.family; - } + connection_options.family = family; } - self.connection = net.createConnection(connection_options); - // Set the options for the connection - self.connection.setKeepAlive(self.keepAlive, self.keepAliveInitialDelay); - self.connection.setTimeout(self.connectionTimeout); - self.connection.setNoDelay(self.noDelay); - - // If we have ssl enabled - if (self.ssl) { - var sslOptions = { - socket: self.connection, - rejectUnauthorized: self.rejectUnauthorized - }; + const connection = net.createConnection(connection_options); - // Merge in options - merge(sslOptions, this.options); - merge(sslOptions, _options); - - // Set options for ssl - if (self.ca) sslOptions.ca = self.ca; - if (self.crl) sslOptions.crl = self.crl; - if (self.cert) sslOptions.cert = self.cert; - if (self.key) sslOptions.key = self.key; - if (self.passphrase) sslOptions.passphrase = self.passphrase; - - // Override checkServerIdentity behavior - if (self.checkServerIdentity === false) { - // Skip the identiy check by retuning undefined as per node documents - // https://nodejs.org/api/tls.html#tls_tls_connect_options_callback - sslOptions.checkServerIdentity = function() { - return undefined; - }; - } else if (typeof self.checkServerIdentity === 'function') { - sslOptions.checkServerIdentity = self.checkServerIdentity; - } - - // Set default sni servername to be the same as host - if (sslOptions.servername == null) { - sslOptions.servername = self.host; - } + // Set the options for the connection + connection.setKeepAlive(self.keepAlive, self.keepAliveInitialDelay); + connection.setTimeout(self.connectionTimeout); + connection.setNoDelay(self.noDelay); + + connection.once('connect', function() { + // Set socket timeout instead of connection timeout + connection.setTimeout(self.socketTimeout); + // Emit connect event + self.emit('connect', self); + }); + + return connection; +} - // Attempt SSL connection - self.connection = tls.connect(self.port, self.host, sslOptions, function() { - // Error on auth or skip - if (self.connection.authorizationError && self.rejectUnauthorized) { - return self.emit('error', self.connection.authorizationError, self, { ssl: true }); - } - - // Set socket timeout instead of connection timeout - self.connection.setTimeout(self.socketTimeout); - // We are done emit connect - self.emit('connect', self); - }); - self.connection.setTimeout(self.connectionTimeout); - } else { - self.connection.once('connect', function() { - // Set socket timeout instead of connection timeout - self.connection.setTimeout(self.socketTimeout); - // Emit connect event - self.emit('connect', self); - }); - } +function doConnect(self, family, _options, _errorHandler) { + self.connection = self.ssl + ? makeSSLConnection(self, _options) + : makeUnsecureConnection(self, family); // Add handlers for events - self.connection.once('error', errorHandler(self)); + self.connection.once('error', _errorHandler); self.connection.once('timeout', timeoutHandler(self)); self.connection.once('close', closeHandler(self)); self.connection.on('data', dataHandler(self)); +} + +/** + * Connect + * @method + */ +Connection.prototype.connect = function(_options) { + _options = _options || {}; + // Set the connections + if (connectionAccounting) addConnection(this.id, this); + // Check if we are overriding the promoteLongs + if (typeof _options.promoteLongs === 'boolean') { + this.responseOptions.promoteLongs = _options.promoteLongs; + this.responseOptions.promoteValues = _options.promoteValues; + this.responseOptions.promoteBuffers = _options.promoteBuffers; + } + + const _errorHandler = errorHandler(this); + + if (this.family !== void 0) { + return doConnect(this, this.family, _options, _errorHandler); + } + + return doConnect(this, 6, _options, err => { + if (this.logger.isDebug()) { + this.logger.debug( + f( + 'connection %s for [%s:%s] errored out with [%s]', + this.id, + this.host, + this.port, + JSON.stringify(err) + ) + ); + } + + return doConnect(this, 4, _options, _errorHandler); + }); }; /** diff --git a/test/tests/functional/connection_tests.js b/test/tests/functional/connection_tests.js new file mode 100644 index 000000000..6b469b404 --- /dev/null +++ b/test/tests/functional/connection_tests.js @@ -0,0 +1,127 @@ +'use strict'; + +const bson = require('bson'); +const expect = require('chai').expect; +const mock = require('../../mock'); +const Connection = require('../../../lib/connection/connection'); + +describe('Connection', function() { + const noop = () => {}; + let server; + afterEach(() => mock.cleanup()); + + function testCase(name, options) { + const config = options.config; + const args = { + metadata: { requires: { topology: ['single'] } }, + test: function(done) { + const connection = new Connection( + noop, + Object.assign( + { + bson, + port: server.port + }, + config + ) + ); + + const cleanup = err => { + connection.destroy(); + done(err); + }; + + const errorHandler = options.error + ? err => { + try { + options.error(err); + cleanup(); + } catch (e) { + cleanup(e); + } + } + : cleanup; + + const connectHandler = options.connect + ? () => { + try { + options.connect(connection); + cleanup(); + } catch (e) { + cleanup(e); + } + } + : () => { + cleanup(new Error('Expected test to not connect, but it connected successfully')); + }; + + connection.on('error', errorHandler); + connection.on('connect', connectHandler); + connection.connect(); + } + }; + + if (options.skip) { + it.skip(name, args); + } else if (options.only) { + it.only(name, args); + } else { + it(name, args); + } + } + + describe('IPv4', function() { + beforeEach(() => mock.createServer(0, '127.0.0.1').then(_server => (server = _server))); + + testCase('should connect with no family', { + config: { host: 'localhost' }, + connect: connection => { + expect(connection.connection.remotePort).to.equal(server.port); + expect(connection.connection.remoteFamily).to.equal('IPv4'); + } + }); + + testCase('should connect with family=4', { + config: { host: 'localhost', family: 4 }, + connect: connection => { + expect(connection.connection.remotePort).to.equal(server.port); + expect(connection.connection.remoteFamily).to.equal('IPv4'); + } + }); + + testCase('should error with family=6', { + config: { host: 'localhost', family: 6 }, + error: err => expect(err).to.be.an.instanceOf(Error) + }); + }); + + describe('IPv6', function() { + beforeEach(() => mock.createServer(0, '::').then(_server => (server = _server))); + + testCase('should connect with no family', { + config: { host: 'localhost' }, + connect: connection => { + expect(connection.connection.remotePort).to.equal(server.port); + expect(connection.connection.remoteFamily).to.equal('IPv6'); + } + }); + + // NOTE: this test is currently being skipped b/c of a "feature" in + // most operating systems where listening on an IPv6 port + // also listens on an IPv4 port. Don't want to spend time working around + // this. See https://github.com/nodejs/node/issues/9390 for more info. + testCase('should error with family=4', { + skip: true, + config: { host: 'localhost', family: 4 }, + error: err => expect(err).to.be.an.instanceOf(Error) + }); + + testCase('should connect with family=6', { + config: { host: 'localhost', family: 6 }, + connect: connection => { + expect(connection.connection.remotePort).to.equal(server.port); + expect(connection.connection.remoteFamily).to.equal('IPv6'); + } + }); + }); +});