Skip to content

fix: honor the configured proxy in HTTP/2 and HTTP/3 code paths - #541

Open
cloud-1104 wants to merge 2 commits into
imroc:masterfrom
cloud-1104:fix-h2-h3-proxy-bypass
Open

cloud-1104 wants to merge 2 commits into
imroc:masterfrom
cloud-1104:fix-h2-h3-proxy-bypass

Conversation

@cloud-1104

Copy link
Copy Markdown

We route outbound HTTPS traffic through HTTP proxies, and noticed requests were occasionally leaving from the machine's real IP straight to origin servers, even though the client had a proxy configured. The tell in the logs was the error shape: plain dial tcp <origin-ip>:443 failures with no proxyconnect prefix. Digging into it, the leak is in the HTTP/2 integration, and there are three variants of it.

1. A dead pooled h2 tunnel makes the next request dial the origin directly

When a proxied HTTPS connection negotiates h2 via ALPN, dialConn registers the tunnel in the h2 conn pool and returns persistConn{alt: t.t2}. Later requests go through alt.RoundTrip, i.e. RoundTripOpt{OnlyCachedConn: false}. On a pool miss — the tunnel died, or the conn hit max concurrent streams — that calls GetClientConn(req, addr, true), which dials with zeroDialer/tls.Dialer straight to the origin authority. That dial never looks at Transport.Proxy, and it doesn't go through the custom dial hooks either.

So the moment a proxy tunnel hiccups, traffic silently leaves the proxy. net/http guards this exact spot with noDialH2RoundTripper: the alt round tripper may only use cached conns, a miss returns ErrNoCachedConn, and the HTTP/1 retry loop then dials again through the proxy. The type exists in internal/http2 but is never referenced — and its RoundTrip calls the dialing RoundTrip, so it wouldn't have worked even if it had been wired up.

