From 262d4b29cba3650ac6d826149e1a7e5e2a2a087f Mon Sep 17 00:00:00 2001 From: Jimmy Cann Date: Mon, 14 Aug 2017 00:24:12 +1000 Subject: [PATCH] https: disallow boolean types for `key` and `cert` options When using https.createServer, passing boolean values for `key` and `cert` properties in the options object parameter doesn't throw an error an could lead to potential issues if they're accidentally passed. This PR aims to throw a reasonable error if a boolean was passed to either of those properties. Fixes: https://github.com/nodejs/node/issues/12802 --- lib/https.js | 6 ++ .../test-https-options-boolean-check.js | 84 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 test/parallel/test-https-options-boolean-check.js diff --git a/lib/https.js b/lib/https.js index fb220872598315..38ae2fa9b93578 100644 --- a/lib/https.js +++ b/lib/https.js @@ -40,6 +40,12 @@ function Server(opts, requestListener) { } opts = util._extend({}, opts); + if (opts && typeof opts.key === 'boolean') + throw new Error('"options.key" must not be a boolean'); + + if (opts && typeof opts.cert === 'boolean') + throw new Error('"options.cert" must not be a boolean'); + if (process.features.tls_npn && !opts.NPNProtocols) { opts.NPNProtocols = ['http/1.1', 'http/1.0']; } diff --git a/test/parallel/test-https-options-boolean-check.js b/test/parallel/test-https-options-boolean-check.js new file mode 100644 index 00000000000000..123b4635b9aa82 --- /dev/null +++ b/test/parallel/test-https-options-boolean-check.js @@ -0,0 +1,84 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const https = require('https'); +const fs = require('fs'); + +assert.doesNotThrow(() => + https.createServer({ + key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`), + cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`) + })); + +assert.throws(() => + https.createServer({ + key: true, + cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`) + }), /"options\.key" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: false, + cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`) + }), /"options\.key" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`), + cert: true + }), /"options\.cert" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`), + cert: false + }), /"options\.cert" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: false, + cert: false + }), /"options\.key" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: true, + cert: true + }), /"options\.key" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: true, + cert: false + }), /"options\.key" must not be a boolean/); + +assert.throws(() => + https.createServer({ + key: false, + cert: true + }), /"options\.key" must not be a boolean/);