Skip to content

Commit

Permalink
refactor: move reading cert file into parseTLSOpts()
Browse files Browse the repository at this point in the history 
validate cert file exists during start server
  • Loading branch information
@waitingsong
waitingsong committed Sep 11, 2018
1 parent 67f22d2 commit b1127de
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 52 deletions.
26 changes: 5 additions & 21 deletions lib/app_worker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ if (options.require) {
});
}

const fs = require('fs');
const debug = require('debug')('egg-cluster');
const gracefulExit = require('graceful-process');
const ConsoleLogger = require('egg-logger').EggConsoleLogger;
Expand All @@ -22,8 +21,8 @@ debug('new Application with options %j', options);
const app = new Application(options);
const clusterConfig = app.config.cluster || /* istanbul ignore next */ {};
const listenConfig = clusterConfig.listen || /* istanbul ignore next */ {};
listenConfig.https = tlsUtil.parseTLSOpts(listenConfig.https);
const https = tlsUtil.mergeTLSOpts(options.https, listenConfig.https);
let https = tlsUtil.mergeTLSOpts(options.https, listenConfig.https);
https = tlsUtil.parseTLSOpts(https);
const port = options.port = options.port || listenConfig.port;
process.send({ to: 'master', action: 'realport', data: port });
app.ready(startServer);
Expand All @@ -44,24 +43,9 @@ function startServer(err) {

app.removeListener('startTimeout', startTimeoutHandler);

let server;
if (https && typeof https === 'object') {
/* istanbul ignore else */
if (typeof https.ca === 'string') {
https.ca = fs.readFileSync(https.ca);
}
/* istanbul ignore else */
if (typeof https.cert === 'string') {
https.cert = fs.readFileSync(https.cert);
}
/* istanbul ignore else */
if (typeof https.key === 'string') {
https.key = fs.readFileSync(https.key);
}
server = require('https').createServer(https, app.callback());
} else {
server = require('http').createServer(app.callback());
}
const server = https && typeof https === 'object'
? require('https').createServer(https, app.callback())
: require('http').createServer(app.callback());

server.once('error', err => {
consoleLogger.error('[app_worker] server got error: %s, code: %s', err.message, err.code);
Expand Down
3 changes: 1 addition & 2 deletions lib/utils/options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const path = require('path');
const assert = require('assert');
const utils = require('egg-utils');

const parseTLSOpts = require('./tls_options').parseTLSOpts;
// const parseTLSOpts = require('./tls_options').parseTLSOpts;

module.exports = function(options) {
const defaults = {
Expand Down Expand Up @@ -35,7 +35,6 @@ module.exports = function(options) {
assert(egg.Application, `should define Application in ${options.framework}`);
assert(egg.Agent, `should define Agent in ${options.framework}`);

options.https = parseTLSOpts(options.https);
options.port = parseInt(options.port, 10) || undefined;
options.workers = parseInt(options.workers, 10);
if (options.require) options.require = [].concat(options.require);
Expand Down
28 changes: 17 additions & 11 deletions lib/utils/tls_options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const fs = require('fs');
const assert = require('assert');
const deprecate = require('depd')('egg');
// const deprecate = require('depd')('egg');

/**
* Parse TLS SecureContextOptions
Expand All @@ -11,31 +11,33 @@ const deprecate = require('depd')('egg');
* @return {SecureContextOptions|void} parsed options
*/
function parseTLSOpts(options) {
if (options === true) {
const msg = '[master] Deprecated: Please use `https: { key, cert }` instead of `https: true`. Docs: https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options';
deprecate(msg);
return;
} else if (!options) {
const msg = '[master] Deprecated: Please use `https: { key, cert }` instead of `https: true`. Docs: https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options';
assert(!(options === true), msg);

if (!options) {
return;
}

const opts = Object.assign({}, options);

/* istanbul ignore else */
if (typeof opts.ca === 'string') {
assert(opts.ca && fs.existsSync(opts.ca), 'File of https.ca should exists');
assert(opts.ca && fs.existsSync(opts.ca), `File of https.ca should exists: "${opts.ca}"`);
opts.ca = fs.readFileSync(opts.ca);
}
/* istanbul ignore else */
if (typeof opts.cert === 'string') {
assert(opts.cert && fs.existsSync(opts.cert), 'File of https.cert should exists');
assert(opts.cert && fs.existsSync(opts.cert), `File of https.cert should exists: "${opts.cert}"`);
opts.cert = fs.readFileSync(opts.cert);
}
/* istanbul ignore else */
if (typeof opts.key === 'string') {
assert(opts.key && fs.existsSync(opts.key), 'File of https.key should exists');
assert(opts.key && fs.existsSync(opts.key), `File of https.key should exists: "${opts.key}"`);
opts.key = fs.readFileSync(opts.key);
}
/* istanbul ignore else */
if (typeof opts.pfx === 'string') {
assert(opts.pfx && fs.existsSync(opts.pfx), 'File of https.pfx should exists');
assert(opts.pfx && fs.existsSync(opts.pfx), `File of https.pfx should exists: "${opts.pfx}"`);
opts.pfx = fs.readFileSync(opts.pfx);
}

if (Object.keys(opts).length) {
Expand All @@ -53,6 +55,10 @@ function parseTLSOpts(options) {
*/
function mergeTLSOpts(optionsHttps, listenHttps) {
const ret = { };
const msg = '[master] Deprecated: Please use `https: { key, cert }` instead of `https: true`. Docs: https://nodejs.org/api/tls.html#tls_tls_createsecurecontext_options';

assert(!(optionsHttps === true), msg);
assert(!(listenHttps === true), msg);

/* istanbul ignore else */
if (listenHttps && typeof listenHttps === 'object') {
Expand Down
99 changes: 81 additions & 18 deletions test/tls_options.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ const parseTLSOpts = tlsUtil.parseTLSOpts;
describe('test/tls_options.test.js', () => {
describe('Should parseTLSOpts()', () => {
it('with https:true', () => {
const ret = parseTLSOpts(true);
assert(typeof ret === 'undefined');
assert.throws(() => parseTLSOpts(true));
});

it('with https:false', () => {
Expand All @@ -29,52 +28,118 @@ describe('test/tls_options.test.js', () => {
assert(typeof ret === 'undefined');
});

it('with https:key/cert by file type', () => {
it('with invalid https.ca file path', () => {
const opts = {
ca: Math.random().toString(),
};
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.ca should exists: "${opts.ca}"`)
);

opts.ca = '';
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.ca should exists: "${opts.ca}"`)
);
});

it('with invalid https.cert file path', () => {
const opts = {
cert: Math.random().toString(),
};
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.cert should exists: "${opts.cert}"`)
);

opts.cert = '';
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.cert should exists: "${opts.cert}"`)
);
});

it('with invalid https.key file path', () => {
const opts = {
key: Math.random().toString(),
};
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.key should exists: "${opts.key}"`)
);

opts.key = '';
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.key should exists: "${opts.key}"`)
);
});

it('with invalid https.pfx file path', () => {
const opts = {
pfx: Math.random().toString(),
};
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.pfx should exists: "${opts.pfx}"`)
);

opts.pfx = '';
assert.throws(
() => parseTLSOpts(opts),
new RegExp(`File of https.pfx should exists: "${opts.pfx}"`)
);
});

it('with https:key/cert by file path', () => {
const key = join(__dirname, 'fixtures/server.key');
const cert = join(__dirname, 'fixtures/server.crt');
const pfx = join(__dirname, 'fixtures/server.crt');
const opts = {
key,
cert,
pfx,
};
const ret = parseTLSOpts(opts);

assert(ret && Object.keys(ret).length === 2);
assert(ret.key === key);
assert(ret.cert === cert);
assert(ret && Object.keys(ret).length === 3);
assert(ret.key.toString() === readFileSync(key).toString());
assert(ret.cert.toString() === readFileSync(cert).toString());
assert(ret.pfx.toString() === readFileSync(cert).toString());
});

it('with https:key/cert by Buffer type', () => {
it('with https:key/cert by Buffer', () => {
const key = join(__dirname, 'fixtures/server.key');
const cert = join(__dirname, 'fixtures/server.crt');
const opts = {
key: readFileSync(key),
cert: readFileSync(cert),
pfx: readFileSync(cert),
};
const ret = parseTLSOpts(opts);

assert(ret && Object.keys(ret).length === 2);
assert(ret && Object.keys(ret).length === 3);
assert(ret.key.toString() === opts.key.toString());
assert(ret.cert.toString() === opts.cert.toString());
assert(ret.pfx.toString() === opts.pfx.toString());
});
});


describe('Should mergeTLSOpts()', () => {
it('with both optionsHttps and listenHttps invalid', () => {
let ret = mergeTLSOpts(true, true);
assert(typeof ret === 'undefined');

ret = mergeTLSOpts({}, {});
let ret = mergeTLSOpts({}, {});
assert(typeof ret === 'undefined');

ret = mergeTLSOpts({});
assert(typeof ret === 'undefined');

ret = mergeTLSOpts(true);
assert(typeof ret === 'undefined');

ret = mergeTLSOpts();
assert(typeof ret === 'undefined');

assert.throws(() => mergeTLSOpts(true));
assert.throws(() => mergeTLSOpts(true, true));
});

it('with valid optionsHttps and invalid listenHttps', () => {
Expand All @@ -97,9 +162,7 @@ describe('test/tls_options.test.js', () => {
assert(ret && Object.keys(ret).length === 2);
assert(ret && ret.key === opts.key && ret.cert === opts.cert);

ret = mergeTLSOpts(true, opts);
assert(ret && Object.keys(ret).length === 2);
assert(ret && ret.key === opts.key && ret.cert === opts.cert);
assert.throws(() => mergeTLSOpts(true, opts));

ret = mergeTLSOpts(false, opts);
assert(ret && Object.keys(ret).length === 2);
Expand Down

0 comments on commit b1127de

Please sign in to comment.