From b80aedf4c19b6d74d6ad303f74d157dc9ddd17e0 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 14 Jul 2017 11:05:00 -0700 Subject: [PATCH 1/4] net: support passing undefined to listen() For consistency with 4.x and 8.x. This commit also contains a forward port of https://github.com/nodejs/node/pull/14232 to confirm that 4.x and 6.x behave identically with respect to the port argument. PR-URL: https://github.com/nodejs/node/pull/14234 Refs: https://github.com/nodejs/node/issues/14205 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Sam Roberts --- lib/net.js | 2 +- test/parallel/test-net-listen-port-option.js | 35 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/lib/net.js b/lib/net.js index 5e653c61d2106c..83a93d6bd42e87 100644 --- a/lib/net.js +++ b/lib/net.js @@ -1338,7 +1338,7 @@ Server.prototype.listen = function() { self.once('listening', lastArg); } - var port = toNumber(arguments[0]); + var port = typeof arguments[0] === 'undefined' ? 0 : toNumber(arguments[0]); // The third optional argument is the backlog size. // When the ip is omitted it can be the second argument. diff --git a/test/parallel/test-net-listen-port-option.js b/test/parallel/test-net-listen-port-option.js index c4851bd533dfbe..3902a709bd9549 100644 --- a/test/parallel/test-net-listen-port-option.js +++ b/test/parallel/test-net-listen-port-option.js @@ -26,3 +26,38 @@ net.Server().listen({ port: '' + common.PORT }, close); net.Server().listen({ port: port }, common.fail); }, /invalid listen argument/i); }); + +// Repeat the tests, passing port as an argument, which validates somewhat +// differently. + +net.Server().listen(undefined, close); +net.Server().listen('0', close); + +// 'nan', skip, treated as a path, not a port +//'+Infinity', skip, treated as a path, not a port +//'-Infinity' skip, treated as a path, not a port + +// 4.x treats these as 0, but 6.x treats them as invalid numbers. +[ + -1, + 123.456, + 0x10000, + 1 / 0, + -1 / 0, +].forEach(function(port) { + assert.throws(function() { + net.Server().listen(port, common.fail); + }, /"port" argument must be >= 0 and < 65536/i); +}); + +// null is treated as 0 +net.Server().listen(null, close); + +// false/true are converted to 0/1, arguably a bug, but fixing would be +// semver-major. Note that true fails because ports that low can't be listened +// on by unprivileged processes. +net.Server().listen(false, close); + +net.Server().listen(true).on('error', common.mustCall(function(err) { + assert.strictEqual(err.code, 'EACCES'); +})); From 02e0b1ac3c88dd83adafaebee91d83ea2a83d283 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 2 May 2017 14:55:58 +0200 Subject: [PATCH 2/4] src: make root_cert_vector function scoped root_cert_vector currently has file scope and external linkage, but is only used in the NewRootCertsStore function. If this is not required to be externally linked perhaps it can be changed to be static and function scoped instead. PR-URL: https://github.com/nodejs/node/pull/12788 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Shigeki Ohtsu --- src/node_crypto.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index f654dcf60cb424..7eb18ad2020a32 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -127,7 +127,6 @@ const char* const root_certs[] = { std::string extra_root_certs_file; // NOLINT(runtime/string) X509_STORE* root_cert_store; -std::vector root_certs_vector; // Just to generate static methods template class SSLWrap; @@ -710,6 +709,7 @@ static int X509_up_ref(X509* cert) { static X509_STORE* NewRootCertStore() { + static std::vector root_certs_vector; if (root_certs_vector.empty()) { for (size_t i = 0; i < arraysize(root_certs); i++) { BIO* bp = NodeBIO::NewFixed(root_certs[i], strlen(root_certs[i])); From f4691bb2f37661cfd481b46461c1a483b99f63b6 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Wed, 24 May 2017 14:37:29 +0200 Subject: [PATCH 3/4] crypto: remove root_cert_store from node_crypto.h root_cert_store is defined as extern in node_crypto.h but only used in node_crypto.cc. It is then set using SSL_CTX_set_cert_store. The only usages of SSL_CTX_get_cert_store are in node_crypto.cc which would all be accessing the same X509_STORE through the root_cert_store pointer as far as I can tell. Am I missing something here? This commit suggests removing it from the header and making it static in node_crypto.cc. PR-URL: https://github.com/nodejs/node/pull/13194 Reviewed-By: Colin Ihrig Reviewed-By: Ben Noordhuis Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Sam Roberts --- src/node_crypto.cc | 6 +++--- src/node_crypto.h | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 7eb18ad2020a32..aa2dafebc5cf81 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -120,13 +120,13 @@ static X509_NAME *cnnic_ev_name = static Mutex* mutexes; -const char* const root_certs[] = { +static const char* const root_certs[] = { #include "node_root_certs.h" // NOLINT(build/include_order) }; -std::string extra_root_certs_file; // NOLINT(runtime/string) +static std::string extra_root_certs_file; // NOLINT(runtime/string) -X509_STORE* root_cert_store; +static X509_STORE* root_cert_store; // Just to generate static methods template class SSLWrap; diff --git a/src/node_crypto.h b/src/node_crypto.h index 38f49ba5a05063..746c954b26fb43 100644 --- a/src/node_crypto.h +++ b/src/node_crypto.h @@ -63,8 +63,6 @@ enum CheckResult { extern int VerifyCallback(int preverify_ok, X509_STORE_CTX* ctx); -extern X509_STORE* root_cert_store; - extern void UseExtraCaCerts(const std::string& file); // Forward declaration From 0b012a642ab7025b28eedbf11c467315394210e2 Mon Sep 17 00:00:00 2001 From: jeyanthinath Date: Wed, 26 Jul 2017 12:04:04 +0530 Subject: [PATCH 4/4] test: mark test-fs-read-buffer-to-string-fail as flaky PR-URL: https://github.com/nodejs/node/pull/14495 Fixes: https://github.com/nodejs/node/issues/14430 Reviewed-By: Refael Ackermann Reviewed-By: Colin Ihrig --- test/parallel/parallel.status | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/parallel.status b/test/parallel/parallel.status index 97e7803f4d40a5..b2eccf8eb69c75 100644 --- a/test/parallel/parallel.status +++ b/test/parallel/parallel.status @@ -5,6 +5,7 @@ prefix parallel # sample-test : PASS,FLAKY [true] # This section applies to all platforms +test-fs-read-buffer-tostring-fail : PASS,FLAKY [$system==win32]