From 6a90ec51b8a761a6c6d3963921b1a996a1673708 Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Thu, 2 Aug 2018 13:28:14 +0300 Subject: [PATCH 1/7] net: throw error if port does not exist in options Throw error ERR_PROPERTY_NOT_IN_OBJECT if the port property does not exist in the options object argument when calling server.listen(). Refs: https://github.com/nodejs/node/issues/16712 --- doc/api/errors.md | 5 +++++ lib/internal/errors.js | 1 + lib/net.js | 5 +++++ .../test-net-server-listen-options.js | 22 ++++++++++++++----- 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index e4a709d45a8031..f2a69ae98ea6ec 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1455,6 +1455,11 @@ A Node.js API was called in an unsupported manner, such as A given value is out of the accepted range. + +### ERR_PROPERTY_NOT_IN_OBJECT + +A property does not exist in an object. + ### ERR_REQUIRE_ESM diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 89b51a4cecc9d8..3db0c62ede9bba 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -773,6 +773,7 @@ E('ERR_OUT_OF_RANGE', msg += ` Received ${value}`; return msg; }, RangeError); +E('ERR_PROPERTY_NOT_IN_OBJECT', '%s does not exist in %s', TypeError); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); diff --git a/lib/net.js b/lib/net.js index 428675173f3e3e..ab46c8e05f664b 100644 --- a/lib/net.js +++ b/lib/net.js @@ -71,6 +71,7 @@ const { ERR_INVALID_FD_TYPE, ERR_INVALID_IP_ADDRESS, ERR_INVALID_OPT_VALUE, + ERR_PROPERTY_NOT_IN_OBJECT, ERR_SERVER_ALREADY_LISTEN, ERR_SERVER_NOT_RUNNING, ERR_SOCKET_BAD_PORT, @@ -1496,6 +1497,10 @@ Server.prototype.listen = function(...args) { return this; } + if (typeof options === 'object' && !('port' in options)) { + throw new ERR_PROPERTY_NOT_IN_OBJECT('port', 'options'); + } + throw new ERR_INVALID_OPT_VALUE('options', util.inspect(options)); }; diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js index 4f7a6bd28f561a..181e81094ce2a0 100644 --- a/test/parallel/test-net-server-listen-options.js +++ b/test/parallel/test-net-server-listen-options.js @@ -57,12 +57,22 @@ const listenOnPort = [ const block = () => { net.createServer().listen(options, common.mustNotCall()); }; - common.expectsError(block, - { - code: 'ERR_INVALID_OPT_VALUE', - type: TypeError, - message: /^The value "{.*}" is invalid for option "options"$/ - }); + + if (typeof options === 'object' && !('port' in options)) { + common.expectsError(block, + { + code: 'ERR_PROPERTY_NOT_IN_OBJECT', + type: TypeError, + message: 'port does not exist in options', + }); + } else { + common.expectsError(block, + { + code: 'ERR_INVALID_OPT_VALUE', + type: TypeError, + message: /^The value "{.*}" is invalid for option "options"$/ + }); + } } shouldFailToListen(false, { port: false }); From 0c0fbe4b2d8f6b6a97f65a1c8f61a480ad847813 Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Sat, 4 Aug 2018 17:59:31 +0300 Subject: [PATCH 2/7] errors: add an additional error message argument to ERR_INVALID_OPT_VALUE --- lib/internal/errors.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 3db0c62ede9bba..a93e1615302c6c 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -672,8 +672,14 @@ E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s', TypeError); E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent', TypeError); E('ERR_INVALID_HTTP_TOKEN', '%s must be a valid HTTP token ["%s"]', TypeError); E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s', TypeError); -E('ERR_INVALID_OPT_VALUE', (name, value) => - `The value "${String(value)}" is invalid for option "${name}"`, +E('ERR_INVALID_OPT_VALUE', (name, value, additionalMessage = undefined) => { + let message = `The value "${String(value)}" is invalid for option "${name}"`; + if (typeof additionalMessage === 'string' && additionalMessage.length > 0) { + message += `. ${additionalMessage}`; + } + + return message; +}, TypeError, RangeError); E('ERR_INVALID_OPT_VALUE_ENCODING', From 54fc80f90b67ceefc1a8044f0f6b2056436f7e3d Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Sat, 4 Aug 2018 18:01:34 +0300 Subject: [PATCH 3/7] net: use ERR_INVALID_OPT_VALUE instead of ERR_PROPERTY_NOT_IN_OBJECT --- lib/net.js | 11 ++++++---- .../test-net-server-listen-options.js | 21 ++++++------------- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/lib/net.js b/lib/net.js index ab46c8e05f664b..19ad6e70bddd46 100644 --- a/lib/net.js +++ b/lib/net.js @@ -71,7 +71,6 @@ const { ERR_INVALID_FD_TYPE, ERR_INVALID_IP_ADDRESS, ERR_INVALID_OPT_VALUE, - ERR_PROPERTY_NOT_IN_OBJECT, ERR_SERVER_ALREADY_LISTEN, ERR_SERVER_NOT_RUNNING, ERR_SOCKET_BAD_PORT, @@ -1497,11 +1496,15 @@ Server.prototype.listen = function(...args) { return this; } - if (typeof options === 'object' && !('port' in options)) { - throw new ERR_PROPERTY_NOT_IN_OBJECT('port', 'options'); + let additionalMessage; + if (typeof options === 'object' && + !(('port' in options) || ('path' in options))) { + + additionalMessage = '"port" or "path" must be specified in options.'; } - throw new ERR_INVALID_OPT_VALUE('options', util.inspect(options)); + throw new ERR_INVALID_OPT_VALUE('options', util.inspect(options), + additionalMessage); }; function lookupAndListen(self, port, address, backlog, exclusive) { diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js index 181e81094ce2a0..6542d0567e0085 100644 --- a/test/parallel/test-net-server-listen-options.js +++ b/test/parallel/test-net-server-listen-options.js @@ -58,21 +58,12 @@ const listenOnPort = [ net.createServer().listen(options, common.mustNotCall()); }; - if (typeof options === 'object' && !('port' in options)) { - common.expectsError(block, - { - code: 'ERR_PROPERTY_NOT_IN_OBJECT', - type: TypeError, - message: 'port does not exist in options', - }); - } else { - common.expectsError(block, - { - code: 'ERR_INVALID_OPT_VALUE', - type: TypeError, - message: /^The value "{.*}" is invalid for option "options"$/ - }); - } + common.expectsError(block, + { + code: 'ERR_INVALID_OPT_VALUE', + type: TypeError, + message: /^The value "{.*}" is invalid for option "options"(?:\. .+)?$/, + }); } shouldFailToListen(false, { port: false }); From daae72ba783d03db181d08c38a385de8478f4bed Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Sat, 4 Aug 2018 18:02:12 +0300 Subject: [PATCH 4/7] errors: remove ERR_PROPERTY_NOT_IN_OBJECT --- doc/api/errors.md | 5 ----- lib/internal/errors.js | 1 - 2 files changed, 6 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index f2a69ae98ea6ec..e4a709d45a8031 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1455,11 +1455,6 @@ A Node.js API was called in an unsupported manner, such as A given value is out of the accepted range. - -### ERR_PROPERTY_NOT_IN_OBJECT - -A property does not exist in an object. - ### ERR_REQUIRE_ESM diff --git a/lib/internal/errors.js b/lib/internal/errors.js index a93e1615302c6c..c2f9bea2c3bd5e 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -779,7 +779,6 @@ E('ERR_OUT_OF_RANGE', msg += ` Received ${value}`; return msg; }, RangeError); -E('ERR_PROPERTY_NOT_IN_OBJECT', '%s does not exist in %s', TypeError); E('ERR_REQUIRE_ESM', 'Must use import to load ES Module: %s', Error); E('ERR_SCRIPT_EXECUTION_INTERRUPTED', 'Script execution was interrupted by `SIGINT`', Error); From 20b525e9edf5c6d4532a83b60616da8bab684f36 Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Sun, 12 Aug 2018 23:55:37 +0300 Subject: [PATCH 5/7] test: add tests for the additional error message --- test/parallel/test-net-server-listen-options.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js index 6542d0567e0085..f3ea47239d12f1 100644 --- a/test/parallel/test-net-server-listen-options.js +++ b/test/parallel/test-net-server-listen-options.js @@ -74,6 +74,11 @@ const listenOnPort = [ shouldFailToListen({ fd: -1 }); // Invalid path in listen(options) shouldFailToListen({ path: -1 }); - // Host without port + + // Neither port or path are specified in options + shouldFailToListen({}); shouldFailToListen({ host: 'localhost' }); + shouldFailToListen({ host: 'localhost:3000' }); + shouldFailToListen({ host: { port: 3000 } }); + shouldFailToListen({ exclusive: true }); } From 7be75d8d0fe029a21cafafbbbccf840f503339eb Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Wed, 15 Aug 2018 21:22:45 +0300 Subject: [PATCH 6/7] errors: revert adding an additional error message argument to ERR_INVALID_OPT_VALUE --- lib/internal/errors.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index c2f9bea2c3bd5e..89b51a4cecc9d8 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -672,14 +672,8 @@ E('ERR_INVALID_FILE_URL_PATH', 'File URL path %s', TypeError); E('ERR_INVALID_HANDLE_TYPE', 'This handle type cannot be sent', TypeError); E('ERR_INVALID_HTTP_TOKEN', '%s must be a valid HTTP token ["%s"]', TypeError); E('ERR_INVALID_IP_ADDRESS', 'Invalid IP address: %s', TypeError); -E('ERR_INVALID_OPT_VALUE', (name, value, additionalMessage = undefined) => { - let message = `The value "${String(value)}" is invalid for option "${name}"`; - if (typeof additionalMessage === 'string' && additionalMessage.length > 0) { - message += `. ${additionalMessage}`; - } - - return message; -}, +E('ERR_INVALID_OPT_VALUE', (name, value) => + `The value "${String(value)}" is invalid for option "${name}"`, TypeError, RangeError); E('ERR_INVALID_OPT_VALUE_ENCODING', From 630d987672297b74d97ec523c0c59f0523308522 Mon Sep 17 00:00:00 2001 From: Yaniv Friedensohn Date: Wed, 15 Aug 2018 21:23:03 +0300 Subject: [PATCH 7/7] net: use ERR_INVALID_ARG_VALUE instead of ERR_INVALID_OPT_VALUE --- lib/net.js | 12 +++++----- .../test-net-server-listen-options.js | 22 ++++++++++++++----- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/lib/net.js b/lib/net.js index 19ad6e70bddd46..5ca4e5d9d777b5 100644 --- a/lib/net.js +++ b/lib/net.js @@ -68,6 +68,7 @@ const errors = require('internal/errors'); const { ERR_INVALID_ADDRESS_FAMILY, ERR_INVALID_ARG_TYPE, + ERR_INVALID_ARG_VALUE, ERR_INVALID_FD_TYPE, ERR_INVALID_IP_ADDRESS, ERR_INVALID_OPT_VALUE, @@ -1496,15 +1497,12 @@ Server.prototype.listen = function(...args) { return this; } - let additionalMessage; - if (typeof options === 'object' && - !(('port' in options) || ('path' in options))) { - - additionalMessage = '"port" or "path" must be specified in options.'; + if (!(('port' in options) || ('path' in options))) { + throw new ERR_INVALID_ARG_VALUE('options', options, + 'must have the property "port" or "path"'); } - throw new ERR_INVALID_OPT_VALUE('options', util.inspect(options), - additionalMessage); + throw new ERR_INVALID_OPT_VALUE('options', util.inspect(options)); }; function lookupAndListen(self, port, address, backlog, exclusive) { diff --git a/test/parallel/test-net-server-listen-options.js b/test/parallel/test-net-server-listen-options.js index f3ea47239d12f1..41cc07d40d4d6e 100644 --- a/test/parallel/test-net-server-listen-options.js +++ b/test/parallel/test-net-server-listen-options.js @@ -58,12 +58,22 @@ const listenOnPort = [ net.createServer().listen(options, common.mustNotCall()); }; - common.expectsError(block, - { - code: 'ERR_INVALID_OPT_VALUE', - type: TypeError, - message: /^The value "{.*}" is invalid for option "options"(?:\. .+)?$/, - }); + if (typeof options === 'object' && + !(('port' in options) || ('path' in options))) { + common.expectsError(block, + { + code: 'ERR_INVALID_ARG_VALUE', + type: TypeError, + message: /^The argument 'options' must have the property "port" or "path"\. Received .+$/, + }); + } else { + common.expectsError(block, + { + code: 'ERR_INVALID_OPT_VALUE', + type: TypeError, + message: /^The value "{.*}" is invalid for option "options"(?:\. .+)?$/, + }); + } } shouldFailToListen(false, { port: false });