Skip to content

Commit

Permalink
net: refactor Server.prototype.listen
Browse files Browse the repository at this point in the history 
This PR simplifies Server.prototype.listen, removing some redundancy and
inconsistency. Because listen and connect have a similar function signature,
normalizeConnectArgs can be reused for listen.
listenAfterLookup renamed to lookupAndListen for consistency with
lookupAndConnect, and moved out of Server.prototype.listen.

PR-URL: #4039
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Glen Keane <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
@jscissr @mcollina
jscissr authored and mcollina committed Sep 12, 2016
1 parent 1ffdbb6 commit fd6af98
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 92 deletions.
2 changes: 1 addition & 1 deletion lib/_tls_wrap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -952,7 +952,7 @@ function SNICallback(servername, callback) {
//
//
function normalizeConnectArgs(listArgs) {
var args = net._normalizeConnectArgs(listArgs);
var args = net._normalizeArgs(listArgs);
var options = args[0];
var cb = args[1];

Expand Down
127 changes: 58 additions & 69 deletions lib/net.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,16 @@ exports.connect = exports.createConnection = function() {
var args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
args = normalizeConnectArgs(args);
args = normalizeArgs(args);
debug('createConnection', args);
var s = new Socket(args[0]);
return Socket.prototype.connect.apply(s, args);
};

// Returns an array [options] or [options, cb]
// Returns an array [options, cb], where cb can be null.
// It is the same as the argument of Socket.prototype.connect().
function normalizeConnectArgs(args) {
// This is used by Server.prototype.listen() and Socket.prototype.connect().
function normalizeArgs(args) {
var options = {};

if (args.length === 0) {
Expand All @@ -91,9 +92,11 @@ function normalizeConnectArgs(args) {
}

var cb = args[args.length - 1];
return typeof cb === 'function' ? [options, cb] : [options];
if (typeof cb !== 'function')
cb = null;
return [options, cb];
}
exports._normalizeConnectArgs = normalizeConnectArgs;
exports._normalizeArgs = normalizeArgs;


// called when creating new Socket, or when re-using a closed Socket
Expand Down Expand Up @@ -888,7 +891,7 @@ Socket.prototype.connect = function(options, cb) {
var args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
args = normalizeConnectArgs(args);
args = normalizeArgs(args);
return Socket.prototype.connect.apply(this, args);
}

Expand Down Expand Up @@ -1325,84 +1328,70 @@ function listen(self, address, port, addressType, backlog, fd, exclusive) {


Server.prototype.listen = function() {
var self = this;
var args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
var [options, cb] = normalizeArgs(args);

var lastArg = arguments[arguments.length - 1];
if (typeof lastArg === 'function') {
self.once('listening', lastArg);
if (typeof cb === 'function') {
this.once('listening', cb);
}

var port = toNumber(arguments[0]);
if (args.length === 0 || typeof args[0] === 'function') {
// Bind to a random port.
options.port = 0;
}

// The third optional argument is the backlog size.
// When the ip is omitted it can be the second argument.
var backlog = toNumber(arguments[1]) || toNumber(arguments[2]);
var backlog = toNumber(args.length > 1 && args[1]) ||
toNumber(args.length > 2 && args[2]);

if (arguments.length === 0 || typeof arguments[0] === 'function') {
// Bind to a random port.
listen(self, null, 0, null, backlog);
} else if (arguments[0] !== null && typeof arguments[0] === 'object') {
var h = arguments[0];
h = h._handle || h.handle || h;

if (h instanceof TCP) {
self._handle = h;
listen(self, null, -1, -1, backlog);
} else if (typeof h.fd === 'number' && h.fd >= 0) {
listen(self, null, null, null, backlog, h.fd);
} else {
// The first argument is a configuration object
if (h.backlog)
backlog = h.backlog;

if (typeof h.port === 'number' || typeof h.port === 'string' ||
(typeof h.port === 'undefined' && 'port' in h)) {
// Undefined is interpreted as zero (random port) for consistency
// with net.connect().
assertPort(h.port);
if (h.host)
listenAfterLookup(h.port | 0, h.host, backlog, h.exclusive);
else
listen(self, null, h.port | 0, 4, backlog, undefined, h.exclusive);
} else if (h.path && isPipeName(h.path)) {
const pipeName = self._pipeName = h.path;
listen(self, pipeName, -1, -1, backlog, undefined, h.exclusive);
} else {
throw new Error('Invalid listen argument: ' + h);
}
}
} else if (isPipeName(arguments[0])) {
// UNIX socket or Windows pipe.
const pipeName = self._pipeName = arguments[0];
listen(self, pipeName, -1, -1, backlog);

} else if (arguments[1] === undefined ||
typeof arguments[1] === 'function' ||
typeof arguments[1] === 'number') {
// The first argument is the port, no IP given.
assertPort(port);
listen(self, null, port, 4, backlog);
options = options._handle || options.handle || options;

if (options instanceof TCP) {
this._handle = options;
listen(this, null, -1, -1, backlog);
} else if (typeof options.fd === 'number' && options.fd >= 0) {
listen(this, null, null, null, backlog, options.fd);
} else {
// The first argument is the port, the second an IP.
assertPort(port);
listenAfterLookup(port, arguments[1], backlog);
}

function listenAfterLookup(port, address, backlog, exclusive) {
require('dns').lookup(address, function(err, ip, addressType) {
if (err) {
self.emit('error', err);
backlog = options.backlog || backlog;

if (typeof options.port === 'number' || typeof options.port === 'string' ||
(typeof options.port === 'undefined' && 'port' in options)) {
// Undefined is interpreted as zero (random port) for consistency
// with net.connect().
assertPort(options.port);
if (options.host) {
lookupAndListen(this, options.port | 0, options.host, backlog,
options.exclusive);
} else {
addressType = ip ? addressType : 4;
listen(self, ip, port, addressType, backlog, undefined, exclusive);
listen(this, null, options.port | 0, 4, backlog, undefined,
options.exclusive);
}
});
} else if (options.path && isPipeName(options.path)) {
// UNIX socket or Windows pipe.
const pipeName = this._pipeName = options.path;
listen(this, pipeName, -1, -1, backlog, undefined, options.exclusive);
} else {
throw new Error('Invalid listen argument: ' + options);
}
}

return self;
return this;
};

function lookupAndListen(self, port, address, backlog, exclusive) {
require('dns').lookup(address, function(err, ip, addressType) {
if (err) {
self.emit('error', err);
} else {
addressType = ip ? addressType : 4;
listen(self, ip, port, addressType, backlog, undefined, exclusive);
}
});
}

Object.defineProperty(Server.prototype, 'listening', {
get: function() {
return !!this._handle;
Expand Down
61 changes: 39 additions & 22 deletions test/parallel/test-net-listen-port-option.js
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,45 @@
'use strict';
var common = require('../common');
var assert = require('assert');
var net = require('net');
const common = require('../common');
const assert = require('assert');
const net = require('net');

function close() { this.close(); }
net.Server().listen({ port: undefined }, close);
net.Server().listen({ port: '' + common.PORT }, close);

[ 'nan',
-1,
123.456,
0x10000,
1 / 0,
-1 / 0,
'+Infinity',
'-Infinity'
].forEach(function(port) {
assert.throws(function() {
net.Server().listen({ port: port }, common.fail);
}, /"port" argument must be >= 0 and < 65536/i);
});
// From lib/net.js
function toNumber(x) { return (x = Number(x)) >= 0 ? x : false; }

function isPipeName(s) {
return typeof s === 'string' && toNumber(s) === false;
}

const listenVariants = [
(port, cb) => net.Server().listen({port}, cb),
(port, cb) => net.Server().listen(port, cb)
];

listenVariants.forEach((listenVariant, i) => {
listenVariant(undefined, common.mustCall(close));
listenVariant('0', common.mustCall(close));

[
'nan',
-1,
123.456,
0x10000,
1 / 0,
-1 / 0,
'+Infinity',
'-Infinity'
].forEach((port) => {
if (i === 1 && isPipeName(port)) {
// skip this, because listen(port) can also be listen(path)
return;
}
assert.throws(() => listenVariant(port, common.fail),
/"port" argument must be >= 0 and < 65536/i);
});

[null, true, false].forEach(function(port) {
assert.throws(function() {
net.Server().listen({ port: port }, common.fail);
}, /invalid listen argument/i);
[null, true, false].forEach((port) =>
assert.throws(() => listenVariant(port, common.fail),
/invalid listen argument/i));
});

1 comment on commit fd6af98

@kainy
Copy link

@kainy kainy commented on fd6af98 Oct 28, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

niubility。

Please sign in to comment.