Skip to content

Commit

Permalink
Do not use spdy on Node 10
Browse files Browse the repository at this point in the history
`spdy` is effectively unmaintained, and as a consequence of an
implementation that extensively relies on Node’s non-public APIs, broken
on Node 10 and above. In those cases, only https will be used for now.
Once express supports Node's built-in HTTP/2 support, migrating over to
that should be the best way to go.

Fixes: #1449
Fixes: nodejs/node#21665
  • Loading branch information
addaleax committed Aug 28, 2018
1 parent 418493d commit e97d345
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 1 deletion.
16 changes: 15 additions & 1 deletion lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const path = require('path');
const ip = require('ip');
const url = require('url');
const http = require('http');
const https = require('https');
const spdy = require('spdy');
const sockjs = require('sockjs');

Expand Down Expand Up @@ -574,7 +575,20 @@ function Server (compiler, options = {}, _log) {
};
}

this.listeningApp = spdy.createServer(options.https, app);
// `spdy` is effectively unmaintained, and as a consequence of an
// implementation that extensively relies on Node’s non-public APIs, broken
// on Node 10 and above. In those cases, only https will be used for now.
// Once express supports Node's built-in HTTP/2 support, migrating over to
// that should be the best way to go.
// The relevant issues are:
// - https://github.com/nodejs/node/issues/21665
// - https://github.com/webpack/webpack-dev-server/issues/1449
// - https://github.com/expressjs/express/issues/3388
if (+process.version.match(/^v(\d+)/)[1] >= 10) {
this.listeningApp = https.createServer(options.https, app);
} else {
this.listeningApp = spdy.createServer(options.https, app);
}
} else {
this.listeningApp = http.createServer(app);
}
Expand Down
33 changes: 33 additions & 0 deletions test/Https.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
'use strict';

const path = require('path');
const request = require('supertest');
const helper = require('./helper');
const config = require('./fixtures/contentbase-config/webpack.config');
require('mocha-sinon');

const contentBasePublic = path.join(__dirname, 'fixtures/contentbase-config/public');

describe('HTTPS', function testHttps() {
let server;
let req;
afterEach(helper.close);

// Increase the timeout to 20 seconds to allow time for key generation.
this.timeout(20000);

describe('to directory', () => {
before((done) => {
server = helper.start(config, {
contentBase: contentBasePublic,
https: true
}, done);
req = request(server.app);
});

it('Request to index', (done) => {
req.get('/')
.expect(200, /Heyo/, done);
});
});
});

0 comments on commit e97d345

Please sign in to comment.