From 53f386932209ae375c3cf811329483e30868f3ac Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 4 Mar 2017 00:52:12 +0800 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/11667 Reviewed-By: Matteo Collina Reviewed-By: James M Snell --- lib/net.js | 170 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 101 insertions(+), 69 deletions(-) diff --git a/lib/net.js b/lib/net.js index d498efb184e713..64e50e907719d5 100644 --- a/lib/net.js +++ b/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,37 +59,50 @@ 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]; } @@ -98,8 +110,9 @@ function normalizeArgs(args) { 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); } @@ -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) {