Skip to content

Commit 1bef717

Browse files
committed
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: #1505 Reviewed-By: Chris Dickinson <[email protected]> Reviewed-By: Yosuke Furukawa <[email protected]>
1 parent 3d3083b commit 1bef717

File tree

3 files changed

+85
-54
lines changed

3 files changed

+85
-54
lines changed

‎lib/net.js‎

Lines changed: 66 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -881,64 +881,77 @@ Socket.prototype.connect = function(options, cb){
881881
connect(self,options.path);
882882

883883
}else{
884-
constdns=require('dns');
885-
varhost=options.host||'localhost';
886-
varport=0;
887-
varlocalAddress=options.localAddress;
888-
varlocalPort=options.localPort;
889-
vardnsopts={
890-
family: options.family,
891-
hints: 0
892-
};
893-
894-
if(localAddress&&!exports.isIP(localAddress))
895-
thrownewTypeError('localAddress must be a valid IP: '+localAddress);
896-
897-
if(localPort&&typeoflocalPort!=='number')
898-
thrownewTypeError('localPort should be a number: '+localPort);
899-
900-
port=options.port;
901-
if(typeofport!=='undefined'){
902-
if(typeofport!=='number'&&typeofport!=='string')
903-
thrownewTypeError('port should be a number or string: '+port);
904-
if(!isLegalPort(port))
905-
thrownewRangeError('port should be >= 0 and < 65536: '+port);
906-
}
907-
port|=0;
884+
lookupAndConnect(self,options);
885+
}
886+
returnself;
887+
};
908888

909-
if(dnsopts.family!==4&&dnsopts.family!==6)
910-
dnsopts.hints=dns.ADDRCONFIG|dns.V4MAPPED;
911889

912-
debug('connect: find host '+host);
913-
debug('connect: dns options '+dnsopts);
914-
self._host=host;
915-
dns.lookup(host,dnsopts,function(err,ip,addressType){
916-
self.emit('lookup',err,ip,addressType);
890+
functionlookupAndConnect(self,options){
891+
constdns=require('dns');
892+
varhost=options.host||'localhost';
893+
varport=options.port;
894+
varlocalAddress=options.localAddress;
895+
varlocalPort=options.localPort;
917896

918-
// It's possible we were destroyed while looking this up.
919-
// XXX it would be great if we could cancel the promise returned by
920-
// the look up.
921-
if(!self._connecting)return;
897+
if(localAddress&&!exports.isIP(localAddress))
898+
thrownewTypeError('localAddress must be a valid IP: '+localAddress);
922899

923-
if(err){
924-
// net.createConnection() creates a net.Socket object and
925-
// immediately calls net.Socket.connect() on it (that's us).
926-
// There are no event listeners registered yet so defer the
927-
// error event to the next tick.
928-
process.nextTick(connectErrorNT,self,err,options);
929-
}else{
930-
self._unrefTimer();
931-
connect(self,
932-
ip,
933-
port,
934-
addressType,
935-
localAddress,
936-
localPort);
937-
}
938-
});
900+
if(localPort&&typeoflocalPort!=='number')
901+
thrownewTypeError('localPort should be a number: '+localPort);
902+
903+
if(typeofport!=='undefined'){
904+
if(typeofport!=='number'&&typeofport!=='string')
905+
thrownewTypeError('port should be a number or string: '+port);
906+
if(!isLegalPort(port))
907+
thrownewRangeError('port should be >= 0 and < 65536: '+port);
939908
}
940-
returnself;
941-
};
909+
port|=0;
910+
911+
// If host is an IP, skip performing a lookup
912+
// TODO(evanlucas) should we hot path this for localhost?
913+
varaddressType=exports.isIP(host);
914+
if(addressType){
915+
connect(self,host,port,addressType,localAddress,localPort);
916+
return;
917+
}
918+
919+
vardnsopts={
920+
family: options.family,
921+
hints: 0
922+
};
923+
924+
if(dnsopts.family!==4&&dnsopts.family!==6)
925+
dnsopts.hints=dns.ADDRCONFIG|dns.V4MAPPED;
926+
927+
debug('connect: find host '+host);
928+
debug('connect: dns options '+dnsopts);
929+
self._host=host;
930+
dns.lookup(host,dnsopts,function(err,ip,addressType){
931+
self.emit('lookup',err,ip,addressType);
932+
933+
// It's possible we were destroyed while looking this up.
934+
// XXX it would be great if we could cancel the promise returned by
935+
// the look up.
936+
if(!self._connecting)return;
937+
938+
if(err){
939+
// net.createConnection() creates a net.Socket object and
940+
// immediately calls net.Socket.connect() on it (that's us).
941+
// There are no event listeners registered yet so defer the
942+
// error event to the next tick.
943+
process.nextTick(connectErrorNT,self,err,options);
944+
}else{
945+
self._unrefTimer();
946+
connect(self,
947+
ip,
948+
port,
949+
addressType,
950+
localAddress,
951+
localPort);
952+
}
953+
});
954+
}
942955

943956

944957
functionconnectErrorNT(self,err,options){
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
varcommon=require('../common');
2+
varassert=require('assert');
3+
varnet=require('net');
4+
5+
functioncheck(addressType){
6+
varserver=net.createServer(function(client){
7+
client.end();
8+
server.close();
9+
});
10+
11+
varaddress=addressType===4 ? '127.0.0.1' : '::1';
12+
server.listen(common.PORT,address,function(){
13+
net.connect(common.PORT,address).on('lookup',assert.fail);
14+
});
15+
}
16+
17+
check(4);
18+
common.hasIPv6&&check(6);

‎test/parallel/test-net-dns-lookup.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ var server = net.createServer(function(client){
99
});
1010

1111
server.listen(common.PORT,'127.0.0.1',function(){
12-
net.connect(common.PORT,'127.0.0.1').on('lookup',function(err,ip,type){
12+
net.connect(common.PORT,'localhost').on('lookup',function(err,ip,type){
1313
assert.equal(err,null);
1414
assert.equal(ip,'127.0.0.1');
1515
assert.equal(type,'4');

0 commit comments

Comments
(0)