fix: detect SSE responses whose content-type carries parameters

[pacocartones]

fix: detect SSE responses whose content-type carries parameters

Summary

lib/request.js disables the upstream request timeout when the upstream answers
with server-sent events, so that an idle SSE stream is not cut. It decides that
with a strict string comparison:

if (res.headers['content-type'] === 'text/event-stream') {
  req.setTimeout(0)
}

That comparison is exact, but the header is not. An upstream that answers
text/event-stream; charset=utf-8 — Starlette's EventSourceResponse does, and
Spring emits text/event-stream;charset=UTF-8 — falls straight through the
check. The timeout stays armed, fires while the SSE stream is idle between
events, and tears the connection down mid-response.

Both the HTTP/1 branch and the HTTP/2 branch have the same comparison.

The mechanism

RFC 9110 §8.3.1 defines the media type as

media-type = type "/" subtype *( OWS ";" OWS parameter )

and states: "The type, subtype, and parameter name tokens are case-insensitive."
So text/event-stream, text/event-stream; charset=utf-8 and
Text/Event-Stream are all the same media type, and only the first one matches
today.

What the user sees when it does not match, in the HTTP/1 path:

1. res.headers['content-type'] is text/event-stream; charset=utf-8, the
   comparison is false, req.setTimeout(0) is skipped.
2. done(null, {...}) runs anyway, so the response headers are forwarded
   downstream and the body starts streaming.
3. The SSE stream goes quiet between events for longer than the configured
   timeout. Because this is a socket-inactivity timeout, it fires.
4. req.once('timeout') builds an HttpRequestTimeoutError and calls
   req.abort(). The client's stream is truncated.
5. The error handler tries to send a 504 — but the headers went out in step 2.
   The proxy throws an uncaught ERR_HTTP_HEADERS_SENT.

Reproduced against main with a 150 ms proxy timeout and an upstream that holds
the stream open for 400 ms:

node v24.14.1 | proxy timeout=150ms | upstream holds stream 400ms

A) text/event-stream
  chunks received               : 2
  stream reached "data: last"   : true
  client error                  : none
  uncaught in proxy process     : none
  => OK

B) text/event-stream; charset=utf-8 (Starlette / Spring)
  chunks received               : 1
  stream reached "data: last"   : false
  client error                  : aborted
  uncaught in proxy process     : ERR_HTTP_HEADERS_SENT
  => BROKEN

C) Text/Event-Stream (RFC 9110 8.3.1: case-insensitive)
  chunks received               : 1
  stream reached "data: last"   : false
  client error                  : aborted
  uncaught in proxy process     : ERR_HTTP_HEADERS_SENT
  => BROKEN

After the fix, all three report OK.

The fix

Parse the header instead of comparing it, with the parser the package already
depends on:

const { safeParse: parseContentType } = require('fast-content-type-parse')

// A media type is case-insensitive and may carry parameters (RFC 9110 §8.3.1),
// so `text/event-stream; charset=utf-8` is the same type as `text/event-stream`.
function isServerSentEvents (contentType) {
  return parseContentType(contentType ?? '').type === 'text/event-stream'
}

used at both call sites.

• No new dependency. fast-content-type-parse is already a direct
  dependency and index.js already uses it this way (index.js, the
  contentTypesToEncode path).
• safeParse, not parse. parse throws on a malformed media type;
  safeParse returns { type: '', parameters: {} }. A malformed or absent
  upstream content-type must not blow up the proxy, and ?? '' keeps the call
  within the declared (header: string) signature.
• safeParse lowercases the type and drops the parameters, which covers both
  halves of the problem in one call.
• No public API change, so types/ is untouched.

Nothing else changed.

Tests

Two tests added, one per transport, cloned from the existing
http sse removes timeout test / http2 sse removes request and session timeout test.

One thing worth flagging: the existing SSE tests answer immediately, so the
timeout never has a chance to fire and they pass whether or not the timeout is
disabled. The new tests keep the stream idle past the timeout, which is what
a real SSE feed does between events, and assert that the whole stream arrives.

Red — against main, without the fix

✔ http sse removes timeout test (17.5971ms)
✖ http sse removes timeout when content-type has parameters (118.8675ms)
✔ http2 sse removes request and session timeout test (13.2886ms)
✖ http2 sse removes request and session timeout when content-type is uppercase and has parameters (130.4865ms)
ℹ tests 9
ℹ pass 7
ℹ fail 2

✖ failing tests:

test at test\http-timeout.test.js:143:1
✖ http sse removes timeout when content-type has parameters (118.8675ms)
  Error [ERR_HTTP_HEADERS_SENT]: Cannot write headers after they are sent to the client
      at ServerResponse.writeHead (node:_http_server:356:11)
      at fallbackErrorHandler (.../fastify/lib/error-handler.js:121:3)
      at onErrorDefault (.../fastify-reply-from/index.js:310:9) {
    code: 'ERR_HTTP_HEADERS_SENT'
  }

test at test\http2-timeout.test.js:180:1
✖ http2 sse removes request and session timeout when content-type is uppercase and has parameters (130.4865ms)
  AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
  + actual - expected

  + 'data: first\n\n'
  - 'data: first\n\ndata: last\n\n'
                    ^

The two reds describe the two halves of the symptom: the HTTP/1 one is the
uncaught ERR_HTTP_HEADERS_SENT inside the proxy, the HTTP/2 one is the
truncated stream the client receives.

Green — with the fix

