reverseproxy: re-normalize WebSocket header casing after header op rebuild#7803
Closed
tejgokani wants to merge 1 commit into
Closed
reverseproxy: re-normalize WebSocket header casing after header op rebuild#7803tejgokani wants to merge 1 commit into
tejgokani wants to merge 1 commit into
Conversation
…build prepareRequest normalizes WebSocket header casing per RFC 6455, but the per-iteration header rebuild in proxyLoopIteration ran copyHeader (which applies http.Header.Add and canonicalizes keys back to Go's textproto form, e.g. Sec-WebSocket-Key -> Sec-Websocket-Key) and never re-normalized afterwards. The rebuild runs whenever any request header op is configured or whenever the HTTPS transport auto-injects its Host op, so the canonical casing reached upstreams that compare header names case-sensitively and rejected the WebSocket handshake. The response-side path in streaming.go was already correct; this brings the request-side path to parity. Extract the inline rebuild block into a small private rebuildRequestHeaders helper (no behavior change beyond the new normalize call) so the bug-prone code path has a clean unit-test seam. Add table-driven regression tests covering user header_up only, the transport-injected Host op alone (simulating an HTTPS upstream), and both together, plus a no-op test that verifies the helper leaves r.Header untouched when no ops are configured. Fixes caddyserver#7784
Author
|
@CLAassistant signed |
This was referenced Jun 6, 2026
Author
|
Closing as a duplicate of #7786, which was opened five days before this one and which I missed during recon. Deferring to that PR. Apologies to @sapirbaruch for the duplicate work — left a comment on #7786 with two things from this PR that might be worth folding in (a regression test that exercises the rebuild path, plus a raw-TCP wire-level repro pattern) in case they save time. |
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
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.
Summary
Fixes #7784. WebSocket header casing (RFC 6455
Sec-WebSocket-*) was being silently reverted to Go's canonical form (Sec-Websocket-*) on the request path whenever anyheader_upwas configured, or whenever the upstream was HTTPS (the HTTP transport auto-injects aHostop for HTTPS upstreams, which triggers the same rebuild block). Upstreams that compare these header names case-sensitively then rejected the WebSocket handshake. The response-side path instreaming.gowas already correct; this brings the request-side path to parity.Problem
prepareRequestnormalizes WebSocket header casing at the end of header preparation. ButproxyLoopIterationrebuildsr.Headeron every iteration (so retries don't accumulate header ops) by callingcopyHeader, which useshttp.Header.Add— andAddrunstextproto.CanonicalMIMEHeaderKey, turningSec-WebSocket-Keyback intoSec-Websocket-Key. Nothing re-normalized afterwards.Repro from the issue:
through
reverse_proxy 127.0.0.1:9081 { header_up X-Repro-Trigger rebuild }.Wire bytes the upstream sees, pre-fix:
Wire bytes the upstream sees, post-fix:
(Captured with a raw
net.Listenupstream so the bytes aren't canonicalized by Go'snet/httpserver on receive.)Solution
Extracted the inline header-rebuild block from
proxyLoopIterationinto a small privaterebuildRequestHeadershelper. The helper preserves the exact prior behavior (same early-out condition, same op-application order) and additionally callsnormalizeWebsocketHeaders(r.Header)at the end, mirroring the response-side normalization atstreaming.go:91. Extracting the helper was necessary to write a regression test that fails if the normalization is later removed.Changes
modules/caddyhttp/reverseproxy/reverseproxy.go— extractrebuildRequestHeadershelper; addnormalizeWebsocketHeaderscall after both transport- and user-ops are applied.modules/caddyhttp/reverseproxy/headers_test.go— addTestRebuildRequestHeadersPreservesWebsocketCasing(table-driven: userheader_uponly, HTTPS transport-injected Host op only, and both together) andTestRebuildRequestHeadersIsNoOpWithoutOps.Testing
Quality gates run locally on macOS arm64 / Go 1.25.5:
I also verified the regression test does its job: with the new
normalizeWebsocketHeaderscall commented out, all three sub-tests ofTestRebuildRequestHeadersPreservesWebsocketCasingfail with the exactSec-Websocket-*leak shown in the issue; restoring the call makes them pass.Manual end-to-end repro (raw
net.Listenupstream +reverse_proxywithheader_up) showed the wire-level header casing transition documented under Problem above.Closes #7784
Assistance Disclosure
I authored and tested the change, and I am able to explain every line. I used Claude (Opus 4.7) for limited assistance: comparing two implementation shapes I had drafted (a one-line inline fix vs. extracting the rebuild block into a small helper) and selecting the shape that gives a clean regression-test seam, plus drafting the doc comment on
rebuildRequestHeaders. The fix logic, test cases, and manual repro were verified by me end-to-end.