feat(connection): attempt both ipv6 and ipv4 when no family entered

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

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