From 107bae5083d40b8521b5796469ddbc3ea2063c7a Mon Sep 17 00:00:00 2001 From: Dan Aprahamian Date: Tue, 19 Dec 2017 14:27:11 -0500 Subject: [PATCH] feat(connection): attempt both ipv6 and ipv4 when no family entered (#260) 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'); + } + }); + }); +});