From 734ff13d88903370d32b56127c451c254478d921 Mon Sep 17 00:00:00 2001 From: Tim Perry <1526883+pimterry@users.noreply.github.com> Date: Sat, 20 Sep 2025 20:18:23 +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. PR-URL: https://github.com/nodejs/node/pull/59924 Reviewed-By: Luigi Pinca Reviewed-By: Rafael Gonzaga Reviewed-By: Matteo Collina --- lib/internal/http2/core.js | 3 + test/common/websocket-server.js | 108 ++++++++++++++++++ .../test-http2-allow-http1-upgrade-ws.js | 39 +++++++ 3 files changed, 150 insertions(+) create mode 100644 test/common/websocket-server.js 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 73a478a7b708e2..335113acc8c23d 100644 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -3379,6 +3379,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 new file mode 100644 index 00000000000000..7f2447396972f7 --- /dev/null +++ b/test/common/websocket-server.js @@ -0,0 +1,108 @@ +'use strict'; +const common = require('./index'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const http = require('http'); +const crypto = require('crypto'); + +class WebSocketServer { + constructor({ + port = 0, + server, + }) { + this.port = port; + this.server = server || http.createServer(); + this.clients = new Set(); + + this.server.on('upgrade', this.handleUpgrade.bind(this)); + } + + start() { + return new Promise((resolve) => { + this.server.listen(this.port, () => { + this.port = this.server.address().port; + resolve(); + }); + }).catch((err) => { + console.error('Failed to start WebSocket server:', err); + }); + } + + handleUpgrade(req, socket, head) { + const key = req.headers['sec-websocket-key']; + const acceptKey = this.generateAcceptValue(key); + const responseHeaders = [ + 'HTTP/1.1 101 Switching Protocols', + 'Upgrade: websocket', + 'Connection: Upgrade', + `Sec-WebSocket-Accept: ${acceptKey}`, + ]; + + socket.write(responseHeaders.join('\r\n') + '\r\n\r\n'); + this.clients.add(socket); + + socket.on('data', (buffer) => { + 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; + } + + socket.write(this.encodeMessage('Hello from server!')); + }); + + socket.on('close', () => { + this.clients.delete(socket); + }); + + socket.on('error', (err) => { + console.error('Socket error:', err); + this.clients.delete(socket); + }); + } + + generateAcceptValue(secWebSocketKey) { + return crypto + .createHash('sha1') + .update(secWebSocketKey + '258EAFA5-E914-47DA-95CA-C5AB0DC85B11', 'binary') + .digest('base64'); + } + + decodeMessage(buffer) { + const secondByte = buffer[1]; + const length = secondByte & 127; + const maskStart = 2; + const dataStart = maskStart + 4; + const masks = buffer.slice(maskStart, dataStart); + const data = buffer.slice(dataStart, dataStart + length); + const result = Buffer.alloc(length); + + for (let i = 0; i < length; i++) { + result[i] = data[i] ^ masks[i % 4]; + } + + return result.toString(); + } + + encodeMessage(message) { + const msgBuffer = Buffer.from(message); + const length = msgBuffer.length; + const frame = [0x81]; + + if (length < 126) { + frame.push(length); + } else if (length < 65536) { + frame.push(126, (length >> 8) & 0xff, length & 0xff); + } else { + throw new Error('Message too long'); + } + + return Buffer.concat([Buffer.from(frame), msgBuffer]); + } +} + +module.exports = WebSocketServer; 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());