FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

net: refactor overloaded argument handling · nodejs/node@b56e851 · GitHub

/ node Public

Commit b56e851

Browse files
joyeecheung
authored and
evanlucas
committed
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>
1 parent a7eba9c commit b56e851

1 file changed

Lines changed: 101 additions & 69 deletions

File tree

‎lib/net.js‎

Lines changed: 101 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ var cluster;
2424
const errnoException = util._errnoException;
2525
const exceptionWithHostPort = util._exceptionWithHostPort;
2626
const isLegalPort = internalNet.isLegalPort;
27-
const assertPort = internalNet.assertPort;
2827

2928
function noop() {}
3029

@@ -60,46 +59,60 @@ exports.createServer = function(options, connectionListener) {
6059
// connect(path, [cb]);
6160
//
6261
exports.connect = exports.createConnection = function() {
63-
var args = new Array(arguments.length);
62+
const args = new Array(arguments.length);
6463
for (var i = 0; i < arguments.length; i++)
6564
args[i] = arguments[i];
66-
args = normalizeArgs(args);
67-
debug('createConnection', args);
68-
var s = new Socket(args[0]);
65+
// TODO(joyeecheung): use destructuring when V8 is fast enough
66+
const normalized = normalizeArgs(args);
67+
const options = normalized[0];
68+
const cb = normalized[1];
69+
debug('createConnection', normalized);
70+
const socket = new Socket(options);
6971

70-
if (args[0].timeout) {
71-
s.setTimeout(args[0].timeout);
72+
if (options.timeout) {
73+
socket.setTimeout(options.timeout);
7274
}
7375

74-
return Socket.prototype.connect.apply(s, args);
76+
return Socket.prototype.connect.call(socket, options, cb);
7577
};
7678

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

80+
// Returns an array [options, cb], where options is an object,
81+
// cb is either a funciton or null.
82+
// Used to normalize arguments of Socket.prototype.connect() and
83+
// Server.prototype.listen(). Possible combinations of paramters:
84+
// (options[...][, cb])
85+
// (path[...][, cb])
86+
// ([port][, host][...][, cb])
87+
// For Socket.prototype.connect(), the [...] part is ignored
88+
// For Server.prototype.listen(), the [...] part is [, backlog]
89+
// but will not be handled here (handled in listen())
90+
function normalizeArgs(args) {
8391
if (args.length === 0) {
84-
return [options];
85-
} else if (args[0] !== null && typeof args[0] === 'object') {
86-
// connect(options, [cb])
87-
options = args[0];
88-
} else if (isPipeName(args[0])) {
89-
// connect(path, [cb]);
90-
options.path = args[0];
92+
return [{}, null];
93+
}
94+
95+
const arg0 = args[0];
96+
var options = {};
97+
if (typeof arg0 === 'object' && arg0 !== null) {
98+
// (options[...][, cb])
99+
options = arg0;
100+
} else if (isPipeName(arg0)) {
101+
// (path[...][, cb])
102+
options.path = arg0;
91103
} else {
92-
// connect(port, [host], [cb])
93-
options.port = args[0];
104+
// ([port][, host][...][, cb])
105+
options.port = arg0;
94106
if (args.length > 1 && typeof args[1] === 'string') {
95107
options.host = args[1];
96108
}
97109
}
98110

99111
var cb = args[args.length - 1];
100112
if (typeof cb !== 'function')
101-
cb = null;
102-
return [options, cb];
113+
return [options, null];
114+
else
115+
return [options, cb];
103116
}
104117
exports._normalizeArgs = normalizeArgs;
105118

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

893906
if (options === null || typeof options !== 'object') {
894907
// Old API:
895-
// connect(port, [host], [cb])
896-
// connect(path, [cb]);
897-
var args = new Array(arguments.length);
908+
// connect(port[, host][, cb])
909+
// connect(path[, cb]);
910+
const args = new Array(arguments.length);
898911
for (var i = 0; i < arguments.length; i++)
899912
args[i] = arguments[i];
900-
args = normalizeArgs(args);
901-
return Socket.prototype.connect.apply(this, args);
913+
const normalized = normalizeArgs(args);
914+
const normalizedOptions = normalized[0];
915+
const normalizedCb = normalized[1];
916+
return Socket.prototype.connect.call(this,
917+
normalizedOptions, normalizedCb);
902918
}
903919

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

926-
if (typeof cb === 'function') {
942+
if (cb !== null) {
927943
this.once('connect', cb);
928944
}
929945

@@ -1333,57 +1349,73 @@ function listen(self, address, port, addressType, backlog, fd, exclusive) {
13331349

13341350

13351351
Server.prototype.listen = function() {
1336-
var args = new Array(arguments.length);
1352+
const args = new Array(arguments.length);
13371353
for (var i = 0; i < arguments.length; i++)
13381354
args[i] = arguments[i];
1339-
var [options, cb] = normalizeArgs(args);
1355+
// TODO(joyeecheung): use destructuring when V8 is fast enough
1356+
const normalized = normalizeArgs(args);
1357+
var options = normalized[0];
1358+
const cb = normalized[1];
13401359

1341-
if (typeof cb === 'function') {
1360+
var hasCallback = (cb !== null);
1361+
if (hasCallback) {
13421362
this.once('listening', cb);
13431363
}
1344-
1345-
if (args.length === 0 || typeof args[0] === 'function') {
1346-
// Bind to a random port.
1347-
options.port = 0;
1348-
}
1349-
1350-
// The third optional argument is the backlog size.
1351-
// When the ip is omitted it can be the second argument.
1352-
var backlog = toNumber(args.length > 1 && args[1]) ||
1353-
toNumber(args.length > 2 && args[2]);
1364+
const backlogFromArgs =
1365+
// (handle, backlog) or (path, backlog) or (port, backlog)
1366+
toNumber(args.length > 1 && args[1]) ||
1367+
toNumber(args.length > 2 && args[2]); // (port, host, backlog)
13541368

13551369
options = options._handle || options.handle || options;
1356-
1370+
// (handle[, backlog][, cb]) where handle is an object with a handle
13571371
if (options instanceof TCP) {
13581372
this._handle = options;
1359-
listen(this, null, -1, -1, backlog);
1360-
} else if (typeof options.fd === 'number' && options.fd >= 0) {
1361-
listen(this, null, null, null, backlog, options.fd);
1362-
} else {
1363-
backlog = options.backlog || backlog;
1364-
1365-
if (typeof options.port === 'number' || typeof options.port === 'string' ||
1366-
(typeof options.port === 'undefined' && 'port' in options)) {
1367-
// Undefined is interpreted as zero (random port) for consistency
1368-
// with net.connect().
1369-
assertPort(options.port);
1370-
if (options.host) {
1371-
lookupAndListen(this, options.port | 0, options.host, backlog,
1372-
options.exclusive);
1373-
} else {
1374-
listen(this, null, options.port | 0, 4, backlog, undefined,
1375-
options.exclusive);
1376-
}
1377-
} else if (options.path && isPipeName(options.path)) {
1378-
// UNIX socket or Windows pipe.
1379-
const pipeName = this._pipeName = options.path;
1380-
listen(this, pipeName, -1, -1, backlog, undefined, options.exclusive);
1381-
} else {
1382-
throw new Error('Invalid listen argument: ' + options);
1373+
listen(this, null, -1, -1, backlogFromArgs);
1374+
return this;
1375+
}
1376+
// (handle[, backlog][, cb]) where handle is an object with a fd
1377+
if (typeof options.fd === 'number' && options.fd >= 0) {
1378+
listen(this, null, null, null, backlogFromArgs, options.fd);
1379+
return this;
1380+
}
1381+
1382+
// ([port][, host][, backlog][, cb]) where port is omitted,
1383+
// that is, listen() or listen(cb),
1384+
// or (options[, cb]) where options.port is explicitly set as undefined,
1385+
// bind to an arbitrary unused port
1386+
if (args.length === 0 || typeof args[0] === 'function' ||
1387+
(typeof options.port === 'undefined' && 'port' in options)) {
1388+
options.port = 0;
1389+
}
1390+
// ([port][, host][, backlog][, cb]) where port is specified
1391+
// or (options[, cb]) where options.port is specified
1392+
// or if options.port is normalized as 0 before
1393+
if (typeof options.port === 'number' || typeof options.port === 'string') {
1394+
if (!isLegalPort(options.port)) {
1395+
throw new RangeError('"port" argument must be >= 0 and < 65536');
1396+
}
1397+
const backlog = options.backlog || backlogFromArgs;
1398+
// start TCP server listening on host:port
1399+
if (options.host) {
1400+
lookupAndListen(this, options.port | 0, options.host, backlog,
1401+
options.exclusive);
1402+
} else { // Undefined host, listens on unspecified address
1403+
listen(this, null, options.port | 0, 4, // addressType will be ignored
1404+
backlog, undefined, options.exclusive);
13831405
}
1406+
return this;
13841407
}
13851408

1386-
return this;
1409+
// (path[, backlog][, cb]) or (options[, cb])
1410+
// where path or options.path is a UNIX domain socket or Windows pipe
1411+
if (options.path && isPipeName(options.path)) {
1412+
const pipeName = this._pipeName = options.path;
1413+
const backlog = options.backlog || backlogFromArgs;
1414+
listen(this, pipeName, -1, -1, backlog, undefined, options.exclusive);
1415+
return this;
1416+
}
1417+
1418+
throw new Error('Invalid listen argument: ' + options);
13871419
};
13881420

13891421
function lookupAndListen(self, port, address, backlog, exclusive) {

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL