Skip to content

fix: strip hop-by-hop headers from the upstream response on the http1 path - #490

Open
pacocartones wants to merge 1 commit into
fastify:mainfrom
pacocartones:fix/strip-response-hop-by-hop-headers
Open

fix: strip hop-by-hop headers from the upstream response on the http1 path#490
pacocartones wants to merge 1 commit into
fastify:mainfrom
pacocartones:fix/strip-response-hop-by-hop-headers

Conversation

@pacocartones

@pacocartones pacocartones commented Aug 16, 2026

Copy link
Copy Markdown

What

On the HTTP/1 downstream path, the upstream response headers were forwarded verbatim:

copyHeaders(rewriteHeaders(res.headers, this.request), this)

Any hop-by-hop header the upstream set — keep-alive, proxy-connection, transfer-encoding, or a header it named in its own Connection header — leaked straight to the downstream client. RFC 7230 §6.1 forbids a proxy from forwarding these; they belong to a single transport hop.

The HTTP/2 downstream branch a few lines above already strips them:

rewriteHeaders(stripHttp1ConnectionHeaders(res.headers), this.request)

This applies the same stripHttp1ConnectionHeaders call to the HTTP/1 branch, so both downstream transports behave identically.

Test

Adds test/strip-connection-headers-response.test.js. It reads the raw HTTP/1 response the proxy sends downstream (undici hides some headers, so the test uses node:http directly) and asserts the upstream's Keep-Alive, proxy-connection and a Connection-listed custom header are all stripped, while end-to-end headers like content-type survive.

Verified the test fails on the current code (upstream Keep-Alive should be stripped) and passes with the fix — a genuine regression guard, not a tautology.

npm run test:unit → 166 pass / 0 fail (5 skipped for env). npm run test:typescript clean.

Scope

One line changed in index.js plus its test. This touches only the response path (upstream → client); it is independent of #489, which touches the request path, and the two can merge in either order.

@ilteoood
ilteoood requested review from a team August 27, 2026 16:08
@pacocartones

Copy link
Copy Markdown
Author

Thanks for the review on the response-side fix. The branch is still even with main with CI green, so nothing has drifted since the approvals. Happy to rebase or adjust anything if you'd prefer.

@mcollina

mcollina commented Sep 4, 2026

Copy link
Copy Markdown
Member

CI is red

… path

When the downstream client speaks HTTP/1, the response headers from the upstream
were forwarded verbatim via copyHeaders(rewriteHeaders(res.headers, ...)). Any
hop-by-hop header the upstream set - keep-alive, proxy-connection,
transfer-encoding, or a header it named in its own Connection header - leaked to
the client, which RFC 7230 Section 6.1 forbids a proxy from forwarding.

The HTTP/2 downstream branch a few lines above already strips these with
stripHttp1ConnectionHeaders(res.headers); this applies the same call to the
HTTP/1 branch so both downstream transports behave identically.

Distinct from fastify#489, which strips hop-by-hop headers on the REQUEST path (client
-> upstream). This is the RESPONSE path (upstream -> client); the two are
independent.

Adds a regression test that reads the raw HTTP/1 response the proxy sends
downstream (undici hides some headers) and asserts the upstream's Keep-Alive,
proxy-connection and Connection-listed custom header are all stripped, while
end-to-end headers (content-type) survive. The test fails on the current code
(the upstream Keep-Alive leaks) and passes with the fix.
@pacocartones
pacocartones force-pushed the fix/strip-response-hop-by-hop-headers branch from e3edaec to a10c16c Compare September 4, 2026 15:38
@pacocartones

Copy link
Copy Markdown
Author

CI is red

Done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants