Skip to content

Commit e97d345

Browse files
committed
Do not use spdy on Node 10
`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
1 parent 418493d commit e97d345

File tree

2 files changed

+48
-1
lines changed

2 files changed

+48
-1
lines changed

lib/Server.js

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ const path = require('path');
1515
const ip = require('ip');
1616
const url = require('url');
1717
const http = require('http');
18+
const https = require('https');
1819
const spdy = require('spdy');
1920
const sockjs = require('sockjs');
2021

@@ -574,7 +575,20 @@ function Server (compiler, options = {}, _log) {
574575
};
575576
}
576577

577-
this.listeningApp = spdy.createServer(options.https, app);
578+
// `spdy` is effectively unmaintained, and as a consequence of an
579+
// implementation that extensively relies on Node’s non-public APIs, broken
580+
// on Node 10 and above. In those cases, only https will be used for now.
581+
// Once express supports Node's built-in HTTP/2 support, migrating over to
582+
// that should be the best way to go.
583+
// The relevant issues are:
584+
// - https://github.com/nodejs/node/issues/21665
585+
// - https://github.com/webpack/webpack-dev-server/issues/1449
586+
// - https://github.com/expressjs/express/issues/3388
587+
if (+process.version.match(/^v(\d+)/)[1] >= 10) {
588+
this.listeningApp = https.createServer(options.https, app);
589+
} else {
590+
this.listeningApp = spdy.createServer(options.https, app);
591+
}
578592
} else {
579593
this.listeningApp = http.createServer(app);
580594
}

test/Https.test.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
'use strict';
2+
3+
const path = require('path');
4+
const request = require('supertest');
5+
const helper = require('./helper');
6+
const config = require('./fixtures/contentbase-config/webpack.config');
7+
require('mocha-sinon');
8+
9+
const contentBasePublic = path.join(__dirname, 'fixtures/contentbase-config/public');
10+
11+
describe('HTTPS', function testHttps() {
12+
let server;
13+
let req;
14+
afterEach(helper.close);
15+
16+
// Increase the timeout to 20 seconds to allow time for key generation.
17+
this.timeout(20000);
18+
19+
describe('to directory', () => {
20+
before((done) => {
21+
server = helper.start(config, {
22+
contentBase: contentBasePublic,
23+
https: true
24+
}, done);
25+
req = request(server.app);
26+
});
27+
28+
it('Request to index', (done) => {
29+
req.get('/')
30+
.expect(200, /Heyo/, done);
31+
});
32+
});
33+
});

0 commit comments

Comments
 (0)