fix: strip hop-by-hop headers from the upstream response on the http1 path - #490
Open
pacocartones wants to merge 1 commit into
Open
fix: strip hop-by-hop headers from the upstream response on the http1 path#490pacocartones wants to merge 1 commit into
pacocartones wants to merge 1 commit into
Conversation
… 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
On the HTTP/1 downstream path, the upstream response headers were forwarded verbatim:
Any hop-by-hop header the upstream set —
keep-alive,proxy-connection,transfer-encoding, or a header it named in its ownConnectionheader — 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:
This applies the same
stripHttp1ConnectionHeaderscall to the HTTP/1 branch, so both downstream transports behave identically.Not a duplicate of #489
#489 strips hop-by-hop headers on the request path (client → upstream). This is the response path (upstream → client). The two are independent; a proxy has to strip on both.
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 usesnode:httpdirectly) and asserts the upstream'sKeep-Alive,proxy-connectionand aConnection-listed custom header are all stripped, while end-to-end headers likecontent-typesurvive.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:typescriptclean.Scope
One line changed in
index.jsplus its test.