From dcd2434f3b9243395e91136cbded54203d202d87 Mon Sep 17 00:00:00 2001 From: Loonride Date: Thu, 4 Apr 2019 11:03:12 -0500 Subject: [PATCH] feat: http2 option to enable/disable HTTP/2 with HTTPS (#1721) --- bin/options.js | 5 + lib/Server.js | 33 ++++- lib/options.json | 4 + lib/utils/createConfig.js | 4 + test/CreateConfig.test.js | 22 ++++ test/Http2.test.js | 120 +++++++++++++++++++ test/__snapshots__/CreateConfig.test.js.snap | 32 +++++ 7 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 test/Http2.test.js diff --git a/bin/options.js b/bin/options.js index 35db9b3158..3670ffe15d 100644 --- a/bin/options.js +++ b/bin/options.js @@ -87,6 +87,11 @@ const options = { group: SSL_GROUP, describe: 'HTTPS', }, + http2: { + type: 'boolean', + group: SSL_GROUP, + describe: 'HTTP/2, must be used with HTTPS', + }, key: { type: 'string', describe: 'Path to a SSL key.', diff --git a/lib/Server.js b/lib/Server.js index 60e8de82a9..7134d10b87 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -89,13 +89,19 @@ class Server { if (options.lazy && !options.filename) { throw new Error("'filename' option must be set in lazy mode."); } - + + // if the user enables http2, we can safely enable https + if (options.http2 && !options.https) { + options.https = true; + } + updateCompiler(compiler, options); this.stats = options.stats && Object.keys(options.stats).length ? options.stats : Server.DEFAULT_STATS; + this.hot = options.hot || options.hotOnly; this.headers = options.headers; this.progress = options.progress; @@ -647,7 +653,21 @@ class Server { options.https.key = options.https.key || fakeCert; options.https.cert = options.https.cert || fakeCert; - if (!options.https.spdy) { + // Only prevent HTTP/2 if http2 is explicitly set to false + const isHttp2 = options.http2 !== false; + + // note that options.spdy never existed. The user was able + // to set options.https.spdy before, though it was not in the + // docs. Keep options.https.spdy if the user sets it for + // backwards compatability, but log a deprecation warning. + if (options.https.spdy) { + // for backwards compatability: if options.https.spdy was passed in before, + // it was not altered in any way + this.log.warn( + 'Providing custom spdy server options is deprecated and will be removed in the next major version.' + ); + } else { + // if the normal https server gets this option, it will not affect it. options.https.spdy = { protocols: ['h2', 'http/1.1'], }; @@ -662,7 +682,14 @@ class Server { // - https://github.com/nodejs/node/issues/21665 // - https://github.com/webpack/webpack-dev-server/issues/1449 // - https://github.com/expressjs/express/issues/3388 - if (semver.gte(process.version, '10.0.0')) { + if (semver.gte(process.version, '10.0.0') || !isHttp2) { + if (options.http2) { + // the user explicitly requested http2 but is not getting it because + // of the node version. + this.log.warn( + 'HTTP/2 is currently unsupported for Node 10.0.0 and above, but will be supported once Express supports it' + ); + } this.listeningApp = https.createServer(options.https, app); } else { /* eslint-disable global-require */ diff --git a/lib/options.json b/lib/options.json index 7e4e2b3db9..d3a3bdda62 100644 --- a/lib/options.json +++ b/lib/options.json @@ -162,6 +162,9 @@ } ] }, + "http2": { + "type": "boolean" + }, "contentBase": { "anyOf": [ { @@ -321,6 +324,7 @@ "disableHostCheck": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserver-disablehostcheck)", "public": "should be {String} (https://webpack.js.org/configuration/dev-server/#devserver-public)", "https": "should be {Object|Boolean} (https://webpack.js.org/configuration/dev-server/#devserver-https)", + "http2": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserver-http2)", "contentBase": "should be {Array} (https://webpack.js.org/configuration/dev-server/#devserver-contentbase)", "watchContentBase": "should be {Boolean} (https://webpack.js.org/configuration/dev-server/#devserver-watchcontentbase)", "open": "should be {String|Boolean} (https://webpack.js.org/configuration/dev-server/#devserver-open)", diff --git a/lib/utils/createConfig.js b/lib/utils/createConfig.js index 8804571a1e..16ff397637 100644 --- a/lib/utils/createConfig.js +++ b/lib/utils/createConfig.js @@ -136,6 +136,10 @@ function createConfig(config, argv, { port }) { options.https = true; } + if (argv.http2) { + options.http2 = true; + } + if (argv.key) { options.key = argv.key; } diff --git a/test/CreateConfig.test.js b/test/CreateConfig.test.js index f8bbb12209..c1227fc3f9 100644 --- a/test/CreateConfig.test.js +++ b/test/CreateConfig.test.js @@ -560,6 +560,28 @@ describe('createConfig', () => { expect(config).toMatchSnapshot(); }); + it('http2 option', () => { + const config = createConfig( + webpackConfig, + Object.assign({}, argv, { https: true, http2: true }), + { port: 8080 } + ); + + expect(config).toMatchSnapshot(); + }); + + it('http2 option (in devServer config)', () => { + const config = createConfig( + Object.assign({}, webpackConfig, { + devServer: { https: true, http2: true }, + }), + argv, + { port: 8080 } + ); + + expect(config).toMatchSnapshot(); + }); + it('key option', () => { const config = createConfig( webpackConfig, diff --git a/test/Http2.test.js b/test/Http2.test.js new file mode 100644 index 0000000000..d71e352043 --- /dev/null +++ b/test/Http2.test.js @@ -0,0 +1,120 @@ +'use strict'; + +const path = require('path'); +const request = require('supertest'); +const semver = require('semver'); +const helper = require('./helper'); +const config = require('./fixtures/contentbase-config/webpack.config'); + +const contentBasePublic = path.join( + __dirname, + 'fixtures/contentbase-config/public' +); + +describe('http2', () => { + let server; + let req; + + // HTTP/2 will only work with node versions below 10.0.0 + // since spdy is broken past that point, and this test will only + // work above Node 8.8.0, since it is the first version where the + // built-in http2 module is exposed without need for a flag + // (https://nodejs.org/en/blog/release/v8.8.0/) + // if someone is testing below this Node version and breaks this, + // their tests will not catch it, but CI will catch it. + if ( + semver.gte(process.version, '8.8.0') && + semver.lt(process.version, '10.0.0') + ) { + /* eslint-disable global-require */ + const http2 = require('http2'); + /* eslint-enable global-require */ + describe('http2 works with https', () => { + beforeAll((done) => { + server = helper.start( + config, + { + contentBase: contentBasePublic, + https: true, + http2: true, + }, + done + ); + req = request(server.app); + }); + + it('confirm http2 client can connect', (done) => { + const client = http2.connect('https://localhost:8080', { + rejectUnauthorized: false, + }); + client.on('error', (err) => console.error(err)); + + const http2Req = client.request({ ':path': '/' }); + + http2Req.on('response', (headers) => { + expect(headers[':status']).toEqual(200); + }); + + http2Req.setEncoding('utf8'); + let data = ''; + http2Req.on('data', (chunk) => { + data += chunk; + }); + http2Req.on('end', () => { + expect(data).toEqual(expect.stringMatching(/Heyo/)); + done(); + }); + http2Req.end(); + }); + + afterAll(helper.close); + }); + } + + describe('server works with http2 option, but without https option', () => { + beforeAll((done) => { + server = helper.start( + config, + { + contentBase: contentBasePublic, + http2: true, + }, + done + ); + req = request(server.app); + }); + + it('Request to index', (done) => { + req.get('/').expect(200, /Heyo/, done); + }); + + afterAll(helper.close); + }); + + describe('https without http2 disables HTTP/2', () => { + beforeAll((done) => { + server = helper.start( + config, + { + contentBase: contentBasePublic, + https: true, + http2: false, + }, + done + ); + req = request(server.app); + }); + + it('Request to index', (done) => { + req + .get('/') + .expect(200, /Heyo/) + .then(({ res }) => { + expect(res.httpVersion).not.toEqual('2.0'); + done(); + }); + }); + + afterAll(helper.close); + }); +}); diff --git a/test/__snapshots__/CreateConfig.test.js.snap b/test/__snapshots__/CreateConfig.test.js.snap index 2c631ecd42..2f146b276b 100644 --- a/test/__snapshots__/CreateConfig.test.js.snap +++ b/test/__snapshots__/CreateConfig.test.js.snap @@ -474,6 +474,38 @@ Object { } `; +exports[`createConfig http2 option (in devServer config) 1`] = ` +Object { + "hot": true, + "hotOnly": false, + "http2": true, + "https": true, + "noInfo": true, + "port": 8080, + "publicPath": "/", + "stats": Object { + "cached": false, + "cachedAssets": false, + }, +} +`; + +exports[`createConfig http2 option 1`] = ` +Object { + "hot": true, + "hotOnly": false, + "http2": true, + "https": true, + "noInfo": true, + "port": 8080, + "publicPath": "/", + "stats": Object { + "cached": false, + "cachedAssets": false, + }, +} +`; + exports[`createConfig https option (in devServer config) 1`] = ` Object { "hot": true,