From 82ae6434b3a95beb64f708078af5a525088d6ccc Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sat, 22 Dec 2012 17:41:00 -0800 Subject: [PATCH] net/http: match curl and others' NO_PROXY wildcard handling NO_PROXY="example.com" should match "foo.example.com", just the same as NO_PROXY=".example.com". This is what curl and Python do. Fixes #4574 R=golang-dev, rsc CC=golang-dev https://golang.org/cl/7005049 --- src/pkg/net/http/proxy_test.go | 2 +- src/pkg/net/http/transport.go | 10 ++++- src/pkg/net/http/transport_test.go | 60 ++++++++++++++++++++++-------- 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/src/pkg/net/http/proxy_test.go b/src/pkg/net/http/proxy_test.go index 86db976b83..449ccaeea7 100644 --- a/src/pkg/net/http/proxy_test.go +++ b/src/pkg/net/http/proxy_test.go @@ -31,7 +31,7 @@ var UseProxyTests = []struct { {"localhost.net", true}, // not match as suffix of address {"local.localhost", true}, // not match as prefix as address {"barbarbaz.net", true}, // not match because NO_PROXY have a '.' - {"www.foobar.com", true}, // not match because NO_PROXY is not .foobar.com + {"www.foobar.com", false}, // match because NO_PROXY includes "foobar.com" } func TestUseProxy(t *testing.T) { diff --git a/src/pkg/net/http/transport.go b/src/pkg/net/http/transport.go index d0505bf13f..98e198e78a 100644 --- a/src/pkg/net/http/transport.go +++ b/src/pkg/net/http/transport.go @@ -450,7 +450,15 @@ func useProxy(addr string) bool { if hasPort(p) { p = p[:strings.LastIndex(p, ":")] } - if addr == p || (p[0] == '.' && (strings.HasSuffix(addr, p) || addr == p[1:])) { + if addr == p { + return false + } + if p[0] == '.' && (strings.HasSuffix(addr, p) || addr == p[1:]) { + // no_proxy ".foo.com" matches "bar.foo.com" or "foo.com" + return false + } + if p[0] != '.' && strings.HasSuffix(addr, p) && addr[len(addr)-len(p)-1] == '.' { + // no_proxy "foo.com" matches "bar.foo.com" return false } } diff --git a/src/pkg/net/http/transport_test.go b/src/pkg/net/http/transport_test.go index c37ef13a41..3cb8263994 100644 --- a/src/pkg/net/http/transport_test.go +++ b/src/pkg/net/http/transport_test.go @@ -1115,18 +1115,43 @@ func TestTransportNoHost(t *testing.T) { } } -var proxyFromEnvTests = []struct { +type proxyFromEnvTest struct { + req string // URL to fetch; blank means "http://example.com" env string - wanturl string + noenv string + want string wanterr error -}{ - {"127.0.0.1:8080", "http://127.0.0.1:8080", nil}, - {"cache.corp.example.com:1234", "http://cache.corp.example.com:1234", nil}, - {"cache.corp.example.com", "http://cache.corp.example.com", nil}, - {"https://cache.corp.example.com", "https://cache.corp.example.com", nil}, - {"http://127.0.0.1:8080", "http://127.0.0.1:8080", nil}, - {"https://127.0.0.1:8080", "https://127.0.0.1:8080", nil}, - {"", "", nil}, +} + +func (t proxyFromEnvTest) String() string { + var buf bytes.Buffer + if t.env != "" { + fmt.Fprintf(&buf, "http_proxy=%q", t.env) + } + if t.noenv != "" { + fmt.Fprintf(&buf, " no_proxy=%q", t.noenv) + } + req := "http://example.com" + if t.req != "" { + req = t.req + } + fmt.Fprintf(&buf, " req=%q", req) + return strings.TrimSpace(buf.String()) +} + +var proxyFromEnvTests = []proxyFromEnvTest{ + {env: "127.0.0.1:8080", want: "http://127.0.0.1:8080"}, + {env: "cache.corp.example.com:1234", want: "http://cache.corp.example.com:1234"}, + {env: "cache.corp.example.com", want: "http://cache.corp.example.com"}, + {env: "https://cache.corp.example.com", want: "https://cache.corp.example.com"}, + {env: "http://127.0.0.1:8080", want: "http://127.0.0.1:8080"}, + {env: "https://127.0.0.1:8080", want: "https://127.0.0.1:8080"}, + {want: ""}, + {noenv: "example.com", req: "http://example.com/", env: "proxy", want: ""}, + {noenv: ".example.com", req: "http://example.com/", env: "proxy", want: ""}, + {noenv: "ample.com", req: "http://example.com/", env: "proxy", want: "http://proxy"}, + {noenv: "example.com", req: "http://foo.example.com/", env: "proxy", want: ""}, + {noenv: ".foo.com", req: "http://example.com/", env: "proxy", want: "http://proxy"}, } func TestProxyFromEnvironment(t *testing.T) { @@ -1134,16 +1159,21 @@ func TestProxyFromEnvironment(t *testing.T) { os.Setenv("http_proxy", "") os.Setenv("NO_PROXY", "") os.Setenv("no_proxy", "") - for i, tt := range proxyFromEnvTests { + for _, tt := range proxyFromEnvTests { os.Setenv("HTTP_PROXY", tt.env) - req, _ := NewRequest("GET", "http://example.com", nil) + os.Setenv("NO_PROXY", tt.noenv) + reqURL := tt.req + if reqURL == "" { + reqURL = "http://example.com" + } + req, _ := NewRequest("GET", reqURL, nil) url, err := ProxyFromEnvironment(req) if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e { - t.Errorf("%d. got error = %q, want %q", i, g, e) + t.Errorf("%v: got error = %q, want %q", tt, g, e) continue } - if got := fmt.Sprintf("%s", url); got != tt.wanturl { - t.Errorf("%d. got URL = %q, want %q", i, url, tt.wanturl) + if got := fmt.Sprintf("%s", url); got != tt.want { + t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want) } } } -- 2.48.1