From 1403d28e7ded280e7582daa6e999164588d2234e Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 1 Sep 2017 16:14:56 +0200 Subject: [PATCH] tls: re-allow falsey option values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5723c4c5f06f138 was an unintentional breaking change in that it changed the behaviour of `tls.createSecureContext()` to throw on false-y input rather than ignoring it. This breaks real-world applications like `npm`. This restores the previous behaviour. PR-URL: https://github.com/nodejs/node/pull/15131 Ref: https://github.com/nodejs/node/pull/15053 Reviewed-By: Refael Ackermann Reviewed-By: Benjamin Gruenbaum Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater Reviewed-By: Luigi Pinca Reviewed-By: Alexey Orlenko Reviewed-By: MichaƫZasso Reviewed-By: Michael Dawson Reviewed-By: Timothy Gu Reviewed-By: Brian White --- lib/_tls_common.js | 6 +- .../test-tls-options-boolean-check.js | 64 ++++++++++++------- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/lib/_tls_common.js b/lib/_tls_common.js index ed0bd4c23fe240..92ba45d57e5526 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -80,7 +80,7 @@ exports.createSecureContext = function createSecureContext(options, context) { // NOTE: It's important to add CA before the cert to be able to load // cert's issuer in C++ code. var ca = options.ca; - if (ca !== undefined) { + if (ca) { if (Array.isArray(ca)) { for (i = 0; i < ca.length; ++i) { val = ca[i]; @@ -96,7 +96,7 @@ exports.createSecureContext = function createSecureContext(options, context) { } var cert = options.cert; - if (cert !== undefined) { + if (cert) { if (Array.isArray(cert)) { for (i = 0; i < cert.length; ++i) { val = cert[i]; @@ -115,7 +115,7 @@ exports.createSecureContext = function createSecureContext(options, context) { // which leads to the crash later on. var key = options.key; var passphrase = options.passphrase; - if (key !== undefined) { + if (key) { if (Array.isArray(key)) { for (i = 0; i < key.length; ++i) { val = key[i]; diff --git a/test/parallel/test-tls-options-boolean-check.js b/test/parallel/test-tls-options-boolean-check.js index 4cc0aaf3771bf0..e866f5a0692d54 100644 --- a/test/parallel/test-tls-options-boolean-check.js +++ b/test/parallel/test-tls-options-boolean-check.js @@ -64,12 +64,9 @@ const invalidCertRE = /^The "cert" argument must be one of type string, Buffer, [false, [certStr, certStr2]], [[{ pem: keyBuff }], false], [[{ pem: keyBuff }, { pem: keyBuff }], false] -].map((params) => { +].map(([key, cert]) => { assert.doesNotThrow(() => { - tls.createServer({ - key: params[0], - cert: params[1] - }); + tls.createServer({ key, cert }); }); }); @@ -100,16 +97,13 @@ const invalidCertRE = /^The "cert" argument must be one of type string, Buffer, [[keyStr, keyStr2], [true, false], invalidCertRE], [[keyStr, keyStr2], true, invalidCertRE], [true, [certBuff, certBuff2], invalidKeyRE] -].map((params) => { +].map(([key, cert, message]) => { assert.throws(() => { - tls.createServer({ - key: params[0], - cert: params[1] - }); + tls.createServer({ key, cert }); }, common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, - message: params[2] + message })); }); @@ -123,13 +117,9 @@ const invalidCertRE = /^The "cert" argument must be one of type string, Buffer, [keyBuff, certBuff, caArrBuff], [keyBuff, certBuff, caArrDataView], [keyBuff, certBuff, false], -].map((params) => { +].map(([key, cert, ca]) => { assert.doesNotThrow(() => { - tls.createServer({ - key: params[0], - cert: params[1], - ca: params[2] - }); + tls.createServer({ key, cert, ca }); }); }); @@ -141,16 +131,44 @@ const invalidCertRE = /^The "cert" argument must be one of type string, Buffer, [keyBuff, certBuff, 1], [keyBuff, certBuff, true], [keyBuff, certBuff, [caCert, true]] -].map((params) => { +].map(([key, cert, ca]) => { assert.throws(() => { - tls.createServer({ - key: params[0], - cert: params[1], - ca: params[2] - }); + tls.createServer({ key, cert, ca }); }, common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError, message: /^The "ca" argument must be one of type string, Buffer, TypedArray, or DataView$/ })); }); + +// Checks to ensure tls.createServer throws an error for CA assignment +// Format ['key', 'cert', 'ca'] +[ + [keyBuff, certBuff, true], + [keyBuff, certBuff, {}], + [keyBuff, certBuff, 1], + [keyBuff, certBuff, true], + [keyBuff, certBuff, [caCert, true]] +].map(([key, cert, ca]) => { + assert.throws(() => { + tls.createServer({ key, cert, ca }); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "ca" argument must be one of type string, Buffer, TypedArray, or DataView$/ + })); +}); + +// Checks to ensure tls.createSecureContext works with false-y input +// Format ['key', 'cert', 'ca'] +[ + [null, null, null], + [false, false, false], + [undefined, undefined, undefined], + ['', '', ''], + [0, 0, 0] +].map(([key, cert, ca]) => { + assert.doesNotThrow(() => { + tls.createSecureContext({ key, cert, ca }); + }); +});