Skip to content

Commit

Permalink
net: cleanup connect logic
Browse files Browse the repository at this point in the history
Separates out the lookup logic for net.Socket. In the event
the `host` property is an IP address, the lookup is skipped.

PR-URL: nodejs#1505
Reviewed-By: Chris Dickinson <[email protected]>
Reviewed-By: Yosuke Furukawa <[email protected]>
  • Loading branch information
evanlucas committed Apr 24, 2015
1 parent 3d3083b commit 1bef717
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 54 deletions.
119 changes: 66 additions & 53 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
18 changes: 18 additions & 0 deletions test/parallel/test-net-dns-lookup-skip.js
Original file line number Diff line number Diff line change
@@ -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);
2 changes: 1 addition & 1 deletion test/parallel/test-net-dns-lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down

0 comments on commit 1bef717

Please sign in to comment.