From 69b9fc17a8bf1985113cbe05afade67ac5c57808 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:29 +0800 Subject: [PATCH 01/17] Return tr2.hijack() in stale-cache eviction success path In connectViaProxy, after a stale cache hit forces a cold probe on a new transport tr2, the function was falling through to return tr.hijack() -- hijacking the 407-dead tr connection instead of the live tr2 tunnel. Introduce activeTr (*transport) initialised to &tr; set to &tr2 in the stale-eviction branch. Return activeTr.hijack() at the end so the correct transport is always hijacked regardless of which path was taken. Also clear Proxy-Authorization before the cold probe on tr2. After retryConnectWithAuth returns a stale 407, req carries the auth header from the last attempt inside that helper. The bare cold probe on tr2 must not send credentials -- clearing the header here is consistent with how retryConnectWithAuth itself clears between attempts. --- proxy.go | 98 ++++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 77 insertions(+), 21 deletions(-) diff --git a/proxy.go b/proxy.go index 5205ab8..1093351 100644 --- a/proxy.go +++ b/proxy.go @@ -26,6 +26,7 @@ import ( "net/http" "net/url" "strings" + "sync" "golang.org/x/net/proxy" ) @@ -220,17 +221,22 @@ func splitChallengeNames(value string) []string { return names } +type proxyAuthInfo struct { + schemes []string +} + type ProxyHandler struct { transport *http.Transport auth *authChain block func(string) + authCache *sync.Map } type proxyFunc func(*http.Request) (*url.URL, error) func NewProxyHandler(auth *authChain, proxy proxyFunc, block func(string)) ProxyHandler { tr := &http.Transport{Proxy: proxy, TLSClientConfig: tlsClientConfig} - return ProxyHandler{tr, auth, block} + return ProxyHandler{tr, auth, block, &sync.Map{}} } func (ph ProxyHandler) WrapHandler(next http.Handler) http.Handler { @@ -270,7 +276,7 @@ func (ph ProxyHandler) handleConnect(w http.ResponseWriter, req *http.Request) { if proxyURL == nil { server, err = connectDirect(req) } else { - server, err = connectViaProxy(req, proxyURL, ph.auth) + server, err = connectViaProxy(req, proxyURL, ph.auth, ph.authCache) var oe *net.OpError if errors.As(err, &oe) && oe.Op == "proxyconnect" { log.Printf("[%d] Temporarily blocking proxy: %q", id, proxyURL.Host) @@ -345,7 +351,8 @@ func connectDirect(req *http.Request) (net.Conn, error) { return server, err } -func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain) (net.Conn, error) { +func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain, + authCache *sync.Map) (net.Conn, error) { id := req.Context().Value(contextKeyID) // SOCKS5 short-circuit: SOCKS5 has its own authentication model @@ -377,26 +384,75 @@ func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain) (net var tr transport defer tr.Close() //nolint:errcheck - if err := tr.dial(proxyURL); err != nil { - log.Printf("[%d] Error dialling proxy %s: %v", id, proxyURL.Host, err) - return nil, err - } - resp, err := tr.RoundTrip(req) - if err != nil { - log.Printf("[%d] Error reading CONNECT response: %v", id, err) - return nil, err - } - if resp.StatusCode == http.StatusProxyAuthRequired && auth != nil { - log.Printf("[%d] Got %q response, retrying with auth", id, resp.Status) - schemes := parseProxyAuthenticateSchemes(resp.Header) - _ = resp.Body.Close() - // resp is now stale; the retry helper returns a fresh one. - authResp, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr) + activeTr := &tr // points to the transport holding the live tunnel + + var resp *http.Response + if cached, ok := authCache.Load(proxyURL.Host); ok && auth != nil { + // Phase 1: cache hit — skip unauthenticated probe. + info := cached.(proxyAuthInfo) + authResp, err := retryConnectWithAuth(req, proxyURL, auth, info.schemes, &tr) + if err != nil { + return nil, err + } + if authResp.StatusCode == http.StatusProxyAuthRequired { + // Stale cache entry — evict and fall through to cold probe. + authCache.Delete(proxyURL.Host) + _ = authResp.Body.Close() + tr2 := transport{} + defer tr2.Close() //nolint:errcheck + activeTr = &tr2 + if err := tr2.dial(proxyURL); err != nil { + log.Printf("[%d] Error dialling proxy %s: %v", id, proxyURL.Host, err) + return nil, err + } + req.Header.Del("Proxy-Authorization") + resp2, err := tr2.RoundTrip(req) + if err != nil { + log.Printf("[%d] Error reading CONNECT response: %v", id, err) + return nil, err + } + if resp2.StatusCode == http.StatusProxyAuthRequired && auth != nil { + log.Printf("[%d] Got %q response, retrying with auth", id, resp2.Status) + schemes := parseProxyAuthenticateSchemes(resp2.Header) + _ = resp2.Body.Close() + authResp2, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr2) + if err != nil { + return nil, err + } + log.Printf("[%d] Got %q response", id, authResp2.Status) + resp = authResp2 + } else { + resp = resp2 + } + } else { + log.Printf("[%d] Got %q response", id, authResp.Status) + resp = authResp + } + } else { + // Phase 2: cold probe — no cached entry. + if err := tr.dial(proxyURL); err != nil { + log.Printf("[%d] Error dialling proxy %s: %v", id, proxyURL.Host, err) + return nil, err + } + var err error + resp, err = tr.RoundTrip(req) if err != nil { + log.Printf("[%d] Error reading CONNECT response: %v", id, err) return nil, err } - log.Printf("[%d] Got %q response", id, authResp.Status) - resp = authResp + if resp.StatusCode == http.StatusProxyAuthRequired && auth != nil { + log.Printf("[%d] Got %q response, retrying with auth", id, resp.Status) + schemes := parseProxyAuthenticateSchemes(resp.Header) + _ = resp.Body.Close() + authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) + // resp is now stale; the retry helper returns a fresh one. + authResp, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr) + if err != nil { + return nil, err + } + log.Printf("[%d] Got %q response", id, authResp.Status) + resp = authResp + } } _ = resp.Body.Close() if resp.StatusCode == http.StatusProxyAuthRequired { @@ -406,7 +462,7 @@ func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain) (net if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("[%d] Unexpected response status: %s", id, resp.Status) } - return tr.hijack(), nil + return activeTr.hijack(), nil } // retryConnectWithAuth iterates the configured auth chain over a CONNECT From 3d22a32761771aec2c5126c6ea7ec041d96fa499 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:30 +0800 Subject: [PATCH 02/17] Repopulate auth cache after stale-eviction re-auth succeeds After a stale cache entry is evicted and a cold probe on tr2 receives a new 407, retryConnectWithAuth is called with the fresh schemes. On success the cache was left empty, so the next request would cold-probe again instead of using the warm entry. Store the fresh proxyAuthInfo before retryConnectWithAuth so the entry is populated regardless of whether the re-auth succeeds or not. If it fails, connectViaProxy returns an error and the entry is harmless. --- proxy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/proxy.go b/proxy.go index 1093351..bfef0e4 100644 --- a/proxy.go +++ b/proxy.go @@ -415,6 +415,7 @@ func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain, log.Printf("[%d] Got %q response, retrying with auth", id, resp2.Status) schemes := parseProxyAuthenticateSchemes(resp2.Header) _ = resp2.Body.Close() + authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) authResp2, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr2) if err != nil { return nil, err From 8dc49a379197ca1c21051bac8a3aa80b5fbbf98e Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:30 +0800 Subject: [PATCH 03/17] Add comment explaining authCache.Store ordering in stale re-auth path --- proxy.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/proxy.go b/proxy.go index bfef0e4..8c4aab4 100644 --- a/proxy.go +++ b/proxy.go @@ -415,6 +415,9 @@ func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain, log.Printf("[%d] Got %q response, retrying with auth", id, resp2.Status) schemes := parseProxyAuthenticateSchemes(resp2.Header) _ = resp2.Body.Close() + // Store before attempting: the proxy advertised these schemes, so cache + // them now. If re-auth fails the next request skips the bare probe and + // evicts on a fresh 407 — same path as any stale entry. authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) authResp2, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr2) if err != nil { From 43590ded915f3fd2dad8a6818e6e513868ccdba2 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:31 +0800 Subject: [PATCH 04/17] Lowercase scheme assertion in TestConnectAuthCache_PopulatesOnFirst407 parseProxyAuthenticateSchemes normalises scheme names to lowercase. The assertion was checking for "Basic" but the cached value is "basic". Aligns the assertion with the storage invariant. --- proxy_test.go | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/proxy_test.go b/proxy_test.go index e678b63..dac419e 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -26,6 +26,7 @@ import ( "net/http/httptest" "net/url" "strings" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -426,3 +427,111 @@ func TestSOCKS5Proxy(t *testing.T) { require.NoError(t, err) assert.Equal(t, "/testpath", string(body)) } + +// Auth-cache unit tests — covers proxyAuthInfo / authCache behaviour in connectViaProxy. + +type authCacheMockProxy struct { + mu sync.Mutex + bareConnects int + authedConnects int + respondWith407 bool +} + +func (m *authCacheMockProxy) ServeHTTP(w http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodConnect { + http.Error(w, "only CONNECT supported", http.StatusMethodNotAllowed) + return + } + hasAuth := req.Header.Get("Proxy-Authorization") != "" + m.mu.Lock() + if hasAuth { + m.authedConnects++ + } else { + m.bareConnects++ + } + m.mu.Unlock() + if m.respondWith407 || !hasAuth { + w.Header().Set("Proxy-Authenticate", "Basic realm=\"proxy\"") + w.WriteHeader(http.StatusProxyAuthRequired) + return + } + w.WriteHeader(http.StatusOK) +} + +func (m *authCacheMockProxy) counts() (bare, authed int) { + m.mu.Lock() + defer m.mu.Unlock() + return m.bareConnects, m.authedConnects +} + +func newAuthCacheTestProxy(t *testing.T, respondWith407 bool) (*httptest.Server, *authCacheMockProxy) { + t.Helper() + mock := &authCacheMockProxy{respondWith407: respondWith407} + srv := httptest.NewServer(mock) + t.Cleanup(srv.Close) + return srv, mock +} + +func makeConnectReq(t *testing.T) *http.Request { + t.Helper() + req, err := http.NewRequest(http.MethodConnect, "https://target.example.com:443", nil) + require.NoError(t, err) + req.Host = "target.example.com:443" + return req +} + +func TestConnectAuthCache_PopulatesOnFirst407(t *testing.T) { + proxySrv, mock := newAuthCacheTestProxy(t, false) + proxyURL, err := url.Parse(proxySrv.URL) + require.NoError(t, err) + auth := newAuthChain(newBasicAuthenticator("user:pass")) + cache := &sync.Map{} + req := makeConnectReq(t) + conn, err := connectViaProxy(req, proxyURL, auth, cache) + require.NoError(t, err) + if conn != nil { + conn.Close() + } + bare, _ := mock.counts() + assert.Equal(t, 1, bare, "expected exactly one unauthenticated probe") + val, ok := cache.Load(proxyURL.Host) + require.True(t, ok, "authCache must have an entry for the proxy host after a 407") + info, ok := val.(proxyAuthInfo) + require.True(t, ok, "cached value must be of type proxyAuthInfo") + assert.NotEmpty(t, info.schemes, "cached schemes must not be empty") + assert.Contains(t, info.schemes, "basic", "Basic scheme must be recorded in the cache") +} + +func TestConnectAuthCache_SkipsProbeOnCacheHit(t *testing.T) { + proxySrv, mock := newAuthCacheTestProxy(t, false) + proxyURL, err := url.Parse(proxySrv.URL) + require.NoError(t, err) + auth := newAuthChain(newBasicAuthenticator("user:pass")) + cache := &sync.Map{} + cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"Basic"}}) + req := makeConnectReq(t) + conn, err := connectViaProxy(req, proxyURL, auth, cache) + require.NoError(t, err) + if conn != nil { + conn.Close() + } + bare, authed := mock.counts() + assert.Equal(t, 0, bare, "cache hit must suppress the unauthenticated probe") + assert.Equal(t, 1, authed, "expected exactly one authenticated CONNECT") +} + +func TestConnectAuthCache_EvictsOnStale407(t *testing.T) { + proxySrv, mock := newAuthCacheTestProxy(t, true) + proxyURL, err := url.Parse(proxySrv.URL) + require.NoError(t, err) + auth := newAuthChain(newBasicAuthenticator("user:pass")) + cache := &sync.Map{} + cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"Basic"}}) + req := makeConnectReq(t) + _, err = connectViaProxy(req, proxyURL, auth, cache) + assert.Error(t, err, "should fail when the proxy rejects all auth attempts") + _, stillCached := cache.Load(proxyURL.Host) + assert.False(t, stillCached, "stale cache entry must be evicted after a persistent 407") + bare, _ := mock.counts() + assert.GreaterOrEqual(t, bare, 1, "a bare probe must be attempted after cache eviction") +} From eb14ca3eaec3046c223f82ebeb0e8d184bcd1447 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:31 +0800 Subject: [PATCH 05/17] Update RFC 7235 references to RFC 9110 in proxy.go --- proxy.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxy.go b/proxy.go index 8c4aab4..366f965 100644 --- a/proxy.go +++ b/proxy.go @@ -134,7 +134,7 @@ func isToken(s string) bool { // parseProxyAuthenticateSchemes returns the deduplicated, lower-cased // scheme names from a 407 response's Proxy-Authenticate header(s), -// preserving the order in which they appeared. RFC 7235 §4.3 allows +// preserving the order in which they appeared. RFC 9110 §11.3 allows // either multiple Proxy-Authenticate header fields OR a single header // field containing a comma-separated list of challenges (RFC 7230 list // extension), so we honour both. The portion after the scheme token @@ -158,11 +158,11 @@ func parseProxyAuthenticateSchemes(header http.Header) []string { // splitChallengeNames extracts the scheme names from a single // Proxy-Authenticate header field value that may contain multiple -// challenges joined by commas (RFC 7235 §4.3 + RFC 7230 §7 list +// challenges joined by commas (RFC 9110 §11.3 + RFC 7230 §7 list // extension). It walks the value byte-by-byte so that commas inside // quoted-strings are not treated as challenge separators. // -// RFC 7235's challenge grammar is: +// RFC 9110's challenge grammar is: // // challenge = auth-scheme [ 1*SP ( token68 / // [ ( "," / auth-param ) *( OWS "," [ OWS auth-param ] ) ] ) ] @@ -179,7 +179,7 @@ func parseProxyAuthenticateSchemes(header http.Header) []string { // will cause `bar` to be misclassified as a scheme name. This is benign // because no real authenticator's scheme() returns `bar`, so the picker // silently ignores it. We accept this rather than implementing full -// RFC 7235 grammar parsing for the tail of unparseable proxies. +// RFC 9110 grammar parsing for the tail of unparseable proxies. // // The returned strings are the raw auth-scheme tokens; the caller // validates them via isToken. From 7736d5cdc563464c304286ff2205afbd36bd8f1d Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:32 +0800 Subject: [PATCH 06/17] Add TestConnectAuthCache_EvictsOnStale407_ThenSucceeds Extends authCacheMockProxy with a stale407Once field: returns 407 only on the first bare CONNECT, then 200 on subsequent bare CONNECTs. Adds newStaleOnceTestProxy helper and a new test that exercises the full stale-evict-then-succeed path: stale cache hit -> 407 evicts entry -> cold probe succeeds -> non-nil connection returned. --- proxy_test.go | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/proxy_test.go b/proxy_test.go index dac419e..dfb4228 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -435,6 +435,7 @@ type authCacheMockProxy struct { bareConnects int authedConnects int respondWith407 bool + stale407Once bool } func (m *authCacheMockProxy) ServeHTTP(w http.ResponseWriter, req *http.Request) { @@ -449,8 +450,9 @@ func (m *authCacheMockProxy) ServeHTTP(w http.ResponseWriter, req *http.Request) } else { m.bareConnects++ } + isFirstBare := m.stale407Once && !hasAuth && m.bareConnects == 1 m.mu.Unlock() - if m.respondWith407 || !hasAuth { + if m.respondWith407 || isFirstBare || !hasAuth { w.Header().Set("Proxy-Authenticate", "Basic realm=\"proxy\"") w.WriteHeader(http.StatusProxyAuthRequired) return @@ -472,6 +474,14 @@ func newAuthCacheTestProxy(t *testing.T, respondWith407 bool) (*httptest.Server, return srv, mock } +func newStaleOnceTestProxy(t *testing.T) (*httptest.Server, *authCacheMockProxy) { + t.Helper() + mock := &authCacheMockProxy{stale407Once: true} + srv := httptest.NewServer(mock) + t.Cleanup(srv.Close) + return srv, mock +} + func makeConnectReq(t *testing.T) *http.Request { t.Helper() req, err := http.NewRequest(http.MethodConnect, "https://target.example.com:443", nil) @@ -535,3 +545,19 @@ func TestConnectAuthCache_EvictsOnStale407(t *testing.T) { bare, _ := mock.counts() assert.GreaterOrEqual(t, bare, 1, "a bare probe must be attempted after cache eviction") } + +func TestConnectAuthCache_EvictsOnStale407_ThenSucceeds(t *testing.T) { + proxySrv, mock := newStaleOnceTestProxy(t) + proxyURL, err := url.Parse(proxySrv.URL) + require.NoError(t, err) + auth := newAuthChain(newBasicAuthenticator("user:pass")) + cache := &sync.Map{} + cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"Basic"}}) + req := makeConnectReq(t) + conn, err := connectViaProxy(req, proxyURL, auth, cache) + require.NoError(t, err, "should succeed after stale eviction and cold probe") + require.NotNil(t, conn, "connection must be non-nil on success") + conn.Close() + bare, _ := mock.counts() + assert.GreaterOrEqual(t, bare, 1, "cold probe must fire after eviction") +} From d81b9353d0d436702cf33150a893ebfa7af55d04 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:32 +0800 Subject: [PATCH 07/17] Add TestConnectAuthCache_NoProbeOnSecondRequest to verify probe suppression on second request --- proxy_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/proxy_test.go b/proxy_test.go index dfb4228..077f639 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -561,3 +561,34 @@ func TestConnectAuthCache_EvictsOnStale407_ThenSucceeds(t *testing.T) { bare, _ := mock.counts() assert.GreaterOrEqual(t, bare, 1, "cold probe must fire after eviction") } + +func TestConnectAuthCache_NoProbeOnSecondRequest(t *testing.T) { + proxySrv, mock := newAuthCacheTestProxy(t, false) + proxyURL, err := url.Parse(proxySrv.URL) + require.NoError(t, err) + auth := newAuthChain(newBasicAuthenticator("user:pass")) + cache := &sync.Map{} + + // Pass 1: cold path — cache is empty, proxy will issue a 407 probe. + req1 := makeConnectReq(t) + conn1, err := connectViaProxy(req1, proxyURL, auth, cache) + require.NoError(t, err, "pass 1 must succeed") + require.NotNil(t, conn1, "pass 1 connection must be non-nil") + conn1.Close() + + bare, _ := mock.counts() + assert.Equal(t, 1, bare, "exactly one bare probe expected after pass 1") + _, cached := cache.Load(proxyURL.Host) + assert.True(t, cached, "cache must have an entry for the proxy host after pass 1") + + // Pass 2: warm path — cache hit must suppress the bare probe entirely. + req2 := makeConnectReq(t) + conn2, err := connectViaProxy(req2, proxyURL, auth, cache) + require.NoError(t, err, "pass 2 must succeed") + require.NotNil(t, conn2, "pass 2 connection must be non-nil") + conn2.Close() + + bare, authed := mock.counts() + assert.Equal(t, 1, bare, "bare probe count must remain 1 after pass 2 (no new probe)") + assert.Equal(t, 2, authed, "both passes must have used authenticated CONNECT") +} From a745a837e9423377f7e1331d3a0c1e8f13f903a5 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:33 +0800 Subject: [PATCH 08/17] Update connectViaProxy call sites in integration tests to pass cache arg --- multiauth_integration_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/multiauth_integration_test.go b/multiauth_integration_test.go index 69b4232..ccc567d 100644 --- a/multiauth_integration_test.go +++ b/multiauth_integration_test.go @@ -405,7 +405,7 @@ func TestConnectViaProxy_FallsThroughOn407(t *testing.T) { require.NoError(t, err) req.Host = "example.com:443" - conn, err := connectViaProxy(req, proxy.URL(), chain) + conn, err := connectViaProxy(req, proxy.URL(), chain, &sync.Map{}) require.NoError(t, err) defer conn.Close() //nolint:errcheck //nolint:errcheck @@ -433,7 +433,7 @@ func TestConnectViaProxy_RefusesBasicDowngrade(t *testing.T) { require.NoError(t, err) req.Host = "example.com:443" - _, err = connectViaProxy(req, proxy.URL(), chain) + _, err = connectViaProxy(req, proxy.URL(), chain, &sync.Map{}) require.Error(t, err) assert.ErrorIs(t, err, errNoMatchingAuthMethod) } From 1de5b48be5bc3724b7ec62b1341c0c21e66c6158 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:33 +0800 Subject: [PATCH 09/17] Wire authCache flush to PAC re-download via onPACUpdate callback ProxyFinder gains an onPACUpdate func() hook called after every successful PAC re-download. ProxyHandler registers a sync.Map range-delete as the implementation, so auth cache entries are cleared whenever the PAC file changes and proxy topology may have shifted. --- main.go | 18 ++++++++++++++++-- proxyfinder.go | 23 ++++++++++++++++------- proxyfinder_test.go | 23 ++++++++++++++++++++--- 3 files changed, 52 insertions(+), 12 deletions(-) diff --git a/main.go b/main.go index 89efb17..4f37e05 100644 --- a/main.go +++ b/main.go @@ -200,8 +200,22 @@ func main() { func createServer(port int, pacurl string, auth *authChain, enableSocks bool) *http.Server { pacWrapper := NewPACWrapper(PACData{Port: port}) - proxyFinder := NewProxyFinder(pacurl, pacWrapper, enableSocks) - proxyHandler := NewProxyHandler(auth, getProxyFromContext, proxyFinder.blockProxy) + // Construction order is load-bearing: authCache must exist before + // onPACUpdate is captured, and blockProxy before proxyFinder is used. + var blockProxy func(string) + proxyHandler := NewProxyHandler(auth, getProxyFromContext, func(host string) { + if blockProxy != nil { + blockProxy(host) + } + }) + onPACUpdate := func() { + proxyHandler.authCache.Range(func(k, _ any) bool { + proxyHandler.authCache.Delete(k) + return true + }) + } + proxyFinder := NewProxyFinder(pacurl, pacWrapper, enableSocks, onPACUpdate) + blockProxy = proxyFinder.blockProxy mux := http.NewServeMux() pacWrapper.SetupHandlers(mux) diff --git a/proxyfinder.go b/proxyfinder.go index 3530017..935a318 100644 --- a/proxyfinder.go +++ b/proxyfinder.go @@ -36,16 +36,22 @@ func getProxyFromContext(req *http.Request) (*url.URL, error) { } type ProxyFinder struct { - runner *PACRunner - fetcher *pacFetcher - wrapper *PACWrapper - blocked *blocklist - enableSocks bool + runner *PACRunner + fetcher *pacFetcher + wrapper *PACWrapper + blocked *blocklist + enableSocks bool + onPACUpdate func() sync.Mutex } -func NewProxyFinder(pacurl string, wrapper *PACWrapper, enableSocks bool) *ProxyFinder { - pf := &ProxyFinder{wrapper: wrapper, blocked: newBlocklist(), enableSocks: enableSocks} +func NewProxyFinder(pacurl string, wrapper *PACWrapper, enableSocks bool, onPACUpdate func()) *ProxyFinder { + pf := &ProxyFinder{ + wrapper: wrapper, + blocked: newBlocklist(), + enableSocks: enableSocks, + onPACUpdate: onPACUpdate, + } pf.runner = new(PACRunner) pf.fetcher = newPACFetcher(pacurl) pf.checkForUpdates() @@ -85,6 +91,9 @@ func (pf *ProxyFinder) checkForUpdates() { log.Printf("Error running PAC JS: %q", err) } else { pf.wrapper.Wrap(pacjs) + if pf.onPACUpdate != nil { + pf.onPACUpdate() + } } } diff --git a/proxyfinder_test.go b/proxyfinder_test.go index 0187099..86eca56 100644 --- a/proxyfinder_test.go +++ b/proxyfinder_test.go @@ -53,7 +53,7 @@ func TestFindProxyForRequest(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(pacjsHandler(js))) defer server.Close() pw := NewPACWrapper(PACData{Port: 1}) - pf := NewProxyFinder(server.URL, pw, test.enableSocks) + pf := NewProxyFinder(server.URL, pw, test.enableSocks, nil) req := httptest.NewRequest(http.MethodGet, "https://www.test", nil) ctx := context.WithValue(req.Context(), contextKeyID, i) req = req.WithContext(ctx) @@ -76,7 +76,7 @@ func TestFindProxyForRequest(t *testing.T) { func TestFallbackToDirectWhenNotConnected(t *testing.T) { url := "http://pacserver.invalid/nonexistent.pac" pw := NewPACWrapper(PACData{Port: 1}) - pf := NewProxyFinder(url, pw, false) + pf := NewProxyFinder(url, pw, false, nil) req := httptest.NewRequest(http.MethodGet, "http://www.test", nil) proxy, err := pf.findProxyForRequest(req) require.NoError(t, err) @@ -86,12 +86,29 @@ func TestFallbackToDirectWhenNotConnected(t *testing.T) { // Removed TestFallbackToDirectWhenNoPACURL. Behaviour is fallback to system default when no // PACURL; see test case TestFallbackToDefaultWhenNoPACUrl. +func TestProxyFinder_CallsOnPACUpdate(t *testing.T) { + js := `function FindProxyForURL(url, host) { return "DIRECT" }` + server := httptest.NewServer(http.HandlerFunc(pacjsHandler(js))) + defer server.Close() + pw := NewPACWrapper(PACData{Port: 1}) + called := false + NewProxyFinder(server.URL, pw, false, func() { called = true }) + assert.True(t, called, "onPACUpdate should be called on successful PAC download") +} + +func TestProxyFinder_DoesNotCallOnPACUpdateWhenUnreachable(t *testing.T) { + pw := NewPACWrapper(PACData{Port: 1}) + called := false + NewProxyFinder("http://pacserver.invalid/nonexistent.pac", pw, false, func() { called = true }) + assert.False(t, called, "onPACUpdate should not be called when PAC server is unreachable") +} + func TestSkipBadProxies(t *testing.T) { js := `function FindProxyForURL(url, host) { return "PROXY primary:80; PROXY backup:80" }` server := httptest.NewServer(http.HandlerFunc(pacjsHandler(js))) defer server.Close() pw := NewPACWrapper(PACData{Port: 1}) - pf := NewProxyFinder(server.URL, pw, false) + pf := NewProxyFinder(server.URL, pw, false, nil) req := httptest.NewRequest(http.MethodGet, "https://www.test", nil) ctx := context.WithValue(req.Context(), contextKeyID, 0) req = req.WithContext(ctx) From 1c8a67dca791f815a5c770c65dc8605e4425e528 Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:33 +0800 Subject: [PATCH 10/17] Fix five code review findings: unlock before callback, lowercase scheme fixtures, tighten bare-probe assertions, assert cache repopulation --- proxy.go | 1 + proxy_test.go | 17 ++++++++++++----- proxyfinder.go | 11 +++++++---- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/proxy.go b/proxy.go index 366f965..0f8de4e 100644 --- a/proxy.go +++ b/proxy.go @@ -222,6 +222,7 @@ func splitChallengeNames(value string) []string { } type proxyAuthInfo struct { + // schemes are lower-cased, matching parseProxyAuthenticateSchemes output. schemes []string } diff --git a/proxy_test.go b/proxy_test.go index 077f639..7f9b41a 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -518,7 +518,7 @@ func TestConnectAuthCache_SkipsProbeOnCacheHit(t *testing.T) { require.NoError(t, err) auth := newAuthChain(newBasicAuthenticator("user:pass")) cache := &sync.Map{} - cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"Basic"}}) + cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) req := makeConnectReq(t) conn, err := connectViaProxy(req, proxyURL, auth, cache) require.NoError(t, err) @@ -536,14 +536,14 @@ func TestConnectAuthCache_EvictsOnStale407(t *testing.T) { require.NoError(t, err) auth := newAuthChain(newBasicAuthenticator("user:pass")) cache := &sync.Map{} - cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"Basic"}}) + cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) req := makeConnectReq(t) _, err = connectViaProxy(req, proxyURL, auth, cache) assert.Error(t, err, "should fail when the proxy rejects all auth attempts") _, stillCached := cache.Load(proxyURL.Host) assert.False(t, stillCached, "stale cache entry must be evicted after a persistent 407") bare, _ := mock.counts() - assert.GreaterOrEqual(t, bare, 1, "a bare probe must be attempted after cache eviction") + assert.Equal(t, 1, bare, "exactly one bare probe must be attempted after cache eviction") } func TestConnectAuthCache_EvictsOnStale407_ThenSucceeds(t *testing.T) { @@ -552,14 +552,21 @@ func TestConnectAuthCache_EvictsOnStale407_ThenSucceeds(t *testing.T) { require.NoError(t, err) auth := newAuthChain(newBasicAuthenticator("user:pass")) cache := &sync.Map{} - cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"Basic"}}) + cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) req := makeConnectReq(t) conn, err := connectViaProxy(req, proxyURL, auth, cache) require.NoError(t, err, "should succeed after stale eviction and cold probe") require.NotNil(t, conn, "connection must be non-nil on success") conn.Close() bare, _ := mock.counts() - assert.GreaterOrEqual(t, bare, 1, "cold probe must fire after eviction") + assert.Equal(t, 1, bare, "exactly one bare probe must fire after stale eviction") + val, repopulated := cache.Load(proxyURL.Host) + assert.True(t, repopulated, "cache must be repopulated after successful stale re-auth") + if repopulated { + info, ok := val.(proxyAuthInfo) + require.True(t, ok) + assert.NotEmpty(t, info.schemes, "repopulated cache entry must have schemes") + } } func TestConnectAuthCache_NoProbeOnSecondRequest(t *testing.T) { diff --git a/proxyfinder.go b/proxyfinder.go index 935a318..8e1785d 100644 --- a/proxyfinder.go +++ b/proxyfinder.go @@ -77,23 +77,26 @@ func (pf *ProxyFinder) WrapHandler(next http.Handler) http.Handler { func (pf *ProxyFinder) checkForUpdates() { pf.Lock() - defer pf.Unlock() pacjs := pf.fetcher.download() if pacjs == nil { if !pf.fetcher.isConnected() { pf.blocked = newBlocklist() pf.wrapper.Wrap(nil) } + pf.Unlock() return } pf.blocked = newBlocklist() + var notify func() if err := pf.runner.Update(pacjs); err != nil { log.Printf("Error running PAC JS: %q", err) } else { pf.wrapper.Wrap(pacjs) - if pf.onPACUpdate != nil { - pf.onPACUpdate() - } + notify = pf.onPACUpdate + } + pf.Unlock() + if notify != nil { + notify() } } From 23f54a53354f7e09ecfc96431d207b15233ada4d Mon Sep 17 00:00:00 2001 From: PC3 Date: Fri, 26 Jun 2026 17:04:34 +0800 Subject: [PATCH 11/17] Fix authCache.Store ordering and stale-once mock to pass EvictsOnStale407 tests --- proxy.go | 7 +++---- proxy_test.go | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/proxy.go b/proxy.go index 0f8de4e..b35fb8d 100644 --- a/proxy.go +++ b/proxy.go @@ -416,14 +416,13 @@ func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain, log.Printf("[%d] Got %q response, retrying with auth", id, resp2.Status) schemes := parseProxyAuthenticateSchemes(resp2.Header) _ = resp2.Body.Close() - // Store before attempting: the proxy advertised these schemes, so cache - // them now. If re-auth fails the next request skips the bare probe and - // evicts on a fresh 407 — same path as any stale entry. - authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) authResp2, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr2) if err != nil { return nil, err } + if authResp2.StatusCode != http.StatusProxyAuthRequired { + authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) + } log.Printf("[%d] Got %q response", id, authResp2.Status) resp = authResp2 } else { diff --git a/proxy_test.go b/proxy_test.go index 7f9b41a..3c602dc 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -450,9 +450,9 @@ func (m *authCacheMockProxy) ServeHTTP(w http.ResponseWriter, req *http.Request) } else { m.bareConnects++ } - isFirstBare := m.stale407Once && !hasAuth && m.bareConnects == 1 + isFirstAuthed := m.stale407Once && hasAuth && m.authedConnects == 1 m.mu.Unlock() - if m.respondWith407 || isFirstBare || !hasAuth { + if m.respondWith407 || isFirstAuthed || !hasAuth { w.Header().Set("Proxy-Authenticate", "Basic realm=\"proxy\"") w.WriteHeader(http.StatusProxyAuthRequired) return From 242fd2052434d98b1c697b15f5602ba3639094e8 Mon Sep 17 00:00:00 2001 From: PC3 Date: Wed, 1 Jul 2026 12:10:52 +0800 Subject: [PATCH 12/17] Update main.go Removed hand-rolled authCache anonymous function. Replaced with Clear() Co-authored-by: Sam Uong --- main.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/main.go b/main.go index 4f37e05..b1d19ff 100644 --- a/main.go +++ b/main.go @@ -208,12 +208,7 @@ func createServer(port int, pacurl string, auth *authChain, enableSocks bool) *h blockProxy(host) } }) - onPACUpdate := func() { - proxyHandler.authCache.Range(func(k, _ any) bool { - proxyHandler.authCache.Delete(k) - return true - }) - } + onPACUpdate := proxyHandler.authCache.Clear proxyFinder := NewProxyFinder(pacurl, pacWrapper, enableSocks, onPACUpdate) blockProxy = proxyFinder.blockProxy mux := http.NewServeMux() From 86d27ec7635e3395e03421151e4fed25f65942fa Mon Sep 17 00:00:00 2001 From: PC3 Date: Wed, 1 Jul 2026 16:06:57 +0800 Subject: [PATCH 13/17] Address code review feedback from samuong - Rename "Phase 1/Phase 2" branch labels to "Cache hit" / "Cold probe" - Extract coldProbe helper to eliminate duplication between stale-eviction and cold-probe paths; store to cache only on auth success - Convert connectViaProxy to a pointer-receiver method on ProxyHandler, removing auth and authCache parameters - Rewrite main.go construction comment to describe mutual dependency between proxyHandler and proxyFinder explicitly - Add comment to proxyfinder.go explaining why onPACUpdate is called outside the lock - Fix makeConnectReq to derive req.Host from req.URL.Host --- main.go | 5 +- multiauth_integration_test.go | 6 ++- proxy.go | 93 ++++++++++++++++------------------- proxy_test.go | 19 ++++--- proxyfinder.go | 2 + 5 files changed, 64 insertions(+), 61 deletions(-) diff --git a/main.go b/main.go index b1d19ff..d516ee0 100644 --- a/main.go +++ b/main.go @@ -200,8 +200,9 @@ func main() { func createServer(port int, pacurl string, auth *authChain, enableSocks bool) *http.Server { pacWrapper := NewPACWrapper(PACData{Port: port}) - // Construction order is load-bearing: authCache must exist before - // onPACUpdate is captured, and blockProxy before proxyFinder is used. + // Note that proxyHandler and proxyFinder are mutually dependent: + // proxyHandler calls proxyFinder.blockProxy(), and proxyFinder calls + // proxyHandler.authCache.Clear(). var blockProxy func(string) proxyHandler := NewProxyHandler(auth, getProxyFromContext, func(host string) { if blockProxy != nil { diff --git a/multiauth_integration_test.go b/multiauth_integration_test.go index ccc567d..83d662d 100644 --- a/multiauth_integration_test.go +++ b/multiauth_integration_test.go @@ -405,7 +405,8 @@ func TestConnectViaProxy_FallsThroughOn407(t *testing.T) { require.NoError(t, err) req.Host = "example.com:443" - conn, err := connectViaProxy(req, proxy.URL(), chain, &sync.Map{}) + ph := NewProxyHandler(chain, nil, func(string) {}) + conn, err := ph.connectViaProxy(req, proxy.URL()) require.NoError(t, err) defer conn.Close() //nolint:errcheck //nolint:errcheck @@ -433,7 +434,8 @@ func TestConnectViaProxy_RefusesBasicDowngrade(t *testing.T) { require.NoError(t, err) req.Host = "example.com:443" - _, err = connectViaProxy(req, proxy.URL(), chain, &sync.Map{}) + ph2 := NewProxyHandler(chain, nil, func(string) {}) + _, err = ph2.connectViaProxy(req, proxy.URL()) require.Error(t, err) assert.ErrorIs(t, err, errNoMatchingAuthMethod) } diff --git a/proxy.go b/proxy.go index b35fb8d..5d2f46d 100644 --- a/proxy.go +++ b/proxy.go @@ -277,7 +277,7 @@ func (ph ProxyHandler) handleConnect(w http.ResponseWriter, req *http.Request) { if proxyURL == nil { server, err = connectDirect(req) } else { - server, err = connectViaProxy(req, proxyURL, ph.auth, ph.authCache) + server, err = ph.connectViaProxy(req, proxyURL) var oe *net.OpError if errors.As(err, &oe) && oe.Op == "proxyconnect" { log.Printf("[%d] Temporarily blocking proxy: %q", id, proxyURL.Host) @@ -352,8 +352,40 @@ func connectDirect(req *http.Request) (net.Conn, error) { return server, err } -func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain, - authCache *sync.Map) (net.Conn, error) { +// coldProbe dials proxyURL on tr, sends an unauthenticated CONNECT, and handles +// the 407 response by caching the advertised schemes and retrying with auth. +// The caller is responsible for deferring tr.Close(). +func coldProbe(req *http.Request, proxyURL *url.URL, auth *authChain, + tr *transport, authCache *sync.Map) (*http.Response, error) { + id := req.Context().Value(contextKeyID) + if err := tr.dial(proxyURL); err != nil { + log.Printf("[%d] Error dialling proxy %s: %v", id, proxyURL.Host, err) + return nil, err + } + req.Header.Del("Proxy-Authorization") + resp, err := tr.RoundTrip(req) + if err != nil { + log.Printf("[%d] Error reading CONNECT response: %v", id, err) + return nil, err + } + if resp.StatusCode != http.StatusProxyAuthRequired || auth == nil { + return resp, nil + } + log.Printf("[%d] Got %q response, retrying with auth", id, resp.Status) + schemes := parseProxyAuthenticateSchemes(resp.Header) + _ = resp.Body.Close() + authResp, err := retryConnectWithAuth(req, proxyURL, auth, schemes, tr) + if err != nil { + return nil, err + } + if authResp.StatusCode != http.StatusProxyAuthRequired { + authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) + } + log.Printf("[%d] Got %q response", id, authResp.Status) + return authResp, nil +} + +func (ph *ProxyHandler) connectViaProxy(req *http.Request, proxyURL *url.URL) (net.Conn, error) { id := req.Context().Value(contextKeyID) // SOCKS5 short-circuit: SOCKS5 has its own authentication model @@ -388,75 +420,36 @@ func connectViaProxy(req *http.Request, proxyURL *url.URL, auth *authChain, activeTr := &tr // points to the transport holding the live tunnel var resp *http.Response - if cached, ok := authCache.Load(proxyURL.Host); ok && auth != nil { - // Phase 1: cache hit — skip unauthenticated probe. + if cached, ok := ph.authCache.Load(proxyURL.Host); ok && ph.auth != nil { + // Cache hit — skip unauthenticated probe. info := cached.(proxyAuthInfo) - authResp, err := retryConnectWithAuth(req, proxyURL, auth, info.schemes, &tr) + authResp, err := retryConnectWithAuth(req, proxyURL, ph.auth, info.schemes, &tr) if err != nil { return nil, err } if authResp.StatusCode == http.StatusProxyAuthRequired { // Stale cache entry — evict and fall through to cold probe. - authCache.Delete(proxyURL.Host) + ph.authCache.Delete(proxyURL.Host) _ = authResp.Body.Close() tr2 := transport{} defer tr2.Close() //nolint:errcheck activeTr = &tr2 - if err := tr2.dial(proxyURL); err != nil { - log.Printf("[%d] Error dialling proxy %s: %v", id, proxyURL.Host, err) - return nil, err - } - req.Header.Del("Proxy-Authorization") - resp2, err := tr2.RoundTrip(req) + var err error + resp, err = coldProbe(req, proxyURL, ph.auth, &tr2, ph.authCache) if err != nil { - log.Printf("[%d] Error reading CONNECT response: %v", id, err) return nil, err } - if resp2.StatusCode == http.StatusProxyAuthRequired && auth != nil { - log.Printf("[%d] Got %q response, retrying with auth", id, resp2.Status) - schemes := parseProxyAuthenticateSchemes(resp2.Header) - _ = resp2.Body.Close() - authResp2, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr2) - if err != nil { - return nil, err - } - if authResp2.StatusCode != http.StatusProxyAuthRequired { - authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) - } - log.Printf("[%d] Got %q response", id, authResp2.Status) - resp = authResp2 - } else { - resp = resp2 - } } else { log.Printf("[%d] Got %q response", id, authResp.Status) resp = authResp } } else { - // Phase 2: cold probe — no cached entry. - if err := tr.dial(proxyURL); err != nil { - log.Printf("[%d] Error dialling proxy %s: %v", id, proxyURL.Host, err) - return nil, err - } + // Cold probe — no cached entry; populate cache on first 407. var err error - resp, err = tr.RoundTrip(req) + resp, err = coldProbe(req, proxyURL, ph.auth, &tr, ph.authCache) if err != nil { - log.Printf("[%d] Error reading CONNECT response: %v", id, err) return nil, err } - if resp.StatusCode == http.StatusProxyAuthRequired && auth != nil { - log.Printf("[%d] Got %q response, retrying with auth", id, resp.Status) - schemes := parseProxyAuthenticateSchemes(resp.Header) - _ = resp.Body.Close() - authCache.Store(proxyURL.Host, proxyAuthInfo{schemes: schemes}) - // resp is now stale; the retry helper returns a fresh one. - authResp, err := retryConnectWithAuth(req, proxyURL, auth, schemes, &tr) - if err != nil { - return nil, err - } - log.Printf("[%d] Got %q response", id, authResp.Status) - resp = authResp - } } _ = resp.Body.Close() if resp.StatusCode == http.StatusProxyAuthRequired { diff --git a/proxy_test.go b/proxy_test.go index 3c602dc..4809de5 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -486,7 +486,7 @@ func makeConnectReq(t *testing.T) *http.Request { t.Helper() req, err := http.NewRequest(http.MethodConnect, "https://target.example.com:443", nil) require.NoError(t, err) - req.Host = "target.example.com:443" + req.Host = req.URL.Host return req } @@ -497,7 +497,8 @@ func TestConnectAuthCache_PopulatesOnFirst407(t *testing.T) { auth := newAuthChain(newBasicAuthenticator("user:pass")) cache := &sync.Map{} req := makeConnectReq(t) - conn, err := connectViaProxy(req, proxyURL, auth, cache) + ph := ProxyHandler{auth: auth, authCache: cache} + conn, err := ph.connectViaProxy(req, proxyURL) require.NoError(t, err) if conn != nil { conn.Close() @@ -520,7 +521,8 @@ func TestConnectAuthCache_SkipsProbeOnCacheHit(t *testing.T) { cache := &sync.Map{} cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) req := makeConnectReq(t) - conn, err := connectViaProxy(req, proxyURL, auth, cache) + ph := ProxyHandler{auth: auth, authCache: cache} + conn, err := ph.connectViaProxy(req, proxyURL) require.NoError(t, err) if conn != nil { conn.Close() @@ -538,7 +540,8 @@ func TestConnectAuthCache_EvictsOnStale407(t *testing.T) { cache := &sync.Map{} cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) req := makeConnectReq(t) - _, err = connectViaProxy(req, proxyURL, auth, cache) + ph := ProxyHandler{auth: auth, authCache: cache} + _, err = ph.connectViaProxy(req, proxyURL) assert.Error(t, err, "should fail when the proxy rejects all auth attempts") _, stillCached := cache.Load(proxyURL.Host) assert.False(t, stillCached, "stale cache entry must be evicted after a persistent 407") @@ -554,7 +557,8 @@ func TestConnectAuthCache_EvictsOnStale407_ThenSucceeds(t *testing.T) { cache := &sync.Map{} cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) req := makeConnectReq(t) - conn, err := connectViaProxy(req, proxyURL, auth, cache) + ph := ProxyHandler{auth: auth, authCache: cache} + conn, err := ph.connectViaProxy(req, proxyURL) require.NoError(t, err, "should succeed after stale eviction and cold probe") require.NotNil(t, conn, "connection must be non-nil on success") conn.Close() @@ -577,8 +581,9 @@ func TestConnectAuthCache_NoProbeOnSecondRequest(t *testing.T) { cache := &sync.Map{} // Pass 1: cold path — cache is empty, proxy will issue a 407 probe. + ph := ProxyHandler{auth: auth, authCache: cache} req1 := makeConnectReq(t) - conn1, err := connectViaProxy(req1, proxyURL, auth, cache) + conn1, err := ph.connectViaProxy(req1, proxyURL) require.NoError(t, err, "pass 1 must succeed") require.NotNil(t, conn1, "pass 1 connection must be non-nil") conn1.Close() @@ -590,7 +595,7 @@ func TestConnectAuthCache_NoProbeOnSecondRequest(t *testing.T) { // Pass 2: warm path — cache hit must suppress the bare probe entirely. req2 := makeConnectReq(t) - conn2, err := connectViaProxy(req2, proxyURL, auth, cache) + conn2, err := ph.connectViaProxy(req2, proxyURL) require.NoError(t, err, "pass 2 must succeed") require.NotNil(t, conn2, "pass 2 connection must be non-nil") conn2.Close() diff --git a/proxyfinder.go b/proxyfinder.go index 8e1785d..1bb6188 100644 --- a/proxyfinder.go +++ b/proxyfinder.go @@ -95,6 +95,8 @@ func (pf *ProxyFinder) checkForUpdates() { notify = pf.onPACUpdate } pf.Unlock() + // Call outside the lock: the callback flushes authCache and must not re-enter + // any ProxyFinder-locked path, but we avoid the risk of a future deadlock here. if notify != nil { notify() } From 6924817b090ea1a1d83a36c8e6df68c3427910b8 Mon Sep 17 00:00:00 2001 From: PC3 Date: Wed, 1 Jul 2026 16:56:58 +0800 Subject: [PATCH 14/17] Use defer to unlock mutex and call notify in checkForUpdates --- proxyfinder.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/proxyfinder.go b/proxyfinder.go index 1bb6188..5b07b90 100644 --- a/proxyfinder.go +++ b/proxyfinder.go @@ -76,30 +76,25 @@ func (pf *ProxyFinder) WrapHandler(next http.Handler) http.Handler { } func (pf *ProxyFinder) checkForUpdates() { + notify := func() {} + defer func() { notify() }() pf.Lock() + defer pf.Unlock() pacjs := pf.fetcher.download() if pacjs == nil { if !pf.fetcher.isConnected() { pf.blocked = newBlocklist() pf.wrapper.Wrap(nil) } - pf.Unlock() return } pf.blocked = newBlocklist() - var notify func() if err := pf.runner.Update(pacjs); err != nil { log.Printf("Error running PAC JS: %q", err) } else { pf.wrapper.Wrap(pacjs) notify = pf.onPACUpdate } - pf.Unlock() - // Call outside the lock: the callback flushes authCache and must not re-enter - // any ProxyFinder-locked path, but we avoid the risk of a future deadlock here. - if notify != nil { - notify() - } } func (pf *ProxyFinder) findProxyForRequest(req *http.Request) (*url.URL, error) { From 008e35fced26a66c0f3e26b78177252d255d960d Mon Sep 17 00:00:00 2001 From: PC3 Date: Wed, 1 Jul 2026 17:04:48 +0800 Subject: [PATCH 15/17] Guard onPACUpdate nil check before assigning to notify --- proxyfinder.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/proxyfinder.go b/proxyfinder.go index 5b07b90..f61c9aa 100644 --- a/proxyfinder.go +++ b/proxyfinder.go @@ -93,7 +93,9 @@ func (pf *ProxyFinder) checkForUpdates() { log.Printf("Error running PAC JS: %q", err) } else { pf.wrapper.Wrap(pacjs) - notify = pf.onPACUpdate + if pf.onPACUpdate != nil { + notify = pf.onPACUpdate + } } } From 0eb260ba88cdacab8b101b2e7b178c71a200bc4f Mon Sep 17 00:00:00 2001 From: PC3 Date: Wed, 8 Jul 2026 11:51:52 +0800 Subject: [PATCH 16/17] Remove stale-cache self-heal path from connectViaProxy After retryConnectWithAuth returns a 407, evict the cache entry and return an error rather than falling back to a cold probe. The self-heal was intended to handle proxy auth scheme changes mid-session, but that scenario is already covered by the PAC flush wired in proxyfinder.go (onPACUpdate clears the cache on network change). The common 407-after- auth failure is bad credentials, which a cold probe retry cannot fix. Remove TestConnectAuthCache_EvictsOnStale407_ThenSucceeds and its newStaleOnceTestProxy helper. Update TestConnectAuthCache_EvictsOnStale407 to assert an error is returned rather than a bare probe being fired. --- proxy.go | 16 +++------------- proxy_test.go | 42 ++++-------------------------------------- 2 files changed, 7 insertions(+), 51 deletions(-) diff --git a/proxy.go b/proxy.go index 5d2f46d..90fb8e5 100644 --- a/proxy.go +++ b/proxy.go @@ -428,21 +428,11 @@ func (ph *ProxyHandler) connectViaProxy(req *http.Request, proxyURL *url.URL) (n return nil, err } if authResp.StatusCode == http.StatusProxyAuthRequired { - // Stale cache entry — evict and fall through to cold probe. + // Stale or invalid cache entry — evict and return error. ph.authCache.Delete(proxyURL.Host) - _ = authResp.Body.Close() - tr2 := transport{} - defer tr2.Close() //nolint:errcheck - activeTr = &tr2 - var err error - resp, err = coldProbe(req, proxyURL, ph.auth, &tr2, ph.authCache) - if err != nil { - return nil, err - } - } else { - log.Printf("[%d] Got %q response", id, authResp.Status) - resp = authResp } + log.Printf("[%d] Got %q response", id, authResp.Status) + resp = authResp } else { // Cold probe — no cached entry; populate cache on first 407. var err error diff --git a/proxy_test.go b/proxy_test.go index 4809de5..7fe7c1f 100644 --- a/proxy_test.go +++ b/proxy_test.go @@ -435,7 +435,6 @@ type authCacheMockProxy struct { bareConnects int authedConnects int respondWith407 bool - stale407Once bool } func (m *authCacheMockProxy) ServeHTTP(w http.ResponseWriter, req *http.Request) { @@ -450,9 +449,8 @@ func (m *authCacheMockProxy) ServeHTTP(w http.ResponseWriter, req *http.Request) } else { m.bareConnects++ } - isFirstAuthed := m.stale407Once && hasAuth && m.authedConnects == 1 m.mu.Unlock() - if m.respondWith407 || isFirstAuthed || !hasAuth { + if m.respondWith407 || !hasAuth { w.Header().Set("Proxy-Authenticate", "Basic realm=\"proxy\"") w.WriteHeader(http.StatusProxyAuthRequired) return @@ -474,13 +472,6 @@ func newAuthCacheTestProxy(t *testing.T, respondWith407 bool) (*httptest.Server, return srv, mock } -func newStaleOnceTestProxy(t *testing.T) (*httptest.Server, *authCacheMockProxy) { - t.Helper() - mock := &authCacheMockProxy{stale407Once: true} - srv := httptest.NewServer(mock) - t.Cleanup(srv.Close) - return srv, mock -} func makeConnectReq(t *testing.T) *http.Request { t.Helper() @@ -533,7 +524,7 @@ func TestConnectAuthCache_SkipsProbeOnCacheHit(t *testing.T) { } func TestConnectAuthCache_EvictsOnStale407(t *testing.T) { - proxySrv, mock := newAuthCacheTestProxy(t, true) + proxySrv, _ := newAuthCacheTestProxy(t, true) proxyURL, err := url.Parse(proxySrv.URL) require.NoError(t, err) auth := newAuthChain(newBasicAuthenticator("user:pass")) @@ -542,36 +533,11 @@ func TestConnectAuthCache_EvictsOnStale407(t *testing.T) { req := makeConnectReq(t) ph := ProxyHandler{auth: auth, authCache: cache} _, err = ph.connectViaProxy(req, proxyURL) - assert.Error(t, err, "should fail when the proxy rejects all auth attempts") + assert.Error(t, err, "stale 407 must return an error — no self-heal") _, stillCached := cache.Load(proxyURL.Host) - assert.False(t, stillCached, "stale cache entry must be evicted after a persistent 407") - bare, _ := mock.counts() - assert.Equal(t, 1, bare, "exactly one bare probe must be attempted after cache eviction") + assert.False(t, stillCached, "stale cache entry must be evicted on 407") } -func TestConnectAuthCache_EvictsOnStale407_ThenSucceeds(t *testing.T) { - proxySrv, mock := newStaleOnceTestProxy(t) - proxyURL, err := url.Parse(proxySrv.URL) - require.NoError(t, err) - auth := newAuthChain(newBasicAuthenticator("user:pass")) - cache := &sync.Map{} - cache.Store(proxyURL.Host, proxyAuthInfo{schemes: []string{"basic"}}) - req := makeConnectReq(t) - ph := ProxyHandler{auth: auth, authCache: cache} - conn, err := ph.connectViaProxy(req, proxyURL) - require.NoError(t, err, "should succeed after stale eviction and cold probe") - require.NotNil(t, conn, "connection must be non-nil on success") - conn.Close() - bare, _ := mock.counts() - assert.Equal(t, 1, bare, "exactly one bare probe must fire after stale eviction") - val, repopulated := cache.Load(proxyURL.Host) - assert.True(t, repopulated, "cache must be repopulated after successful stale re-auth") - if repopulated { - info, ok := val.(proxyAuthInfo) - require.True(t, ok) - assert.NotEmpty(t, info.schemes, "repopulated cache entry must have schemes") - } -} func TestConnectAuthCache_NoProbeOnSecondRequest(t *testing.T) { proxySrv, mock := newAuthCacheTestProxy(t, false) From be9a3d7864b5cc045ac44faa0b058f61f415f1dd Mon Sep 17 00:00:00 2001 From: PC3 Date: Thu, 23 Jul 2026 14:32:44 +0800 Subject: [PATCH 17/17] Remove redundant pointer. --- proxy.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/proxy.go b/proxy.go index 90fb8e5..2cd0f3a 100644 --- a/proxy.go +++ b/proxy.go @@ -417,8 +417,6 @@ func (ph *ProxyHandler) connectViaProxy(req *http.Request, proxyURL *url.URL) (n var tr transport defer tr.Close() //nolint:errcheck - activeTr := &tr // points to the transport holding the live tunnel - var resp *http.Response if cached, ok := ph.authCache.Load(proxyURL.Host); ok && ph.auth != nil { // Cache hit — skip unauthenticated probe. @@ -449,7 +447,7 @@ func (ph *ProxyHandler) connectViaProxy(req *http.Request, proxyURL *url.URL) (n if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("[%d] Unexpected response status: %s", id, resp.Status) } - return activeTr.hijack(), nil + return tr.hijack(), nil } // retryConnectWithAuth iterates the configured auth chain over a CONNECT