Repository: nodejs/node
Pull request: http2: fix allowHttp1+Upgrade, broken by shouldUpgradeCallback

Fixes #59922

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 (#59824), which wasn't correctly added to the corresponding allowHttp1 part of the HTTP/2 implementation. The test here passes in v24.8.0, but fails on `main` without the fix here.

This also includes a fix for the websocket test server's connection shutdown. Without this, the recent update to Undici v7.16.0 on `main` means that this throws an error for unclear websocket closure (see https://github.com/nodejs/undici/issues/4487).

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
2. Update documentation if relevant.
3. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: lib/internal/http2/core.js
patch: @@ -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')

filename: test/common/websocket-server.js
patch: @@ -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;

filename: test/parallel/test-http2-allow-http1-upgrade-ws.js
patch: @@ -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());