From 1036f51a56458a1fa44d4657a214d0fc9e88a8b0 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 3 Nov 2020 08:53:01 -0800 Subject: [PATCH] net/tshttpproxy: aggressively rate-limit error logs in Transport.Proxy path Otherwise log upload HTTP requests generate proxy errrors which generate logs which generate HTTP requests which generate proxy errors which generate more logs, etc. Fixes #879 --- net/tshttpproxy/tshttpproxy_windows.go | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/net/tshttpproxy/tshttpproxy_windows.go b/net/tshttpproxy/tshttpproxy_windows.go index 90f2b3f95..bd85d2104 100644 --- a/net/tshttpproxy/tshttpproxy_windows.go +++ b/net/tshttpproxy/tshttpproxy_windows.go @@ -19,6 +19,7 @@ "github.com/alexbrainman/sspi/negotiate" "golang.org/x/sys/windows" + "tailscale.com/types/logger" ) var ( @@ -38,6 +39,13 @@ func init() { val *url.URL } +// proxyErrorf is a rate-limited logger specifically for errors asking +// WinHTTP for the proxy information. We don't want to log about +// errors often, otherwise the log message itself will generate a new +// HTTP request which ultimately will call back into us to log again, +// forever. So for errors, we only log a bit. +var proxyErrorf = logger.RateLimitedFn(log.Printf, 10*time.Minute, 2 /* burst*/, 10 /* maxCache */) + func proxyFromWinHTTPOrCache(req *http.Request) (*url.URL, error) { if req.URL == nil { return nil, nil @@ -79,7 +87,14 @@ type result struct { setNoProxyUntil(10 * time.Second) return nil, nil } - log.Printf("tshttpproxy: winhttp: GetProxyForURL(%q): %v/%#v", urlStr, err, err) + if err == windows.ERROR_INVALID_PARAMETER { + // Seen on Windows 8.1. (https://github.com/tailscale/tailscale/issues/879) + // TODO(bradfitz): figure this out. + setNoProxyUntil(time.Hour) + proxyErrorf("tshttpproxy: winhttp: GetProxyForURL(%q): ERROR_INVALID_PARAMETER [unexpected]", urlStr) + return nil, nil + } + proxyErrorf("tshttpproxy: winhttp: GetProxyForURL(%q): %v/%#v", urlStr, err, err) if err == syscall.Errno(ERROR_WINHTTP_UNABLE_TO_DOWNLOAD_SCRIPT) { setNoProxyUntil(10 * time.Second) return nil, nil @@ -88,7 +103,7 @@ type result struct { case <-ctx.Done(): cachedProxy.Lock() defer cachedProxy.Unlock() - log.Printf("tshttpproxy: winhttp: GetProxyForURL(%q): timeout; using cached proxy %v", urlStr, cachedProxy.val) + proxyErrorf("tshttpproxy: winhttp: GetProxyForURL(%q): timeout; using cached proxy %v", urlStr, cachedProxy.val) return cachedProxy.val, nil } } @@ -96,7 +111,7 @@ type result struct { func proxyFromWinHTTP(ctx context.Context, urlStr string) (proxy *url.URL, err error) { whi, err := winHTTPOpen() if err != nil { - log.Printf("winhttp: Open: %v", err) + proxyErrorf("winhttp: Open: %v", err) return nil, err } defer whi.Close()