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

diff --git a/src/pkg/net/http/proxy_test.go b/src/pkg/net/http/proxy_test.go
index 86db976b838f583c1d2c56b399a43f11cd190519..449ccaeea760a4cb7266028258f65b8ca725b18e 100644 (file)
--- 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 d0505bf13f041918a2ebcb727ba54564aa3b27b3..98e198e78af71bf4983c770a88c8ec10bd5cc20f 100644 (file)
--- 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 c37ef13a4163bb7ad8b26117ec33e977eb889df5..3cb8263994eb4f3951415a77a8509fbb9b72e5be 100644 (file)
--- 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>", 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: "<nil>"},
+       {noenv: "example.com", req: "http://example.com/", env: "proxy", want: "<nil>"},
+       {noenv: ".example.com", req: "http://example.com/", env: "proxy", want: "<nil>"},
+       {noenv: "ample.com", req: "http://example.com/", env: "proxy", want: "http://proxy"},
+       {noenv: "example.com", req: "http://foo.example.com/", env: "proxy", want: "<nil>"},
+       {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)
                }
        }
 }