Skip to content

Commit

Permalink
tls: re-allow falsey option values
Browse files Browse the repository at this point in the history
5723c4c 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: nodejs#15131
Ref: nodejs#15053
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
Reviewed-By: MichaëZasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
  • Loading branch information
addaleax authored and mhdawson committed Sep 5, 2017
1 parent dc7f03c commit 1403d28
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 26 deletions.
6 changes: 3 additions & 3 deletions lib/_tls_common.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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];
Expand All @@ -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];
Expand Down
64 changes: 41 additions & 23 deletions test/parallel/test-tls-options-boolean-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
});

Expand Down Expand Up @@ -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
}));
});

Expand All @@ -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 });
});
});

Expand All @@ -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 });
});
});

0 comments on commit 1403d28

Please sign in to comment.