From d1433e44f492f1e83bc9e5bc9749ab8d1e0fc164 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Thu, 18 Sep 2025 12:53:58 +0200 Subject: [PATCH] http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback This is required to use HTTP/1 websockets on an HTTP/2 server, which is fairly common as websockets over HTTP/2 is much less widely supported. This was broken by the recent shouldUpgradeCallback HTTP/1 addition, which wasn't correctly added to the corresponding allowHttp1 part of the HTTP/2 implementation. --- lib/internal/http2/core.js | 3 ++ test/common/websocket-server.js | 5 ++- .../test-http2-allow-http1-upgrade-ws.js | 39 +++++++++++++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http2-allow-http1-upgrade-ws.js diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index 4462fa639317f0..751e8b189b2462 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -3374,6 +3374,9 @@ class Http2SecureServer extends TLSServer { this.headersTimeout = 60_000; // Minimum between 60 seconds or requestTimeout this.requestTimeout = 300_000; // 5 minutes this.connectionsCheckingInterval = 30_000; // 30 seconds + this.shouldUpgradeCallback = function() { + return this.listenerCount('upgrade') > 0; + }; this.on('listening', setupConnectionsTracking); } if (typeof requestListener === 'function') diff --git a/test/common/websocket-server.js b/test/common/websocket-server.js index a3b12ab8d30d49..7f2447396972f7 100644 --- a/test/common/websocket-server.js +++ b/test/common/websocket-server.js @@ -8,9 +8,10 @@ const crypto = require('crypto'); class WebSocketServer { constructor({ port = 0, + server, }) { this.port = port; - this.server = http.createServer(); + this.server = server || http.createServer(); this.clients = new Set(); this.server.on('upgrade', this.handleUpgrade.bind(this)); @@ -44,6 +45,8 @@ class WebSocketServer { const opcode = buffer[0] & 0x0f; if (opcode === 0x8) { + // Send a minimal close frame in response: + socket.write(Buffer.from([0x88, 0x00])); socket.end(); this.clients.delete(socket); return; diff --git a/test/parallel/test-http2-allow-http1-upgrade-ws.js b/test/parallel/test-http2-allow-http1-upgrade-ws.js new file mode 100644 index 00000000000000..028dc4e4cde71c --- /dev/null +++ b/test/parallel/test-http2-allow-http1-upgrade-ws.js @@ -0,0 +1,39 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); +if (!common.hasCrypto) common.skip('missing crypto'); + +const http2 = require('http2'); + +const undici = require('internal/deps/undici/undici'); +const WebSocketServer = require('../common/websocket-server'); + +(async function main() { + const server = http2.createSecureServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + allowHTTP1: true, + }); + + server.on('request', common.mustNotCall()); + new WebSocketServer({ server }); // Handles websocket 'upgrade' events + + await new Promise((resolve) => server.listen(0, resolve)); + + await new Promise((resolve, reject) => { + const ws = new WebSocket(`wss://localhost:${server.address().port}`, { + dispatcher: new undici.EnvHttpProxyAgent({ + connect: { rejectUnauthorized: false } + }) + }); + ws.addEventListener('open', common.mustCall(() => { + ws.close(); + resolve(); + })); + ws.addEventListener('error', reject); + }); + + server.close(); +})().then(common.mustCall());