fix: close rejected HTTP/2 WebSocket streams on v7.x

lib/web/websocket/connection.js

@@ -101,13 +101,13 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
         // The presence of a session property on the socket indicates HTTP2
         // HTTP1
         if (response.socket?.session == null) {
-          failWebsocketConnection(handler, 1002, 'Received network error or non-101 status code.', response.error)
+          failHandshake(handler, response, 1002, 'Received network error or non-101 status code.', response.error)
           return
         }
 
         // HTTP2
         if (response.status !== 200) {
-          failWebsocketConnection(handler, 1002, 'Received network error or non-200 status code.', response.error)
+          failHandshake(handler, response, 1002, 'Received network error or non-200 status code.', response.error)
           return
         }
       }
@@ -122,7 +122,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       //    header list results in null, failure, or the empty byte
       //    sequence, then fail the WebSocket connection.
       if (protocols.length !== 0 && !response.headersList.get('Sec-WebSocket-Protocol')) {
-        failWebsocketConnection(handler, 1002, 'Server did not respond with sent protocols.')
+        failHandshake(handler, response, 1002, 'Server did not respond with sent protocols.')
         return
       }
 
@@ -138,7 +138,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       //    _Fail the WebSocket Connection_.
       //    For H2, no upgrade header is expected.
       if (response.socket.session == null && response.headersList.get('Upgrade')?.toLowerCase() !== 'websocket') {
-        failWebsocketConnection(handler, 1002, 'Server did not set Upgrade header to "websocket".')
+        failHandshake(handler, response, 1002, 'Server did not set Upgrade header to "websocket".')
         return
       }
 
@@ -148,7 +148,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       //    MUST _Fail the WebSocket Connection_.
       //    For H2, no connection header is expected.
       if (response.socket.session == null && response.headersList.get('Connection')?.toLowerCase() !== 'upgrade') {
-        failWebsocketConnection(handler, 1002, 'Server did not set Connection header to "upgrade".')
+        failHandshake(handler, response, 1002, 'Server did not set Connection header to "upgrade".')
         return
       }
 
@@ -162,7 +162,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
       const secWSAccept = response.headersList.get('Sec-WebSocket-Accept')
       const digest = crypto.hash('sha1', keyValue + uid, 'base64')
       if (secWSAccept !== digest) {
-        failWebsocketConnection(handler, 1002, 'Incorrect hash received in Sec-WebSocket-Accept header.')
+        failHandshake(handler, response, 1002, 'Incorrect hash received in Sec-WebSocket-Accept header.')
         return
       }
 
@@ -180,7 +180,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
         extensions = parseExtensions(secExtension)
 
         if (!extensions.has('permessage-deflate')) {
-          failWebsocketConnection(handler, 1002, 'Sec-WebSocket-Extensions header does not match.')
+          failHandshake(handler, response, 1002, 'Sec-WebSocket-Extensions header does not match.')
           return
         }
       }
@@ -201,7 +201,7 @@ function establishWebSocketConnection (url, protocols, client, handler, options)
         // the selected subprotocol values in its response for the connection to
         // be established.
         if (requestProtocols === null || !requestProtocols.includes(secProtocol)) {
-          failWebsocketConnection(handler, 1002, 'Protocol was not set in the opening handshake.')
+          failHandshake(handler, response, 1002, 'Protocol was not set in the opening handshake.')
           return
         }
       }
@@ -296,6 +296,16 @@ function closeWebSocketConnection (object, code, reason, validate = false) {
   }
 }
 
+function failHandshake (handler, response, code, reason, cause) {
+  // The H2 upgrade request has already completed and handed off its stream.
+  // Aborting the request cannot close that stream after handshake validation fails.
+  if (response.socket?.session != null && !response.socket.destroyed) {
+    response.socket.destroy()
+  }
+
+  failWebsocketConnection(handler, code, reason, cause)
+}
+
 /**
  * @param {import('./websocket').Handler} handler
  * @param {number} code

test/websocket/h2-rejected-stream-cleanup.js

@@ -0,0 +1,51 @@
+'use strict'
+
+const { test } = require('node:test')
+const { once } = require('node:events')
+const { createSecureServer } = require('node:http2')
+const { key, cert } = require('@metcoder95/https-pem')
+const { Agent, WebSocket } = require('../..')
+const { runtimeFeatures } = require('../../lib/util/runtime-features')
+
+for (const responseHeaders of [
+  { ':status': 200 },
+  { ':status': 200, 'sec-websocket-protocol': 'chat', 'sec-websocket-accept': 'invalid' }
+]) {
+  test('a rejected H2 WebSocket handshake closes its stream', { skip: !runtimeFeatures.has('crypto'), timeout: 10000 }, async (t) => {
+    const server = createSecureServer({ key, cert, settings: { enableConnectProtocol: true } })
+    const sessions = new Set()
+    let closeStream
+    const streamClosed = new Promise((resolve) => { closeStream = resolve })
+
+    server.on('session', (session) => {
+      sessions.add(session)
+      session.on('close', () => sessions.delete(session))
+    })
+    server.on('stream', (stream) => {
+      stream.on('error', () => {})
+      stream.once('close', closeStream)
+      stream.respond(responseHeaders)
+      // Leave the stream open: the client must close it after rejecting the handshake.
+    })
+
+    server.listen(0)
+    await once(server, 'listening')
+    const dispatcher = new Agent({ allowH2: true, connect: { rejectUnauthorized: false } })
+    t.after(async () => {
+      for (const session of sessions) session.destroy()
+      await new Promise((resolve) => server.close(resolve))
+      await dispatcher.close()
+    })
+
+    const ws = new WebSocket(`wss://localhost:${server.address().port}`, { dispatcher, protocols: ['chat'] })
+    ws.addEventListener('error', () => {})
+    ws.addEventListener('open', () => t.assert.fail('handshake should be rejected'))
+    const [close] = await once(ws, 'close')
+    t.assert.strictEqual(close.code, 1006)
+
+    await new Promise((resolve, reject) => {
+      const timer = setTimeout(() => reject(new Error('H2 stream remained open after WebSocket closed')), 1000)
+      streamClosed.then(() => { clearTimeout(timer); resolve() })
+    })
+  })
+}