Skip to content

h2: CONNECT crashes the process when the peer sends GOAWAY naming an older stream #5638

Description

@ondraulehla

Bug Description

A CONNECT over HTTP/2 brings the whole process down if the peer answers with a GOAWAY that names an older stream.

severRequestStream() in lib/dispatcher/client-h2.js:224 nulls stream[kHTTP2Session] through closeStreamSession() and detaches completeRequestStream from 'close'. Upgrade streams don't use that listener though, they get onUpgradeStreamClose on line 950, so it stays attached, fires when the stream closes, and calls closeStreamSession() a second time with the session already gone.

The repro below fails on bd43ddf from #5603 and passes on its parent. It landed after the 2026-07-24 releases, so nothing published is affected, but it is on main today (checked at ae4a3e3).

Reproduction

Save as test/repro-h2c.js, then node --test --test-reporter=tap test/repro-h2c.js. Node's own http2 server raises lastStreamID to the last stream it processed and cannot send this GOAWAY, which is why the peer here is a raw socket.

Standalone reproduction script:

'use strict'

const { test } = require('node:test')
const assert = require('node:assert')
const net = require('node:net')
const { H2CClient } = require('..')

const frame = (type, flags, streamId, payload = Buffer.alloc(0)) => {
  const head = Buffer.alloc(9)
  head.writeUIntBE(payload.length, 0, 3)
  head[3] = type
  head[4] = flags
  head.writeUInt32BE(streamId, 5)
  return Buffer.concat([head, payload])
}

const SETTINGS = frame(0x4, 0x0, 0)
const SETTINGS_ACK = frame(0x4, 0x1, 0)
const GOAWAY = frame(0x7, 0x0, 0, Buffer.alloc(8)) // lastStreamID 0, NO_ERROR

test('h2 CONNECT settles when the peer sends GOAWAY naming an older stream', { timeout: 30000 }, async (t) => {
  const server = net.createServer((socket) => {
    let buf = Buffer.alloc(0)
    let preface = false
    let sent = false

    socket.on('error', () => {})
    socket.write(SETTINGS)

    socket.on('data', (chunk) => {
      buf = Buffer.concat([buf, chunk])
      if (!preface) {
        if (buf.length < 24) return
        buf = buf.subarray(24)
        preface = true
      }

      while (buf.length >= 9) {
        const length = buf.readUIntBE(0, 3)
        const type = buf[3]
        const flags = buf[4]
        if (buf.length < 9 + length) break
        buf = buf.subarray(9 + length)

        if (type === 0x4 && !(flags & 0x1)) socket.write(SETTINGS_ACK)
        if (type === 0x1 && !sent) {
          sent = true
          socket.write(GOAWAY)
          setTimeout(() => socket.end(), 50)
        }
      }
    })
  })

  await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve))
  t.after(() => server.close())

  const client = new H2CClient(`http://127.0.0.1:${server.address().port}`)
  t.after(() => client.close())

  await assert.rejects(client.connect({ path: '/' }))
})

Expected Behavior

The request settles. On 1089808, the parent commit, it rejects with InformationalError: HTTP/2: stream closed before response headers and the test passes.

Actual Behavior

not ok 1 - h2 CONNECT settles when the peer sends GOAWAY naming an older stream
  failureType: 'uncaughtException'
  error: "Cannot read properties of null (reading 'Symbol(open streams)')"

The throw comes from closeStreamSession on line 767, called by onUpgradeStreamClose on 781 through onceWrapper, so it leaves the stream's own 'close' handler and there is nothing user code can wrap around it. A plain GET against the same peer is fine, only the upgrade path breaks.

Environment

  • OS: Ubuntu 24.04.4 LTS
  • Node.js version: v24.18.1
  • undici version: main at ae4a3e3

Additional context

I tried the two obvious fixes and neither is one. Detaching onUpgradeStreamClose in severRequestStream, and giving closeStreamSession a null check, both stop the crash, but then the request never settles at all, even when the peer closes the connection right after the GOAWAY. The retry semantics felt like yours to decide, so I stopped there.

kOpenStreams has been double decremented before in #5071 and #5074, along different paths.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions