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: #59924 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Author: pimterry

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')

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;

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());