net: refactor overloaded argument handling

* 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>

Author: joyeecheung

lib/net.js

@@ -24,7 +24,6 @@ var cluster;
 const errnoException = util._errnoException;
 const exceptionWithHostPort = util._exceptionWithHostPort;
 const isLegalPort = internalNet.isLegalPort;
-const assertPort = internalNet.assertPort;
 
 function noop() {}
 
@@ -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;
 
@@ -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) {
@@ -923,7 +939,7 @@ Socket.prototype.connect = function(options, cb) {
  initSocketHandle(this);
  }
 
- if (typeof cb === 'function') {
+ if (cb !== null) {
  this.once('connect', cb);
  }
 
@@ -1333,57 +1349,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) {