2. EnableForceHTTP2 ignores the proxy (#206)

roundTrip short-circuits to t.t2.RoundTrip(req), and the h2 transport always dials the origin directly, proxy or not.

3. HTTP/3 ignores the proxy (#347, #455)

QUIC is UDP and can't traverse a CONNECT or SOCKS proxy, but EnableForceHTTP3 and Alt-Svc h3 upgrades still dial the origin directly when a proxy is set.

Reproduction

Each case has a regression test built around a local CONNECT proxy that counts CONNECT requests. All of them fail before the fix and pass after:

  • TestHTTP2ProxyNoDirectDialOnTunnelDeath: establish h2 through the proxy, kill the tunnel, then the next request must produce a second CONNECT. Before the fix it succeeds with no new CONNECT — direct dial.
  • TestForceHTTP2ViaProxy: forced h2 must go through the proxy. Before: zero CONNECTs.
  • TestForceHTTP2ViaProxyRejectsHTTP1Server: forced h2 against an HTTP/1.1-only server must fail after dialing through the proxy, not by bypassing it. Before: the request never reaches the proxy and fails with http2: unexpected ALPN protocol from a direct dial.
  • TestForceHTTP3WithProxyFails: forced h3 with a proxy now fails fast with req: HTTP3 cannot be used with a proxy. Before: it burned a full QUIC handshake timeout dialing the origin.
  • TestHTTP3AltSvcSkippedWithProxy: an Alt-Svc h3 advertisement with a proxy configured must not trigger any QUIC traffic, verified with a plain UDP listener. Before: probe packets show up.

Fix

  • persistConn.alt is now wrapped in a real NoDialH2RoundTripper (exported from internal/http2, RoundTrip uses RoundTripOnlyCachedConn). Pool miss → ErrNoCachedConn → the HTTP/1 retry loop redials through the proxy. Same mechanism as net/http.
  • Forced h2 for https no longer short-circuits roundTrip; it goes through the standard flow, which honors the proxy and already enforces h2 during the TLS handshake. Also added the missing enforcement to customTlsHandshake (the TLS fingerprinting path), so forced h2 can't silently downgrade to h1 there. h2c stays on the h2 transport, but h2c + proxy now returns an error instead of bypassing. Fixes EnableForceHTTP2 会导致 SetProxyURL 不起作用 #206.
  • HTTP/3 can't work through a proxy at all, so: forced h3 + proxy is an explicit error, and Alt-Svc h3 upgrades (handleAltSvc, checkAltSvc, and the cached-conn fast path) are skipped when a proxy applies — the same fallback browsers do. Related: Proxy over HTTP3 doesn't work #347, making a HTTP3 request using proxy doesnt actually use the proxy. #455.

No behavior change when no proxy is configured. go test ./... passes; the only failures here are TestSetFile and TestTraceInfo, which also fail on a clean checkout of master on this machine.

When an HTTPS request through a CONNECT proxy negotiates HTTP/2 via
ALPN, the tunnel is pooled with persistConn.alt = t.t2. On a pool miss
(dead tunnel, max concurrent streams) the h2 transport dialed the
origin directly with its own dialer, ignoring Transport.Proxy and the
custom dial hooks, so any proxy failure silently turned into direct
traffic. The stdlib guards this spot with noDialH2RoundTripper; the
type existed in internal/http2 but was never used, and its RoundTrip
called the dialing RoundTrip anyway.

- wrap persistConn.alt in an exported NoDialH2RoundTripper that only
  uses cached conns; a miss returns ErrNoCachedConn and the HTTP/1
  retry loop redials through the proxy, same as net/http
- forced HTTP/2 over https no longer short-circuits roundTrip: it goes
  through the standard flow which honors the proxy and enforces h2
  during the TLS handshake (enforcement added to customTlsHandshake
  too). h2c with a proxy now errors instead of bypassing it.
  Fixes imroc#206.
- HTTP/3 cannot traverse a proxy, so forced h3 with a proxy is now an
  explicit error, and Alt-Svc h3 upgrades are skipped when a proxy
  applies. Related: imroc#347, imroc#455.

Regression tests included: each one fails before the fix and passes
after.
@imroc

imroc commented Sep 18, 2026

Copy link
Copy Markdown
Owner

Thanks for the detailed write-up and the regression tests — I reviewed this on a Linux machine (go1.27.0) and independently verified all three bugs. Blocking one issue found below; everything else looks correct.

Verification

  • go build ./... / go vet ./... clean.
  • All 5 new proxy tests fail on master and pass on this branch — the bugs and the tests capturing them are confirmed:
    • TestHTTP2ProxyNoDirectDialOnTunnelDeath — confirmed: dialConn registers the h2 tunnel as persistConn{alt: t.t2}, and a later pool miss went through t2.RoundTrip's direct dial to the origin, ignoring Transport.Proxy. The NoDialH2RoundTripper wrap matches how net/http guards the same spot (http2noDialH2RoundTripper + OnlyCachedConn), and req's retry loop already handles IsNoCachedConnError by dropping the idle conn and re-dialing via getConn, which honors the proxy. The rewrite of the previously-dead noDialH2RoundTripper.RoundTrip (it called the dialing RoundTrip) is also correct.
    • Forced h2 for https now goes through the standard flow: enforcement is complete — addTLS (tunnel path calls it with forProxy=false) and the new customTlsHandshake check both reject non-h2 ALPN, so TestForceHTTP2ViaProxyRejectsHTTP1Server fails exactly after one CONNECT. Good.
    • HTTP/3: explicit error for forced h3 + proxy, and skipping handleAltSvc/checkAltSvc/the cached-conn fast path matches browser behavior.

Blocking issue: TestTraceInfo regression (not environment-specific)

go test ./... on this branch fails TestTraceInfo deterministically (3/3 runs), while clean master passes 3/3 on the same machine:

--- FAIL: TestTraceInfo (0.01s)
    assert.go:31: Expected [true], got [false]
    assert.go:31: Expected [true], got [false]

I suspected the same "fails on clean master here too" as TestSetFile (which does pass on this machine, so that one is indeed environment-specific), but this one is a real regression introduced by the alt wrap:

Root cause: RoundTripOpt's OnlyCachedConn branch hardcodes traceGotConn(req, cc, true):

if opt.OnlyCachedConn {
    cc, err = t.connPool().GetClientConn(req, addr, false)
    ...
    traceGotConn(req, cc, true)   // reused hardcoded

On master, the first request over a freshly dialed h2 conn reached the dialing branch, where reused := !atomic.CompareAndSwapUint32(&cc.reused, 0, 1) correctly reported false. With alt now wrapped in NoDialH2RoundTripper, that first request takes the OnlyCachedConn branch, so GotConnInfo.Reused is reported as true for a brand-new connection — TraceInfo().IsConnReused/WasIdle/ConnIdleTime become wrong for every first h2 request, not just the test.

One-line fix (mirrors the dialing branch):

reused := !atomic.CompareAndSwapUint32(&cc.reused, 0, 1)
traceGotConn(req, cc, reused)

I verified this locally: with that change TestTraceInfo passes, all 5 proxy tests still pass, and the full go test ./... is green. It also keeps req's existing t.t2.RoundTripOnlyCachedConn fast path correct — for genuinely cached conns the CAS has already been consumed by the first request, so it still reports true.

Minor notes (non-blocking)

  • proxyUnsupportedError invokes t.Proxy(req) on every call, and a single request may call it several times (forced-version check, alt-svc handling, h3 cached-conn fast path). A user Proxy func with side effects or logging will fire more often than before — probably fine, just worth being aware.
  • h2c + proxy now returns an explicit error where it previously bypassed the proxy silently. I think that's the right call, but it's a behavior change worth a line in the release notes.
  • Skipping handleAltSvc entirely when a proxy applies also means the advertisement is not stored in the alt-svc jar, so it won't be used even if the proxy is later removed from the client. Same as browsers, just noting it.

With the traceGotConn fix applied, this looks good to me. Since it touches internal/http2/ and the modified transport.go, I'll leave the merge decision to the maintainer.

RoundTripOpt hardcoded reused=true in the OnlyCachedConn branch. Once the
HTTP/2 alt RoundTripper is wrapped in NoDialH2RoundTripper, the first
request over a freshly dialed connection also takes that branch, so
GotConnInfo.Reused (and TraceInfo IsConnReused / IsConnWasIdle /
ConnIdleTime) were wrong for every first HTTP/2 request.

Compute reused with the same compare-and-swap the dialing branch uses, so
a brand new connection reports false and genuinely cached ones report true.
@cloud-1104

Copy link
Copy Markdown
Author

Thanks for the thorough review — you're right on the root cause, and the fix is pushed as b217679.

@imroc

imroc commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Confirmed — b217679 matches the suggested fix exactly (CAS-based reused detection in the OnlyCachedConn branch, mirroring the dialing branch).

Verified locally on top of b217679 (go1.27.0, Linux):

  • TestTraceInfo passes 3/3 runs (was failing 3/3 before the fix, deterministic)
  • all 5 proxy regression tests still pass
  • full go test ./... is green

That resolves the blocking issue from my earlier review; everything else there still stands. From my side this looks good — as before, since it touches internal/http2/ and the modified transport.go, I'll leave the merge decision to the maintainer.

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.

EnableForceHTTP2 会导致 SetProxyURL 不起作用

2 participants