✔ http request timeout (342.6099ms)
✔ http request with specific timeout (291.9178ms)
✔ http sse removes timeout test (17.1535ms)
✔ http sse removes timeout when content-type has parameters (326.5709ms)
✔ http2 request timeout (340.4261ms)
✔ http2 request with specific timeout (290.048ms)
✔ http2 session timeout (121.3692ms)
✔ http2 sse removes request and session timeout test (12.5479ms)
✔ http2 sse removes request and session timeout when content-type is uppercase and has parameters (316.0554ms)
ℹ tests 9
ℹ pass 9
ℹ fail 0

Full suite and the other CI gates, locally

$ npm run lint
> eslint
(exit 0)

$ npm run test:unit
ℹ tests 171
ℹ pass 166
ℹ fail 0
ℹ skipped 5        # the unix-socket tests, skipped on Windows
All files               |   96.42 |    96.39 |   96.96 |   96.42

$ npm run test:typescript
pass ./types/index.tst.ts
Targets: 1 passed, 1 total    Assertions: 13 passed, 13 total

$ npx license-checker --production --summary --onlyAllow="0BSD;Apache-2.0;BlueOak-1.0.0;BSD-2-Clause;BSD-3-Clause;ISC;MIT;"
├─ MIT: 9
└─ ISC: 2
(exit 0)

What I did not verify

• Only Node 24.14.1 on Windows. CI runs 20/22/24/26 across three OSes. The
  five skipped tests are the unix-socket ones, which do not run on Windows —
  they are skipped on main here too, not by anything in this change.
• The undici path is untouched and still has no SSE handling.
  handleUndici — the default transport when neither http nor http2 is set
  — never had the text/event-stream exemption at all; it relies on
  headersTimeout / bodyTimeout. Whether SSE over the default transport has
  an equivalent problem is a separate question I have not investigated, and this
  PR deliberately does not touch it.
• Timing. The new tests use the same order of magnitude as the existing ones
  in these two files (100 ms timeout, 300 ms hold). I have not run them under
  CI load, so I cannot rule out flakiness on a very slow runner. Happy to raise
  the margins if you would rather.
• multipart/form-data; boundary=... and similar — I only fixed the two SSE
  comparisons. I did not audit the rest of the codebase for other strict
  content-type comparisons.
• Interaction with Use content-type fast parse #472. That open PR swaps fast-content-type-parse for
  content-type@2. If it lands first, the import here needs the same swap;
  it is a one-line follow-up either way. I did not want to guess which way you
  will go, so this PR uses the dependency that is on main today.
• README. The SSE timeout exemption is not documented in the
  "HTTP & HTTP2 timeouts" section at all. I left the docs alone to keep the
  diff to the bug, but I am glad to add a note if you want one.

Checklist

• run npm run test && npm run benchmark --if-present (no benchmark script in this package)
• tests and/or benchmarks are included
• documentation is changed or added — no public API change; see the note above
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

---

This change was written with AI assistance, with me in the loop throughout: I
reproduced the failure myself before writing anything, ran each test against
main to confirm it fails and against the patch to confirm it passes, and ran
every CI gate locally. The literal output of all of it is pasted above.

[mcollina]

lgtm

[pacocartones]

Thanks for the review. The test jobs were red and that was mine, so here is what happened and what changed.

What broke. The http2 test I added hung and took test/http2-timeout.test.js past the 30s file limit in 10 of the 11 jobs. It never reported: the subtests go ok 76 … ok 79 and then the file times out.

Why. The session timer is armed when the http2 client connects, so sessionTimeout: 100 had to cover connecting plus the target's first response. A loaded runner does not fit in that: in the same file http2 request timeout took 6167ms where it takes 325ms on my machine. When the timer fires before the response arrives, done runs a second time with the downstream headers already sent, the proxied response is never terminated, and the test waits forever instead of failing — which is a close cousin of the bug this PR is about.

What changed. Only the two numbers, in both SSE tests: a 1000ms timeout with a 2000ms quiet period. The quiet period still outlasts the timeout, which is the property under test. The http1 sibling passed everywhere, but it has the same shape and the same latent race, so it gets the same headroom rather than flaking on someone else later.

Both are still red without the fix, and for the right reason:

http1   AssertionError: ERR_HTTP_HEADERS_SENT, uncaught, headers already sent
http2   actual   'data: first\n\n'
        expected 'data: first\n\ndata: last\n\n'

Locally: lint clean, 166 unit tests green over three consecutive runs, tstyche 13/13.

[mcollina]

Can you avoid setTImeout? It makes tests falky

[pacocartones]

Done in 8b1d706 — the two tests no longer arm a timer at all.

You were right that the flakiness was mine: those tests set a proxy timeout and then a longer setTimeout on the target, so two clocks were racing. On a loaded runner the proxy timeout could fire before the first response arrived, and the test hung instead of failing.

The realisation was that the thing this PR actually fixes is parsing the content-type — a media type is case-insensitive and may carry parameters (RFC 9110 §8.3.1). That is a pure function, so I made it testable as one: isServerSentEvents is now exported and test/sse-content-type.test.js covers bare, parametrised, uppercase, negative and empty inputs with no server, socket or timer. Reverting it to the old === 'text/event-stream' turns those cases red, which is the regression — so the discriminating coverage lives there now, deterministically.

The two integration tests keep their end-to-end reach but drop the timers: the target answers the SSE response immediately and the test asserts 200 plus the body, the same shape as the * sse removes timeout test already in each file. npm test is green (unit + tstyche), and the parametrised SSE cases now run in ~13ms instead of 2s.