From b1127debfc81e5be2eb2e38d97309109d758a8a1 Mon Sep 17 00:00:00 2001 From: waiting <1661926154@qq.com> Date: Tue, 11 Sep 2018 15:18:01 +0800 Subject: [PATCH] refactor: move reading cert file into parseTLSOpts() validate cert file exists during start server --- lib/app_worker.js | 26 ++--------- lib/utils/options.js | 3 +- lib/utils/tls_options.js | 28 +++++++----- test/tls_options.test.js | 99 ++++++++++++++++++++++++++++++++-------- 4 files changed, 104 insertions(+), 52 deletions(-) diff --git a/lib/app_worker.js b/lib/app_worker.js index b543e6f..2f28520 100644 --- a/lib/app_worker.js +++ b/lib/app_worker.js @@ -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; @@ -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); @@ -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); diff --git a/lib/utils/options.js b/lib/utils/options.js index 627c331..687a572 100644 --- a/lib/utils/options.js +++ b/lib/utils/options.js @@ -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 = { @@ -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); diff --git a/lib/utils/tls_options.js b/lib/utils/tls_options.js index 0d9e46c..82d8cce 100644 --- a/lib/utils/tls_options.js +++ b/lib/utils/tls_options.js @@ -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 @@ -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) { @@ -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') { diff --git a/test/tls_options.test.js b/test/tls_options.test.js index ca3b758..aa5a45a 100644 --- a/test/tls_options.test.js +++ b/test/tls_options.test.js @@ -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', () => { @@ -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', () => { @@ -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);