From 897b1d2e5ec89247432d5422a0df1dd3a193376c Mon Sep 17 00:00:00 2001 From: James M Snell Date: Tue, 18 Feb 2020 09:14:50 -0800 Subject: [PATCH] lib: move isLegalPort to validators, refactor isLegalPort was used multiple places in the same way -- to validate the port and throw if necessary. Moved into internal/validators. PR-URL: https://github.com/nodejs/node/pull/31851 Reviewed-By: Richard Lau --- lib/dgram.js | 24 +++--------------- lib/dns.js | 11 ++++---- lib/internal/cluster/master.js | 7 ++---- lib/internal/dns/promises.js | 12 ++++----- lib/internal/errors.js | 2 +- lib/internal/net.js | 10 -------- lib/internal/validators.js | 25 +++++++++++++++---- lib/net.js | 16 ++++++------ .../test-internal-validators-validateport.js | 23 +++++++++++++++++ test/parallel/test-net-internal.js | 20 --------------- 10 files changed, 69 insertions(+), 81 deletions(-) create mode 100644 test/parallel/test-internal-validators-validateport.js delete mode 100644 test/parallel/test-net-internal.js diff --git a/lib/dgram.js b/lib/dgram.js index dba1bc024272eb..f032d3025d9efc 100644 --- a/lib/dgram.js +++ b/lib/dgram.js @@ -35,15 +35,11 @@ const { newHandle, } = require('internal/dgram'); const { guessHandleType } = internalBinding('util'); -const { - isLegalPort, -} = require('internal/net'); const { ERR_INVALID_ARG_TYPE, ERR_MISSING_ARGS, ERR_SOCKET_ALREADY_BOUND, ERR_SOCKET_BAD_BUFFER_SIZE, - ERR_SOCKET_BAD_PORT, ERR_SOCKET_BUFFER_SIZE, ERR_SOCKET_CANNOT_SEND, ERR_SOCKET_DGRAM_IS_CONNECTED, @@ -54,7 +50,8 @@ const { const { isInt32, validateString, - validateNumber + validateNumber, + validatePort, } = require('internal/validators'); const { Buffer } = require('buffer'); const { deprecate } = require('internal/util'); @@ -352,21 +349,8 @@ Socket.prototype.bind = function(port_, address_ /* , callback */) { return this; }; - -function validatePort(port) { - const legal = isLegalPort(port); - if (legal) - port = port | 0; - - if (!legal || port === 0) - throw new ERR_SOCKET_BAD_PORT(port); - - return port; -} - - Socket.prototype.connect = function(port, address, callback) { - port = validatePort(port); + port = validatePort(port, 'Port', { allowZero: false }); if (typeof address === 'function') { callback = address; address = ''; @@ -612,7 +596,7 @@ Socket.prototype.send = function(buffer, } if (!connected) - port = validatePort(port); + port = validatePort(port, 'Port', { allowZero: false }); // Normalize callback so it's either a function or undefined but not anything // else. diff --git a/lib/dns.js b/lib/dns.js index 8a6c7456babbd0..e33dd2620e1e3f 100644 --- a/lib/dns.js +++ b/lib/dns.js @@ -29,7 +29,7 @@ const { const cares = internalBinding('cares_wrap'); const { toASCII } = require('internal/idna'); -const { isIP, isLegalPort } = require('internal/net'); +const { isIP } = require('internal/net'); const { customPromisifyArgs } = require('internal/util'); const errors = require('internal/errors'); const { @@ -45,9 +45,11 @@ const { ERR_INVALID_CALLBACK, ERR_INVALID_OPT_VALUE, ERR_MISSING_ARGS, - ERR_SOCKET_BAD_PORT } = errors.codes; -const { validateString } = require('internal/validators'); +const { + validatePort, + validateString, +} = require('internal/validators'); const { GetAddrInfoReqWrap, @@ -175,8 +177,7 @@ function lookupService(address, port, callback) { if (isIP(address) === 0) throw new ERR_INVALID_OPT_VALUE('address', address); - if (!isLegalPort(port)) - throw new ERR_SOCKET_BAD_PORT(port); + validatePort(port); if (typeof callback !== 'function') throw new ERR_INVALID_CALLBACK(callback); diff --git a/lib/internal/cluster/master.js b/lib/internal/cluster/master.js index 9bdb0181d3db93..46c77900f42d5e 100644 --- a/lib/internal/cluster/master.js +++ b/lib/internal/cluster/master.js @@ -14,13 +14,12 @@ const RoundRobinHandle = require('internal/cluster/round_robin_handle'); const SharedHandle = require('internal/cluster/shared_handle'); const Worker = require('internal/cluster/worker'); const { internal, sendHelper } = require('internal/cluster/utils'); -const { ERR_SOCKET_BAD_PORT } = require('internal/errors').codes; const cluster = new EventEmitter(); const intercom = new EventEmitter(); const SCHED_NONE = 1; const SCHED_RR = 2; -const { isLegalPort } = require('internal/net'); const [ minPort, maxPort ] = [ 1024, 65535 ]; +const { validatePort } = require('internal/validators'); module.exports = cluster; @@ -118,9 +117,7 @@ function createWorkerProcess(id, env) { else inspectPort = cluster.settings.inspectPort; - if (!isLegalPort(inspectPort)) { - throw new ERR_SOCKET_BAD_PORT(inspectPort); - } + validatePort(inspectPort); } else { inspectPort = process.debugPort + debugPortOffset; if (inspectPort > maxPort) diff --git a/lib/internal/dns/promises.js b/lib/internal/dns/promises.js index ae007fd31934e2..6ade8854964c2e 100644 --- a/lib/internal/dns/promises.js +++ b/lib/internal/dns/promises.js @@ -14,7 +14,7 @@ const { } = require('internal/dns/utils'); const { codes, dnsException } = require('internal/errors'); const { toASCII } = require('internal/idna'); -const { isIP, isLegalPort } = require('internal/net'); +const { isIP } = require('internal/net'); const { getaddrinfo, getnameinfo, @@ -27,10 +27,11 @@ const { ERR_INVALID_ARG_TYPE, ERR_INVALID_OPT_VALUE, ERR_MISSING_ARGS, - ERR_SOCKET_BAD_PORT } = codes; -const { validateString } = require('internal/validators'); - +const { + validatePort, + validateString +} = require('internal/validators'); function onlookup(err, addresses) { if (err) { @@ -162,8 +163,7 @@ function lookupService(address, port) { if (isIP(address) === 0) throw new ERR_INVALID_OPT_VALUE('address', address); - if (!isLegalPort(port)) - throw new ERR_SOCKET_BAD_PORT(port); + validatePort(port); return createLookupServicePromise(address, +port); } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index cfab59f2b721d3..460cdf65c9b48f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1278,7 +1278,7 @@ E('ERR_SOCKET_ALREADY_BOUND', 'Socket is already bound', Error); E('ERR_SOCKET_BAD_BUFFER_SIZE', 'Buffer size must be a positive integer', TypeError); E('ERR_SOCKET_BAD_PORT', - 'Port should be >= 0 and < 65536. Received %s.', RangeError); + '%s should be >= 0 and < 65536. Received %s.', RangeError); E('ERR_SOCKET_BAD_TYPE', 'Bad socket type specified. Valid types are: udp4, udp6', TypeError); E('ERR_SOCKET_BUFFER_SIZE', diff --git a/lib/internal/net.js b/lib/internal/net.js index 728c6f587a82f7..4a9a156aeabf66 100644 --- a/lib/internal/net.js +++ b/lib/internal/net.js @@ -41,15 +41,6 @@ function isIP(s) { return 0; } -// Check that the port number is not NaN when coerced to a number, -// is an integer and that it falls within the legal range of port numbers. -function isLegalPort(port) { - if ((typeof port !== 'number' && typeof port !== 'string') || - (typeof port === 'string' && port.trim().length === 0)) - return false; - return +port === (+port >>> 0) && port <= 0xFFFF; -} - function makeSyncWrite(fd) { return function(chunk, enc, cb) { if (enc !== 'buffer') @@ -72,7 +63,6 @@ module.exports = { isIP, isIPv4, isIPv6, - isLegalPort, makeSyncWrite, normalizedArgsSymbol: Symbol('normalizedArgs') }; diff --git a/lib/internal/validators.js b/lib/internal/validators.js index 1d656c3d65233e..04c05c2851548a 100644 --- a/lib/internal/validators.js +++ b/lib/internal/validators.js @@ -10,6 +10,7 @@ const { const { hideStackFrames, codes: { + ERR_SOCKET_BAD_PORT, ERR_INVALID_ARG_TYPE, ERR_INVALID_ARG_VALUE, ERR_OUT_OF_RANGE, @@ -180,6 +181,19 @@ function validateEncoding(data, encoding) { } } +// Check that the port number is not NaN when coerced to a number, +// is an integer and that it falls within the legal range of port numbers. +function validatePort(port, name = 'Port', { allowZero = true } = {}) { + if ((typeof port !== 'number' && typeof port !== 'string') || + (typeof port === 'string' && port.trim().length === 0) || + +port !== (+port >>> 0) || + port > 0xFFFF || + (port === 0 && !allowZero)) { + throw new ERR_SOCKET_BAD_PORT(name, port); + } + return port | 0; +} + module.exports = { isInt32, isUint32, @@ -188,11 +202,12 @@ module.exports = { validateBoolean, validateBuffer, validateEncoding, - validateObject, - validateInteger, validateInt32, - validateUint32, - validateString, + validateInteger, validateNumber, - validateSignalName + validateObject, + validatePort, + validateSignalName, + validateString, + validateUint32, }; diff --git a/lib/net.js b/lib/net.js index 21d17702b53798..67e38497e6f885 100644 --- a/lib/net.js +++ b/lib/net.js @@ -41,7 +41,6 @@ const { isIP, isIPv4, isIPv6, - isLegalPort, normalizedArgsSymbol, makeSyncWrite } = require('internal/net'); @@ -92,7 +91,6 @@ const { ERR_INVALID_OPT_VALUE, ERR_SERVER_ALREADY_LISTEN, ERR_SERVER_NOT_RUNNING, - ERR_SOCKET_BAD_PORT, ERR_SOCKET_CLOSED }, errnoException, @@ -100,7 +98,11 @@ const { uvExceptionWithHostPort } = require('internal/errors'); const { isUint8Array } = require('internal/util/types'); -const { validateInt32, validateString } = require('internal/validators'); +const { + validateInt32, + validatePort, + validateString +} = require('internal/validators'); const kLastWriteQueueSize = Symbol('lastWriteQueueSize'); const { DTRACE_NET_SERVER_CONNECTION, @@ -997,9 +999,7 @@ function lookupAndConnect(self, options) { throw new ERR_INVALID_ARG_TYPE('options.port', ['number', 'string'], port); } - if (!isLegalPort(port)) { - throw new ERR_SOCKET_BAD_PORT(port); - } + validatePort(port); } port |= 0; @@ -1436,9 +1436,7 @@ Server.prototype.listen = function(...args) { // or if options.port is normalized as 0 before let backlog; if (typeof options.port === 'number' || typeof options.port === 'string') { - if (!isLegalPort(options.port)) { - throw new ERR_SOCKET_BAD_PORT(options.port); - } + validatePort(options.port, 'options.port'); backlog = options.backlog || backlogFromArgs; // start TCP server listening on host:port if (options.host) { diff --git a/test/parallel/test-internal-validators-validateport.js b/test/parallel/test-internal-validators-validateport.js new file mode 100644 index 00000000000000..ea9c3a7b58b692 --- /dev/null +++ b/test/parallel/test-internal-validators-validateport.js @@ -0,0 +1,23 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const assert = require('assert'); +const { validatePort } = require('internal/validators'); + +for (let n = 0; n <= 0xFFFF; n++) { + validatePort(n); + validatePort(`${n}`); + validatePort(`0x${n.toString(16)}`); + validatePort(`0o${n.toString(8)}`); + validatePort(`0b${n.toString(2)}`); +} + +[ + -1, 'a', {}, [], false, true, + 0xFFFF + 1, Infinity, -Infinity, NaN, + undefined, null, '', ' ', 1.1, '0x', + '-0x1', '-0o1', '-0b1', '0o', '0b' +].forEach((i) => assert.throws(() => validatePort(i), { + code: 'ERR_SOCKET_BAD_PORT' +})); diff --git a/test/parallel/test-net-internal.js b/test/parallel/test-net-internal.js deleted file mode 100644 index 309b56d4d9aafc..00000000000000 --- a/test/parallel/test-net-internal.js +++ /dev/null @@ -1,20 +0,0 @@ -'use strict'; - -// Flags: --expose-internals - -require('../common'); -const assert = require('assert'); -const isLegalPort = require('internal/net').isLegalPort; - -for (let n = 0; n <= 0xFFFF; n++) { - assert(isLegalPort(n)); - assert(isLegalPort(String(n))); - assert(`0x${n.toString(16)}`); - assert(`0o${n.toString(8)}`); - assert(`0b${n.toString(2)}`); -} - -const bad = [-1, 'a', {}, [], false, true, 0xFFFF + 1, Infinity, - -Infinity, NaN, undefined, null, '', ' ', 1.1, '0x', - '-0x1', '-0o1', '-0b1', '0o', '0b']; -bad.forEach((i) => assert(!isLegalPort(i)));