Skip to content

Commit

Permalink
https: disallow boolean types for key and cert options
Browse files Browse the repository at this point in the history
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: #12802
  • Loading branch information
jimmycann committed Aug 13, 2017
1 parent b5556e4 commit 262d4b2
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
}
Expand Down
84 changes: 84 additions & 0 deletions test/parallel/test-https-options-boolean-check.js
Original file line number Diff line number Diff line change
@@ -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/);

0 comments on commit 262d4b2

Please sign in to comment.