Fix reverse-proxy SSE/streaming fidelity (event: lines, gzip, header propagation, Host)#657
Conversation
The default httputil.SingleHostReverseProxy Director rewrites the outbound URL scheme/host/path but deliberately leaves req.Host as the inbound caller's Host (e.g. authbridge-ab1:8080). Cloudflare-fronted backends like api.anthropic.com validate Host against the request line and reject the mismatch, breaking the real-LLM allow-path. Wrap the existing Director to set req.Host to the backend target's host after the default rewrite runs. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
The reverse proxy only propagated an Authorization mutation from the inbound pipeline's pctx.Headers back onto the forwarded request. A plugin like static-inject that sets x-api-key and deletes Authorization had the x-api-key addition silently dropped and the deleted Authorization still forwarded, so api.anthropic.com saw no valid credential (401). Replace the Authorization-only propagation with a full header-diff sync: delete headers present on r.Header but absent from pctx.Headers, then copy every remaining pctx.Headers entry onto r.Header. Content-Length / Content-Encoding are left untouched since they're managed by the body-rewrite block and transport. Adds tests for: x-api-key set + Authorization deleted reaching the backend, and an untouched header still passing through unchanged. Confirmed the existing Authorization-mint regression test (TestInboundPropagation_RewrittenAuthReachesBackend) still passes. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
…oxy streaming Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe reverse proxy now captures SSE ChangesSSE reverse proxy behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant ReverseProxy
participant InboundPipeline
participant Backend
participant streamingResponseBody
Client->>ReverseProxy: Send streaming request
ReverseProxy->>InboundPipeline: Run inbound pipeline
InboundPipeline-->>ReverseProxy: Return mutated headers
ReverseProxy->>Backend: Forward normalized request
Backend-->>streamingResponseBody: Return SSE event and data lines
streamingResponseBody-->>Client: Re-frame event and data lines
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
huang195
left a comment
There was a problem hiding this comment.
Summary
Perfetto — three tight, correct fixes, each with a test that locks in the exact bug.
- Host rewrite — the Director wrap sets
req.Host = target.Hostafter calling the original director. Correct for Cloudflare/SNI-fronted backends (api.anthropic.com) that validateHostagainst the request line. - Full inbound-header diff propagation — the delete-then-set loop reconciles
pctx.Headers(the canonical clone) againstr.Header, so a plugin-setx-api-keyand a plugin-deletedAuthorizationboth reach the backend correctly. This is exactly what unblocks the #655static-injectplugin — nice tie-in. - SSE
event:preservation —sseframe.Readernow captures theevent:field (reset per frame, last-event-wins per the SSE spec, copied out of the reused buffer) and the re-framer emitsevent: <type>before thedata:lines. Together with theAccept-Encodingstrip (so Go's transport transparently decompresses and the re-framer sees plaintext), this fixes SSE clients (Anthropic SDK) decoding zero typed events.
Tests cover the tricky parts: header set + delete, Host rewrite, event-line preservation, event-reset-per-frame, and the Accept-Encoding strip. One low, optional efficiency note inline (the strip is unconditional); nothing blocking. Bravo. 🇮🇹
Maintainer author, green CI, DCO signed.
Assisted-By: Claude Code
…elines The Director stripped the client's Accept-Encoding on every request, but plaintext is only required when a plugin actually inspects the response body — a StreamingResponder (SSE re-framing) or any ReadsBody/WritesBody plugin (buffered read into pctx.ResponseBody). For a pure pass-through proxy the strip forced needless upstream->client decompression, a small efficiency regression on a remote backend or large non-streamed bodies. Gate the strip on inbound.NeedsBody() || inbound.HasStreamingResponders() so pass-through responses keep the caller's Accept-Encoding intact. Addresses review feedback on rossoctl#657. Update TestReverseProxy_StripsAcceptEncoding to carry a body-inspecting StreamingResponder so the strip stays in scope, and add TestReverseProxy_PreservesAcceptEncoding_PassThrough asserting an empty pipeline preserves the header. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
Fixes #656.
Three reverse-proxy fidelity fixes so
authbridge-proxyfaithfully proxies streaming (SSE) responses — required for spec-compliant SSE clients (e.g. the Anthropic SDK) to work through the reverse proxy when a response-side StreamingResponder plugin (e.g.inference-parser) is active.Commits
req.Host = target.Host, so the backend receives its own host. Cloudflare-fronted backends (api.anthropic.com) reject a mismatchedHost.Authorization, so a header a plugin sets on the inbound pipeline (e.g.x-api-key) actually reaches the backend. Content-Length/Content-Encoding stay managed by the body-rewrite path.event:line and stripAccept-Encoding—sseframe.Readernow captures theevent:field (LastEvent()) and the streaming re-framer emitsevent: <type>before thedata:lines; the Director strips the client'sAccept-Encodingso Go's transport transparently decompresses upstream gzip (noContent-Encoding: gzipover re-emitted plaintext). Without these, SSE clients decode zero events.Testing
go test ./authlib/listener/reverseproxy/... ./authlib/listener/internal/sseframe/...passes, including new tests: sseframe event capture; reverse-proxyevent:-preservation,Accept-Encodingstrip (verified via the Director's output), full inbound-header propagation, and Host rewrite. Validated end-to-end in the RC1 AuthBridge egress PoC (harness → AuthBridge → api.anthropic.com streaming agent leaf, 13/13 live on Kind).Assisted-By: Claude Code
Summary by CodeRabbit
event:types alongsidedata:lines.Hostto match the selected backend.Accept-Encodingwhen response-body inspection is needed.event:preservation,Hostrewriting, header mutation behavior, andAccept-Encodinghandling in streaming paths.