Skip to content

Commit

Permalink
net: refactor overloaded argument handling
Browse files Browse the repository at this point in the history 
* Make normalizeArgs return either [options, null] or [options, cb]
  (the second element won't be undefined anymore) and avoid OOB read
* Use Socket.prototype.connect.call instead of .apply when the number
  of arguments is certain(returned by normalizeArgs).
* Rename some args[i] for readability
* Refactor Server.prototype.listen, separate backlogFromArgs and
  options.backlog, comment the overloading process, refactor control
  flow

PR-URL: #11667
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
@joyeecheung
joyeecheung committed Mar 8, 2017
1 parent 06942d8 commit 53f3869
Showing 1 changed file with 101 additions and 69 deletions.
170 changes: 101 additions & 69 deletions lib/net.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ var cluster;
const errnoException = util._errnoException;
const exceptionWithHostPort = util._exceptionWithHostPort;
const isLegalPort = internalNet.isLegalPort;
const assertPort = internalNet.assertPort;

function noop() {}

Expand Down Expand Up @@ -60,46 +59,60 @@ exports.createServer = function(options, connectionListener) {
// connect(path, [cb]);
//
exports.connect = exports.createConnection = function() {
var args = new Array(arguments.length);
const args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
args = normalizeArgs(args);
debug('createConnection', args);
var s = new Socket(args[0]);
// TODO(joyeecheung): use destructuring when V8 is fast enough
const normalized = normalizeArgs(args);
const options = normalized[0];
const cb = normalized[1];
debug('createConnection', normalized);
const socket = new Socket(options);

if (args[0].timeout) {
s.setTimeout(args[0].timeout);
if (options.timeout) {
socket.setTimeout(options.timeout);
}

return Socket.prototype.connect.apply(s, args);
return Socket.prototype.connect.call(socket, options, cb);
};

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

// Returns an array [options, cb], where options is an object,
// cb is either a funciton or null.
// Used to normalize arguments of Socket.prototype.connect() and
// Server.prototype.listen(). Possible combinations of paramters:
// (options[...][, cb])
// (path[...][, cb])
// ([port][, host][...][, cb])
// For Socket.prototype.connect(), the [...] part is ignored
// For Server.prototype.listen(), the [...] part is [, backlog]
// but will not be handled here (handled in listen())
function normalizeArgs(args) {
if (args.length === 0) {
return [options];
} else if (args[0] !== null && typeof args[0] === 'object') {
// connect(options, [cb])
options = args[0];
} else if (isPipeName(args[0])) {
// connect(path, [cb]);
options.path = args[0];
return [{}, null];
}

const arg0 = args[0];
var options = {};
if (typeof arg0 === 'object' && arg0 !== null) {
// (options[...][, cb])
options = arg0;
} else if (isPipeName(arg0)) {
// (path[...][, cb])
options.path = arg0;
} else {
// connect(port, [host], [cb])
options.port = args[0];
// ([port][, host][...][, cb])
options.port = arg0;
if (args.length > 1 && typeof args[1] === 'string') {
options.host = args[1];
}
}

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

Expand Down Expand Up @@ -892,13 +905,16 @@ Socket.prototype.connect = function(options, cb) {

if (options === null || typeof options !== 'object') {
// Old API:
// connect(port, [host], [cb])
// connect(path, [cb]);
var args = new Array(arguments.length);
// connect(port[, host][, cb])
// connect(path[, cb]);
const args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
args = normalizeArgs(args);
return Socket.prototype.connect.apply(this, args);
const normalized = normalizeArgs(args);
const normalizedOptions = normalized[0];
const normalizedCb = normalized[1];
return Socket.prototype.connect.call(this,
normalizedOptions, normalizedCb);
}

if (this.destroyed) {
Expand All @@ -923,7 +939,7 @@ Socket.prototype.connect = function(options, cb) {
initSocketHandle(this);
}

if (typeof cb === 'function') {
if (cb !== null) {
this.once('connect', cb);
}

Expand Down Expand Up @@ -1334,57 +1350,73 @@ function listen(self, address, port, addressType, backlog, fd, exclusive) {


Server.prototype.listen = function() {
var args = new Array(arguments.length);
const args = new Array(arguments.length);
for (var i = 0; i < arguments.length; i++)
args[i] = arguments[i];
var [options, cb] = normalizeArgs(args);
// TODO(joyeecheung): use destructuring when V8 is fast enough
const normalized = normalizeArgs(args);
var options = normalized[0];
const cb = normalized[1];

if (typeof cb === 'function') {
var hasCallback = (cb !== null);
if (hasCallback) {
this.once('listening', cb);
}

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(args.length > 1 && args[1]) ||
toNumber(args.length > 2 && args[2]);
const backlogFromArgs =
// (handle, backlog) or (path, backlog) or (port, backlog)
toNumber(args.length > 1 && args[1]) ||
toNumber(args.length > 2 && args[2]); // (port, host, backlog)

options = options._handle || options.handle || options;

// (handle[, backlog][, cb]) where handle is an object with a handle
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 {
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 {
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);
listen(this, null, -1, -1, backlogFromArgs);
return this;
}
// (handle[, backlog][, cb]) where handle is an object with a fd
if (typeof options.fd === 'number' && options.fd >= 0) {
listen(this, null, null, null, backlogFromArgs, options.fd);
return this;
}

// ([port][, host][, backlog][, cb]) where port is omitted,
// that is, listen() or listen(cb),
// or (options[, cb]) where options.port is explicitly set as undefined,
// bind to an arbitrary unused port
if (args.length === 0 || typeof args[0] === 'function' ||
(typeof options.port === 'undefined' && 'port' in options)) {
options.port = 0;
}
// ([port][, host][, backlog][, cb]) where port is specified
// or (options[, cb]) where options.port is specified
// or if options.port is normalized as 0 before
if (typeof options.port === 'number' || typeof options.port === 'string') {
if (!isLegalPort(options.port)) {
throw new RangeError('"port" argument must be >= 0 and < 65536');
}
const backlog = options.backlog || backlogFromArgs;
// start TCP server listening on host:port
if (options.host) {
lookupAndListen(this, options.port | 0, options.host, backlog,
options.exclusive);
} else { // Undefined host, listens on unspecified address
listen(this, null, options.port | 0, 4, // addressType will be ignored
backlog, undefined, options.exclusive);
}
return this;
}

return this;
// (path[, backlog][, cb]) or (options[, cb])
// where path or options.path is a UNIX domain socket or Windows pipe
if (options.path && isPipeName(options.path)) {
const pipeName = this._pipeName = options.path;
const backlog = options.backlog || backlogFromArgs;
listen(this, pipeName, -1, -1, backlog, undefined, options.exclusive);
return this;
}

throw new Error('Invalid listen argument: ' + options);
};

function lookupAndListen(self, port, address, backlog, exclusive) {
Expand Down

0 comments on commit 53f3869

Please sign in to comment.