http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback

pimterry · targos · commit c5d910a0a958 · 2025-09-24T09:03:42.000+02:00
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: #59924 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git 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
@@ -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
@@ -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());
