From 1bef71747678c19c7214048de5b9e3848889248d Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Wed, 22 Apr 2015 16:46:21 -0500 Subject: [PATCH] net: cleanup connect logic Separates out the lookup logic for net.Socket. In the event the `host` property is an IP address, the lookup is skipped. PR-URL: https://github.com/iojs/io.js/pull/1505 Reviewed-By: Chris Dickinson Reviewed-By: Yosuke Furukawa --- lib/net.js | 119 ++++++++++++---------- test/parallel/test-net-dns-lookup-skip.js | 18 ++++ test/parallel/test-net-dns-lookup.js | 2 +- 3 files changed, 85 insertions(+), 54 deletions(-) create mode 100644 test/parallel/test-net-dns-lookup-skip.js diff --git a/lib/net.js b/lib/net.js index 847e417e67f4ca..ce2033f9daf101 100644 --- a/lib/net.js +++ b/lib/net.js @@ -881,64 +881,77 @@ Socket.prototype.connect = function(options, cb) { connect(self, options.path); } else { - const dns = require('dns'); - var host = options.host || 'localhost'; - var port = 0; - var localAddress = options.localAddress; - var localPort = options.localPort; - var dnsopts = { - family: options.family, - hints: 0 - }; - - if (localAddress && !exports.isIP(localAddress)) - throw new TypeError('localAddress must be a valid IP: ' + localAddress); - - if (localPort && typeof localPort !== 'number') - throw new TypeError('localPort should be a number: ' + localPort); - - port = options.port; - if (typeof port !== 'undefined') { - if (typeof port !== 'number' && typeof port !== 'string') - throw new TypeError('port should be a number or string: ' + port); - if (!isLegalPort(port)) - throw new RangeError('port should be >= 0 and < 65536: ' + port); - } - port |= 0; + lookupAndConnect(self, options); + } + return self; +}; - if (dnsopts.family !== 4 && dnsopts.family !== 6) - dnsopts.hints = dns.ADDRCONFIG | dns.V4MAPPED; - debug('connect: find host ' + host); - debug('connect: dns options ' + dnsopts); - self._host = host; - dns.lookup(host, dnsopts, function(err, ip, addressType) { - self.emit('lookup', err, ip, addressType); +function lookupAndConnect(self, options) { + const dns = require('dns'); + var host = options.host || 'localhost'; + var port = options.port; + var localAddress = options.localAddress; + var localPort = options.localPort; - // It's possible we were destroyed while looking this up. - // XXX it would be great if we could cancel the promise returned by - // the look up. - if (!self._connecting) return; + if (localAddress && !exports.isIP(localAddress)) + throw new TypeError('localAddress must be a valid IP: ' + localAddress); - if (err) { - // net.createConnection() creates a net.Socket object and - // immediately calls net.Socket.connect() on it (that's us). - // There are no event listeners registered yet so defer the - // error event to the next tick. - process.nextTick(connectErrorNT, self, err, options); - } else { - self._unrefTimer(); - connect(self, - ip, - port, - addressType, - localAddress, - localPort); - } - }); + if (localPort && typeof localPort !== 'number') + throw new TypeError('localPort should be a number: ' + localPort); + + if (typeof port !== 'undefined') { + if (typeof port !== 'number' && typeof port !== 'string') + throw new TypeError('port should be a number or string: ' + port); + if (!isLegalPort(port)) + throw new RangeError('port should be >= 0 and < 65536: ' + port); } - return self; -}; + port |= 0; + + // If host is an IP, skip performing a lookup + // TODO(evanlucas) should we hot path this for localhost? + var addressType = exports.isIP(host); + if (addressType) { + connect(self, host, port, addressType, localAddress, localPort); + return; + } + + var dnsopts = { + family: options.family, + hints: 0 + }; + + if (dnsopts.family !== 4 && dnsopts.family !== 6) + dnsopts.hints = dns.ADDRCONFIG | dns.V4MAPPED; + + debug('connect: find host ' + host); + debug('connect: dns options ' + dnsopts); + self._host = host; + dns.lookup(host, dnsopts, function(err, ip, addressType) { + self.emit('lookup', err, ip, addressType); + + // It's possible we were destroyed while looking this up. + // XXX it would be great if we could cancel the promise returned by + // the look up. + if (!self._connecting) return; + + if (err) { + // net.createConnection() creates a net.Socket object and + // immediately calls net.Socket.connect() on it (that's us). + // There are no event listeners registered yet so defer the + // error event to the next tick. + process.nextTick(connectErrorNT, self, err, options); + } else { + self._unrefTimer(); + connect(self, + ip, + port, + addressType, + localAddress, + localPort); + } + }); +} function connectErrorNT(self, err, options) { diff --git a/test/parallel/test-net-dns-lookup-skip.js b/test/parallel/test-net-dns-lookup-skip.js new file mode 100644 index 00000000000000..7a129b979510b7 --- /dev/null +++ b/test/parallel/test-net-dns-lookup-skip.js @@ -0,0 +1,18 @@ +var common = require('../common'); +var assert = require('assert'); +var net = require('net'); + +function check(addressType) { + var server = net.createServer(function(client) { + client.end(); + server.close(); + }); + + var address = addressType === 4 ? '127.0.0.1' : '::1'; + server.listen(common.PORT, address, function() { + net.connect(common.PORT, address).on('lookup', assert.fail); + }); +} + +check(4); +common.hasIPv6 && check(6); diff --git a/test/parallel/test-net-dns-lookup.js b/test/parallel/test-net-dns-lookup.js index e7c058fe144a56..92ba794d74520c 100644 --- a/test/parallel/test-net-dns-lookup.js +++ b/test/parallel/test-net-dns-lookup.js @@ -9,7 +9,7 @@ var server = net.createServer(function(client) { }); server.listen(common.PORT, '127.0.0.1', function() { - net.connect(common.PORT, '127.0.0.1').on('lookup', function(err, ip, type) { + net.connect(common.PORT, 'localhost').on('lookup', function(err, ip, type) { assert.equal(err, null); assert.equal(ip, '127.0.0.1'); assert.equal(type, '4');