[go] net/http: add ProxyConfig
roger peppe (Gerrit)
roger peppe has uploaded this change for review.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
3 files changed, 107 insertions(+), 29 deletions(-)
diff --git a/src/net/http/proxy_test.go b/src/net/http/proxy_test.go
index f59a551..0947582 100644
--- a/src/net/http/proxy_test.go
+++ b/src/net/http/proxy_test.go
@@ -35,10 +35,11 @@
}
func TestUseProxy(t *testing.T) {
- ResetProxyEnv()
- os.Setenv("NO_PROXY", "foobar.com, .barbaz.net")
+ cfg := ProxyConfig{
+ NoProxy: "foobar.com, .barbaz.net",
+ }
for _, test := range UseProxyTests {
- if useProxy(test.host+":80") != test.match {
+ if cfg.useProxy(test.host+":80") != test.match {
t.Errorf("useProxy(%v) = %v, want %v", test.host, !test.match, test.match)
}
}
@@ -81,7 +82,8 @@
}
func TestInvalidNoProxy(t *testing.T) {
- ResetProxyEnv()
- os.Setenv("NO_PROXY", ":1")
- useProxy("example.com:80") // should not panic
+ cfg := ProxyConfig{
+ NoProxy: ":1",
+ }
+ cfg.useProxy("example.com:80") // should not panic
}
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 034d016..d6a8ccb 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -247,37 +247,32 @@
}
}
-// ProxyFromEnvironment returns the URL of the proxy to use for a
-// given request, as indicated by the environment variables
-// HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the lowercase versions
-// thereof). HTTPS_PROXY takes precedence over HTTP_PROXY for https
-// requests.
-//
-// The environment values may be either a complete URL or a
-// "host[:port]", in which case the "http" scheme is assumed.
-// An error is returned if the value is a different form.
-//
-// A nil URL and nil error are returned if no proxy is defined in the
-// environment, or a proxy should not be used for the given request,
-// as defined by NO_PROXY.
-//
-// As a special case, if req.URL.Host is "localhost" (with or without
-// a port number), then a nil URL and nil error will be returned.
-func ProxyFromEnvironment(req *Request) (*url.URL, error) {
+// ProxyConfig holds configuration for HTTP proxy settings.
+// See ProxyFromEnvironment for details.
+type ProxyConfig struct {
+ RequestMethod string
+ HTTPProxy string
+ HTTPSProxy string
+ NoProxy string
+}
+
+// ProxyForRequest determines the URL to use for the given request.
+// This method may be used as a value for Transport.Proxy.
+func (cfg *ProxyConfig) ProxyForRequest(req *Request) (*url.URL, error) {
var proxy string
if req.URL.Scheme == "https" {
- proxy = httpsProxyEnv.Get()
+ proxy = cfg.HTTPSProxy
}
if proxy == "" {
- proxy = httpProxyEnv.Get()
- if proxy != "" && os.Getenv("REQUEST_METHOD") != "" {
+ proxy = cfg.HTTPProxy
+ if proxy != "" && cfg.RequestMethod != "" {
return nil, errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
}
}
if proxy == "" {
return nil, nil
}
- if !useProxy(canonicalAddr(req.URL)) {
+ if !cfg.useProxy(canonicalAddr(req.URL)) {
return nil, nil
}
proxyURL, err := url.Parse(proxy)
@@ -299,6 +294,46 @@
return proxyURL, nil
}
+// ProxyConfigFromEnvironment returns a ProxyConfig
+// instance populated from environment variables.
+// ProxyFromEnvironment is equivalent to ProxyConfigFromEnvironment().Proxy
+// except that it only reads the environment variables
+// other than REQUEST_METHOD once, the first time it is called.
+func ProxyConfigFromEnvironment() *ProxyConfig {
+ return &ProxyConfig{
+ RequestMethod: os.Getenv("REQUEST_METHOD"),
+ HTTPProxy: os.Getenv("HTTP_PROXY"),
+ HTTPSProxy: os.Getenv("HTTPS_PROXY"),
+ NoProxy: os.Getenv("NO_PROXY"),
+ }
+}
+
+// ProxyFromEnvironment returns the URL of the proxy to use for a
+// given request, as indicated by the environment variables
+// HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the lowercase versions
+// thereof). HTTPS_PROXY takes precedence over HTTP_PROXY for https
+// requests.
+//
+// The environment values may be either a complete URL or a
+// "host[:port]", in which case the "http" scheme is assumed.
+// An error is returned if the value is a different form.
+//
+// A nil URL and nil error are returned if no proxy is defined in the
+// environment, or a proxy should not be used for the given request,
+// as defined by NO_PROXY.
+//
+// As a special case, if req.URL.Host is "localhost" (with or without
+// a port number), then a nil URL and nil error will be returned.
+func ProxyFromEnvironment(req *Request) (*url.URL, error) {
+ cfg := ProxyConfig{
+ HTTPSProxy: httpsProxyEnv.Get(),
+ HTTPProxy: httpProxyEnv.Get(),
+ RequestMethod: os.Getenv("REQUEST_METHOD"),
+ NoProxy: noProxyEnv.Get(),
+ }
+ return cfg.ProxyForRequest(req)
+}
+
// ProxyURL returns a proxy function (for use in a Transport)
// that always returns the same URL.
func ProxyURL(fixedURL *url.URL) func(*Request) (*url.URL, error) {
@@ -1212,7 +1247,7 @@
// useProxy reports whether requests to addr should use a proxy,
// according to the NO_PROXY or no_proxy environment variable.
// addr is always a canonicalAddr with a host and port.
-func useProxy(addr string) bool {
+func (cfg *ProxyConfig) useProxy(addr string) bool {
if len(addr) == 0 {
return true
}
@@ -1229,7 +1264,7 @@
}
}
- noProxy := noProxyEnv.Get()
+ noProxy := cfg.NoProxy
if noProxy == "*" {
return false
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 39b5cd3..fab58d9 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -2294,6 +2294,47 @@
}
}
+func TestProxyConfig(t *testing.T) {
+ for _, tt := range proxyFromEnvTests {
+ cfg := ProxyConfig{
+ HTTPProxy: tt.env,
+ HTTPSProxy: tt.httpsenv,
+ NoProxy: tt.noenv,
+ RequestMethod: tt.reqmeth,
+ }
+ reqURL := tt.req
+ if reqURL == "" {
+ reqURL = "http://example.com"
+ }
+ req, _ := NewRequest("GET", reqURL, nil)
+ url, err := cfg.ProxyForRequest(req)
+ if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e {
+ t.Errorf("%v: got error = %q, want %q", tt, g, e)
+ continue
+ }
+ if got := fmt.Sprintf("%s", url); got != tt.want {
+ t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
+ }
+ }
+}
+
+func TestProxyConfigFromEnvironment(t *testing.T) {
+ os.Setenv("HTTP_PROXY", "httpproxy")
+ os.Setenv("HTTPS_PROXY", "httpsproxy")
+ os.Setenv("NO_PROXY", "noproxy")
+ os.Setenv("REQUEST_METHOD", "requestmethod")
+ got := ProxyConfigFromEnvironment()
+ want := ProxyConfig{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ RequestMethod: "requestmethod",
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
+ }
+}
+
func TestIdleConnChannelLeak(t *testing.T) {
// Not parallel: uses global test hooks.
var mu sync.Mutex
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
3 comments:
Fixes #22079
// except that it only reads the environment variables
// other than REQUEST_METHOD once, the first time it is called.
I'm about to post an update on the bug -- see there.
Patch Set #1, Line 307:
NoProxy: os.Getenv("NO_PROXY"),ProxyFromEnvironment supports lower and upper case versions of these environment variables, while ProxyConfigFromEnvironment supports upper case only. This is surprising -- they should support the same set of variables.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #2 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
3 files changed, 171 insertions(+), 56 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
File src/net/http/transport.go:
Patch Set #1, Line 307:
NoProxy: os.Getenv("NO_PROXY"),ProxyFromEnvironment supports lower and upper case versions of these environment variables, while Pr […]
Done, and added some more tests too.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #3 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
3 files changed, 183 insertions(+), 54 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
2 comments:
File src/net/http/transport.go:
// Note that if fields in ProxyConfig are updated while a server
// is using it, the update must be protected with a mutex.
// For example:
//
// transport.Proxy = func(req *http.Request) (*url.URL, error) {
// myType.mu.Lock()
// defer myType.mu.Unlock()
// return myType.proxyConfig.ProxyForRequest(req)
// }Sorry for the back-and-forth. I don't think this comment is necessary. My confusion stemmed from the choice of getEnvAny vs envOnce, below.
Patch Set #3, Line 321:
getEnvAny(noProxyVars)Why is this getEnvAny(noProxyVars) instead of noProxyEnv.Get()? This is the source of my confusion. It looks like we are expecting code like the following:
config1 := ProxyConfigFromEnvironment()
...
// much later
os.Setenv("HTTP_PROXY", newValue)
config2 := ProxyConfigFromEnvironment()I am not sure we want to support that use case because it's difficult to reason about concurrent updates to environment variables. e.g., there could be other code in the program that uses ProxyFromEnvironment() and runs concurrently with the above code. I would instead write the above code as follows (with locking elided):
config := ProxyConfigFromEnvironment()
...
// much later
config.HTTPProxy = newValue
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
2 comments:
// Note that if fields in ProxyConfig are updated while a server
// is using it, the update must be protected with a mutex.
// For example:
//
// transport.Proxy = func(req *http.Request) (*url.URL, error) {
// myType.mu.Lock()
// defer myType.mu.Unlock()
// return myType.proxyConfig.ProxyForRequest(req)
// }
Sorry for the back-and-forth. I don't think this comment is necessary. […]
I thought that perhaps it was useful anyway, as it would be a very easy mistake to make. I'm happy to remove it though.
Patch Set #3, Line 321:
getEnvAny(noProxyVars)
Why is this getEnvAny(noProxyVars) instead of noProxyEnv.Get()? This is the source of my confusion. […]
I think that it's more straightforward to understand if ProxyConfigFromEnvironment always reads from the environment rather than cached values.
I was a little surprised when I discovered that ProxyFromEnvironment used cached values (but I can understand it, given that looking up a value in the environment has a cost, and otherwise it would need to do that work on every request); however I don't see that ProxyConfigFromEnvironment needs the same optimization, as its costs are clear and calling its ProxyForRequest method doesn't incur them again.
It's not uncommon to want to use something like this in tests, and having it depend on hidden (and forever-unchangeable) static variables makes it harder to use and less versatile, I think.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
1 comment:
File src/net/http/transport.go:
Patch Set #3, Line 251:
// See ProxyFromEnvironment for details.Each field needs to be documented if this change goes in.
(I haven't reviewed anything else in this CL.)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
1 comment:
File src/net/http/transport.go:
Patch Set #3, Line 321:
getEnvAny(noProxyVars)I think that it's more straightforward to understand if ProxyConfigFromEnvironment always reads from […]
Now that there's a ProxyConfig struct, couldn't a test set the ProxyConfig fields however it wants?
It's also confusing for ProxyFromEnvironment() and ProxyConfigFromEnvironment() to behave differently.
I didn't write the original code to cache os.Getenv lookups in ProxyFromEnvironment so I can't say why it was done. (I also don't understand why the REQUEST_METHOD lookup is not cached.) There may have been performance concerns. However, I come at this from a different angle: I generally consider "mutable environment variables" an anti-pattern, especially in multi-goroutine programs where it's not trivial to atomically read or write a set of environment variables.
@bradfitz, any thoughts?
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #4 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
This changes ProxyFromEnvironment such that it uses a
cached copy of the REQUEST_METHOD environment variable,
which should be safe, as the logic involving that method is
only there to mitigate a security issue that is not present if that
environment variable is set in the code.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
4 files changed, 213 insertions(+), 97 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
I've uploaded another version that makes things a little simpler, based on the comment below.
1 comment:
Patch Set #3, Line 321:
getEnvAny(noProxyVars)
Now that there's a ProxyConfig struct, couldn't a test set the ProxyConfig fields however it wants? […]
The kind of test I'm thinking of is one that tries to test as much of "main" as possible, including the reading of the proxy values from environment variables. I've seen many tests that set up the expected environment variables before invoking the code that's meant to read them.
I agree that mutable environment variables in production code are an anti pattern, but I'd also consider immutable copies of external state to be an anti-pattern too, because it means that it's hard to test the code that uses them (it's no coincidence that net/http has a special "ResetCachedEnvironment" method to avoid the behaviour that ProxyForRequest uses). Another example of this would be the global system CA roots pool used by crypto/x509.
As for ProxyFromEnvironment being different from ProxyConfigFromEnvironment, I *think* it's reasonable that
transport.Proxy = http.ProxyFromEnvironment
is identical to
transport.Proxy = ProxyConfigFromEnvironment().Proxy
with the somewhat odd exception of REQUEST_METHOD which would be cached by the latter form but isn't by the first.
I think the fact that REQUEST_METHOD is not currently cached is an oversight - I think we could change that without breaking things. It's only there to guard against a possible security issue that isn't a problem when running tests. Then we could have a single Once instance that initializes the global ProxyConfig used by ProxyFromEnvironment, which makes things simpler.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #5 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
This changes ProxyFromEnvironment such that it uses a
cached copy of the REQUEST_METHOD environment variable,
which should be safe, as the logic involving that method is
only there to mitigate a security issue that is not present if that
environment variable is set in the code.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
4 files changed, 211 insertions(+), 97 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
File src/net/http/transport.go:
Patch Set #3, Line 251:
// See ProxyFromEnvironment for details.Each field needs to be documented if this change goes in. […]
Done
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
1 comment:
File src/net/http/transport.go:
Patch Set #3, Line 321:
onment except that Pro
The kind of test I'm thinking of is one that tries to test as much of "main" as possible, including the reading of the proxy values from environment variables. I've seen many tests that set up the expected environment variables before invoking the code that's meant to read them.
I've seen this kind of test work by running the test binary multiple times with a different environment on each execution. Can this setup work for your test? It is possible (if a bit tedious) to do this with exec.Command from TestMain.
I think the fact that REQUEST_METHOD is not currently cached is an oversight - I think we could change that without breaking things.
Agreed.
Then we could have a single Once instance that initializes the global ProxyConfig used by ProxyFromEnvironment, which makes things simpler.
Agreed, this would be nice.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
File src/net/http/transport.go:
Patch Set #3, Line 321:
getEnvAny(noProxyVars)
Agreed, this would be nice.
FWIW, this has been done now.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
Looks good with nits.
4 comments:
File src/net/http/transport.go:
Patch Set #3, Line 321:
onment except that Pro
I think that it's more straightforward to understand if ProxyConfigFromEnvironment always reads from the environment rather than cached values.
I thought about this some more and decided this is OK with me.
File src/net/http/transport.go:
Patch Set #5, Line 324:
func ProxyConfigFromEnvironment() *ProxyConfig {Naming nit: How about NewProxyConfigFromEnvironment, to emphasize that this re-reads the environment on each call, and also to visually distinguish from ProxyFromEnvironment?
Patch Set #5, Line 637:
proxyConfignit: defaultProxyConfig?
nit: as before, // reset is used by tests
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
Patch Set 5:
(4 comments)
Looks good with nits.
Roger, ping. Still a chance to get this in for Go 1.10.
Brad Fitzpatrick (Gerrit)
4 comments:
File src/net/http/proxy_test.go:
Patch Set #5, Line 38:
cfg := ProxyConfig{&ProxyConfig
Patch Set #5, Line 85:
cfg := ProxyConfig{&ProxyConfig
File src/net/http/transport.go:
the HTTP_PROXY or http_proxy environment
// variableFor these, I'd say "the value of the", as elsewhere in Go we represent environment variable as the string "key=value".
// RequestMethod represents the REQUEST_METHOD environment
// variable. If set, this causes the HTTPProxy field to be
// ignored (see golang.org/s/cgihttpproxy).
RequestMethod stringthis is a weird addition. I'd prefer if we didn't need to have this.
I don't want a CGI detail leaking into API.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe has uploaded this change for review.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
This changes ProxyFromEnvironment such that it will always behave the same
even when the REQUEST_METHOD environment variable changes, which should
be safe, as the logic involving that method is only there to mitigate a
security issue that is not present if that environment variable is set
in the code.
Change-Id: I891dbe8b9b400a086874aa36372cf34f92b9c198
---
M src/go/format/format.go
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
5 files changed, 237 insertions(+), 98 deletions(-)
diff --git a/src/go/format/format.go b/src/go/format/format.go
index b9cacfe..c2bea6b 100644
--- a/src/go/format/format.go
+++ b/src/go/format/format.go
@@ -56,7 +56,7 @@
file, err = parser.ParseFile(fset, "", buf.Bytes(), parserMode)
if err != nil {
// We should never get here. If we do, provide good diagnostic.
- return fmt.Errorf("format.Node internal error (%s)", err)
+ return fmt.Errorf("format.Node internal error (%s); data: %q", err, buf.Bytes())
}
ast.SortImports(fset, file)
diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index f57e0c1..a85e972 100644
--- a/src/net/http/export_test.go
+++ b/src/net/http/export_test.go
@@ -76,9 +76,7 @@
}
func ResetCachedEnvironment() {
- httpProxyEnv.reset()
- httpsProxyEnv.reset()
- noProxyEnv.reset()
+ resetProxyConfig()
}
func (t *Transport) NumPendingRequestsForTesting() int {
diff --git a/src/net/http/proxy_test.go b/src/net/http/proxy_test.go
index f59a551..aa91dd5 100644
--- a/src/net/http/proxy_test.go
+++ b/src/net/http/proxy_test.go
@@ -35,10 +35,11 @@
}
func TestUseProxy(t *testing.T) {
- ResetProxyEnv()
- os.Setenv("NO_PROXY", "foobar.com, .barbaz.net")
+ cfg := ProxyConfig{
+ NoProxy: "foobar.com, .barbaz.net",
+ }
for _, test := range UseProxyTests {
- if useProxy(test.host+":80") != test.match {
+ if cfg.useProxy(test.host+":80") != test.match {
t.Errorf("useProxy(%v) = %v, want %v", test.host, !test.match, test.match)
}
}
@@ -74,14 +75,15 @@
}
func ResetProxyEnv() {
- for _, v := range []string{"HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy"} {
+ for _, v := range []string{"HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy", "REQUEST_METHOD"} {
os.Unsetenv(v)
}
ResetCachedEnvironment()
}
func TestInvalidNoProxy(t *testing.T) {
- ResetProxyEnv()
- os.Setenv("NO_PROXY", ":1")
- useProxy("example.com:80") // should not panic
+ cfg := ProxyConfig{
+ NoProxy: ":1",
+ }
+ cfg.useProxy("example.com:80") // should not panic
}
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 45e3fd2..1113302 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -247,6 +247,101 @@
}
}
+// ProxyConfig holds configuration for HTTP proxy settings. See
+// ProxyFromEnvironment for details.
+type ProxyConfig struct {
+ // Error, when non-nil, causes ProxyForRequest to return the
+ // error instead of a proxy destination, ignoring all other
+ // ProxyConfig field values.
+ Error error
+
+ // HTTPProxy represents the value of the HTTP_PROXY or
+ // http_proxy environment variable. It will be used as the proxy
+ // URL for HTTP requests and HTTPS requests unless overridden by
+ // HTTPSProxy or NoProxy.
+ HTTPProxy string
+
+ // HTTPSProxy represents the HTTPS_PROXY or https_proxy
+ // environment variable. It will be used as the proxy URL for
+ // HTTPS requests unless overridden by NoProxy.
+ HTTPSProxy string
+
+ // NoProxy represents the NO_PROXY or no_proxy environment
+ // variable. It specifies URLs that should be excluded from
+ // proxying as a comma-separated list of domain names or a
+ // single asterisk (*) to indicate that no proxying should be
+ // done. A domain name matches that name and all subdomains. A
+ // domain name with a leading "." matches subdomains only. For
+ // example "foo.com" matches "foo.com" and "bar.foo.com";
+ // ".y.com" matches "x.y.com" but not "y.com".
+ NoProxy string
+}
+
+// ProxyForRequest determines the URL to use for the given request.
+// This method may be used as a value for Transport.Proxy.
+func (cfg *ProxyConfig) ProxyForRequest(req *Request) (*url.URL, error) {
+ if cfg.Error != nil {
+ return nil, cfg.Error
+ }
+ var proxy string
+ if req.URL.Scheme == "https" {
+ proxy = cfg.HTTPSProxy
+ }
+ if proxy == "" {
+ proxy = cfg.HTTPProxy
+ }
+ if proxy == "" {
+ return nil, nil
+ }
+ if !cfg.useProxy(canonicalAddr(req.URL)) {
+ return nil, nil
+ }
+ proxyURL, err := url.Parse(proxy)
+ if err != nil ||
+ (proxyURL.Scheme != "http" &&
+ proxyURL.Scheme != "https" &&
+ proxyURL.Scheme != "socks5") {
+ // proxy was bogus. Try prepending "http://" to it and
+ // see if that parses correctly. If not, we fall
+ // through and complain about the original one.
+ if proxyURL, err := url.Parse("http://" + proxy); err == nil {
+ return proxyURL, nil
+ }
+ }
+ if err != nil {
+ return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
+ }
+ return proxyURL, nil
+}
+
+// NewProxyConfigFromEnvironment returns a ProxyConfig instance populated
+// from environment variables.
+//
+// Calling ProxyFromEnvironment().Proxy is equivalent to calling
+// ProxyFromEnvironment except that ProxyFromEnvironment only reads the
+// environment variables once, the first time it is called, where
+// NewProxyConfigFromEnvironment reads the variables every time it is called.
+func NewProxyConfigFromEnvironment() *ProxyConfig {
+ cfg := &ProxyConfig{
+ HTTPProxy: getEnvAny("HTTP_PROXY", "http_proxy"),
+ HTTPSProxy: getEnvAny("HTTPS_PROXY", "https_proxy"),
+ NoProxy: getEnvAny("NO_PROXY", "no_proxy"),
+ }
+ if cfg.HTTPProxy != "" && os.Getenv("REQUEST_METHOD") != "" {
+ cfg.Error = errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
+ }
+ return cfg
+}
+
+func getEnvAny(names ...string) string {
+ for _, n := range names {
+ if val := os.Getenv(n); val != "" {
+ return val
+ }
+ }
+ return ""
+}
+
// ProxyFromEnvironment returns the URL of the proxy to use for a
// given request, as indicated by the environment variables
// HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the lowercase versions
@@ -264,39 +359,7 @@
// As a special case, if req.URL.Host is "localhost" (with or without
// a port number), then a nil URL and nil error will be returned.
func ProxyFromEnvironment(req *Request) (*url.URL, error) {
- var proxy string
- if req.URL.Scheme == "https" {
- proxy = httpsProxyEnv.Get()
- }
- if proxy == "" {
- proxy = httpProxyEnv.Get()
- if proxy != "" && os.Getenv("REQUEST_METHOD") != "" {
- return nil, errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
- }
- }
- if proxy == "" {
- return nil, nil
- }
- if !useProxy(canonicalAddr(req.URL)) {
- return nil, nil
- }
- proxyURL, err := url.Parse(proxy)
- if err != nil ||
- (proxyURL.Scheme != "http" &&
- proxyURL.Scheme != "https" &&
- proxyURL.Scheme != "socks5") {
- // proxy was bogus. Try prepending "http://" to it and
- // see if that parses correctly. If not, we fall
- // through and complain about the original one.
- if proxyURL, err := url.Parse("http://" + proxy); err == nil {
- return proxyURL, nil
- }
-
- }
- if err != nil {
- return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
- }
- return proxyURL, nil
+ return defaultProxyConfig().ProxyForRequest(req)
}
// ProxyURL returns a proxy function (for use in a Transport)
@@ -566,44 +629,25 @@
//
var (
- httpProxyEnv = &envOnce{
- names: []string{"HTTP_PROXY", "http_proxy"},
- }
- httpsProxyEnv = &envOnce{
- names: []string{"HTTPS_PROXY", "https_proxy"},
- }
- noProxyEnv = &envOnce{
- names: []string{"NO_PROXY", "no_proxy"},
- }
+ // proxyConfigOnce guards proxyConfig
+ proxyConfigOnce sync.Once
+ proxyConfigValue *ProxyConfig
)
-// envOnce looks up an environment variable (optionally by multiple
-// names) once. It mitigates expensive lookups on some platforms
-// (e.g. Windows).
-type envOnce struct {
- names []string
- once sync.Once
- val string
+// defaultProxyConfig returns a ProxyConfig value looked up
+// from the environment. This mitigates expensive lookups
+// on some platforms (e.g. Windows).
+func defaultProxyConfig() *ProxyConfig {
+ proxyConfigOnce.Do(func() {
+ proxyConfigValue = NewProxyConfigFromEnvironment()
+ })
+ return proxyConfigValue
}
-func (e *envOnce) Get() string {
- e.once.Do(e.init)
- return e.val
-}
-
-func (e *envOnce) init() {
- for _, n := range e.names {
- e.val = os.Getenv(n)
- if e.val != "" {
- return
- }
- }
-}
-
-// reset is used by tests
-func (e *envOnce) reset() {
- e.once = sync.Once{}
- e.val = ""
+// resetProxyConfig is used by tests.
+func resetProxyConfig() {
+ proxyConfigOnce = sync.Once{}
+ proxyConfigValue = nil
}
func (t *Transport) connectMethodForRequest(treq *transportRequest) (cm connectMethod, err error) {
@@ -1244,7 +1288,7 @@
// useProxy reports whether requests to addr should use a proxy,
// according to the NO_PROXY or no_proxy environment variable.
// addr is always a canonicalAddr with a host and port.
-func useProxy(addr string) bool {
+func (cfg *ProxyConfig) useProxy(addr string) bool {
if len(addr) == 0 {
return true
}
@@ -1261,7 +1305,7 @@
}
}
- noProxy := noProxyEnv.Get()
+ noProxy := cfg.NoProxy
if noProxy == "*" {
return false
}
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index dc55816..6832d28 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -2391,28 +2391,123 @@
{noenv: ".foo.com", req: "http://example.com/", env: "proxy", want: "http://proxy"},
}
+func testProxyForRequest(t *testing.T, tt proxyFromEnvTest, proxyForRequest func(req *Request) (*url.URL, error)) {
+ t.Helper()
+ reqURL := tt.req
+ if reqURL == "" {
+ reqURL = "http://example.com"
+ }
+ req, _ := NewRequest("GET", reqURL, nil)
+ url, err := proxyForRequest(req)
+ if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e {
+ t.Errorf("%v: got error = %q, want %q", tt, g, e)
+ return
+ }
+ if got := fmt.Sprintf("%s", url); got != tt.want {
+ t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
+ }
+}
+
func TestProxyFromEnvironment(t *testing.T) {
ResetProxyEnv()
defer ResetProxyEnv()
for _, tt := range proxyFromEnvTests {
- os.Setenv("HTTP_PROXY", tt.env)
- os.Setenv("HTTPS_PROXY", tt.httpsenv)
- os.Setenv("NO_PROXY", tt.noenv)
- os.Setenv("REQUEST_METHOD", tt.reqmeth)
- ResetCachedEnvironment()
- 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("%v: got error = %q, want %q", tt, g, e)
- continue
- }
- if got := fmt.Sprintf("%s", url); got != tt.want {
- t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
- }
+ testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
+ os.Setenv("HTTP_PROXY", tt.env)
+ os.Setenv("HTTPS_PROXY", tt.httpsenv)
+ os.Setenv("NO_PROXY", tt.noenv)
+ os.Setenv("REQUEST_METHOD", tt.reqmeth)
+ ResetCachedEnvironment()
+ return ProxyFromEnvironment(req)
+ })
+ }
+}
+
+func TestProxyFromEnvironmentLowerCase(t *testing.T) {
+ ResetProxyEnv()
+ defer ResetProxyEnv()
+ for _, tt := range proxyFromEnvTests {
+ testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
+ os.Setenv("http_proxy", tt.env)
+ os.Setenv("https_proxy", tt.httpsenv)
+ os.Setenv("no_proxy", tt.noenv)
+ os.Setenv("REQUEST_METHOD", tt.reqmeth)
+ ResetCachedEnvironment()
+ return ProxyFromEnvironment(req)
+ })
+ }
+}
+
+func TestProxyConfig(t *testing.T) {
+ for _, tt := range proxyFromEnvTests {
+ testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
+ cfg := ProxyConfig{
+ HTTPProxy: tt.env,
+ HTTPSProxy: tt.httpsenv,
+ NoProxy: tt.noenv,
+ }
+ if tt.reqmeth != "" && tt.env != "" {
+ cfg.Error = errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
+ }
+ return cfg.ProxyForRequest(req)
+ })
+ }
+}
+
+func TestNewProxyConfigFromEnvironment(t *testing.T) {
+ ResetProxyEnv()
+ defer ResetProxyEnv()
+ os.Setenv("HTTP_PROXY", "httpproxy")
+ os.Setenv("HTTPS_PROXY", "httpsproxy")
+ os.Setenv("NO_PROXY", "noproxy")
+ got := NewProxyConfigFromEnvironment()
+ want := ProxyConfig{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
+ }
+}
+
+func TestNewProxyConfigFromEnvironmentWithRequestMethod(t *testing.T) {
+ ResetProxyEnv()
+ defer ResetProxyEnv()
+ os.Setenv("HTTP_PROXY", "httpproxy")
+ os.Setenv("HTTPS_PROXY", "httpsproxy")
+ os.Setenv("NO_PROXY", "noproxy")
+ os.Setenv("REQUEST_METHOD", "requestmethod")
+ got := NewProxyConfigFromEnvironment()
+ expectErr := errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
+ if got.Error == nil || got.Error.Error() != expectErr.Error() {
+ t.Errorf("unexpected proxy config error, got %#v want %#v", got.Error, expectErr)
+ }
+ got.Error = nil
+ want := ProxyConfig{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
+ }
+}
+
+func TestNewProxyConfigFromEnvironmentLowerCase(t *testing.T) {
+ ResetProxyEnv()
+ defer ResetProxyEnv()
+ os.Setenv("http_proxy", "httpproxy")
+ os.Setenv("https_proxy", "httpsproxy")
+ os.Setenv("no_proxy", "noproxy")
+ got := NewProxyConfigFromEnvironment()
+ want := ProxyConfig{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
}
}
To view, visit change 76192. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
I think I've addressed all feedback now. The Error field one is the most likely to be controversial.
5 comments:
the HTTP_PROXY or http_proxy environment
// variable
For these, I'd say "the value of the", as elsewhere in Go we represent environment variable as the s […]
Done
// RequestMethod represents the REQUEST_METHOD environment
// variable. If set, this causes the HTTPProxy field to be
// ignored (see golang.org/s/cgihttpproxy).
RequestMethod stringthis is a weird addition. I'd prefer if we didn't need to have this. […]
I agree it's odd to have it here - it's there because I didn't want to change the existing logic. We could make ProxyConfigFromEnvironment return an empty ProxyConfig value if REQUEST_METHOD is set, but the problem with that is that it will change existing behaviour - currently if REQUEST_METHOD is set, we'll see an error "refusing to use HTTP_PROXY value in CGI environment" and you won't be able make any HTTP calls because RoundTrip will fail.
Another possibility might be to have an Error field in ProxyConfig:
// Error, when non-nil, causes ProxyForRequest to return the error instead of a valid
// proxy destination, ignoring all other field values.
Error errorI'll go with that for the time being. What do you think?
Patch Set #5, Line 324:
func ProxyConfigFromEnvironment() *ProxyConfig {
Naming nit: How about NewProxyConfigFromEnvironment, to emphasize that this re-reads the environment […]
Done
Patch Set #5, Line 637:
proxyConfignit: defaultProxyConfig?
Done
nit: as before, // reset is used by tests
Done
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #2 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
This changes ProxyFromEnvironment such that it will always behave the same
even when the REQUEST_METHOD environment variable changes, which should
be safe, as the logic involving that method is only there to mitigate a
security issue that is not present if that environment variable is set
in the code.
Change-Id: I891dbe8b9b400a086874aa36372cf34f92b9c198
---
M src/go/format/format.go
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
5 files changed, 237 insertions(+), 98 deletions(-)
To view, visit change 76192. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
2 comments:
Patch Set #5, Line 38:
cfg := ProxyConfig{&ProxyConfig
Done
Patch Set #5, Line 85:
cfg := ProxyConfig{&ProxyConfig
Done
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Daniel Martí (Gerrit)
Did you mean to duplicate this CL?
roger peppe (Gerrit)
roger peppe uploaded patch set #6 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
This changes ProxyFromEnvironment such that it will always behave the same
even when the REQUEST_METHOD environment variable changes, which should
be safe, as the logic involving that method is only there to mitigate a
security issue that is not present if that environment variable is set
in the code.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
4 files changed, 211 insertions(+), 97 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
You didn't upload any new changes. You only updated the commit message.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick abandoned this change.
To view, visit change 76192. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #7 to this change.
net/http: add ProxyConfig
This makes the functionality of ProxyFromEnvironment
more general. See discussion at https://github.com/golang/go/issues/22079.
This changes ProxyFromEnvironment such that it will always behave the same
even when the REQUEST_METHOD environment variable changes, which should
be safe, as the logic involving that method is only there to mitigate a
security issue that is not present if that environment variable is set
in the code.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/go/format/format.go
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
5 files changed, 237 insertions(+), 98 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
Looks good to me but want to see if Brad has any comments about the Error field.
Patch set 7:Run-TryBot +1Code-Review +1
2 comments:
Patch Set #7, Line 59:
return fmt.Errorf("format.Node internal error (%s); data: %q", err, buf.Bytes())I think another change is leaking here? Looks like this belongs in a different change.
File src/net/http/transport_test.go:
Patch Set #7, Line 2383:
wanterr: errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")},nit: if we make this a private errCannotUseProxyInCGI variable, we can check err exactly rather than the somewhat hacky string comparison.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=4f6f7c65
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on darwin-amd64-10_11:
See https://storage.googleapis.com/go-build-log/4f6f7c65/darwin-amd64-10_11_491fb13d.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Brad Fitzpatrick (Gerrit)
4 comments:
Patch Set #7, Line 7:
add ProxyConfigadd ProxyConfig and NewProxyConfigFromEnvironment
Patch Set #7, Line 10:
more general. See discussion at https://github.com/golang/go/issues/22079.delete this last sentence and add the conventional footer line:
Fixes #22079
// Error, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Error errorper convention, looking at $GOROOT/api/*.txt, this would be named "Err error" instead:
go1.txt:pkg net, type AddrError struct, Err string
go1.txt:pkg net, type DNSConfigError struct, Err error
go1.txt:pkg net, type DNSError struct, Err string
go1.txt:pkg net, type OpError struct, Err error
go1.txt:pkg net/url, type Error struct, Err error
go1.txt:pkg os, type LinkError struct, Err error
go1.txt:pkg os, type PathError struct, Err error
go1.txt:pkg os, type SyscallError struct, Err error
go1.txt:pkg os/exec, type Error struct, Err error
go1.txt:pkg strconv, type NumError struct, Err error
go1.txt:pkg syscall (windows-386), type DLLError struct, Err error
go1.txt:pkg syscall (windows-amd64), type DLLError struct, Err erroretc
This mitigates expensive lookups
// on some platforms (e.g. Windows).
is performance part of this CL? mention it in the commit message, if so. ideally with numbers.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
1 of 21 TryBots failed:
Failed on darwin-amd64-10_11: https://storage.googleapis.com/go-build-log/4f6f7c65/darwin-amd64-10_11_491fb13d.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 7:TryBot-Result -1
roger peppe (Gerrit)
roger peppe uploaded patch set #8 to this change.
net/http: add ProxyConfig and NewProxyConfigFromEnvironment
This makes the functionality of ProxyFromEnvironment
more general.
This changes ProxyFromEnvironment such that it will always behave the same
even when the REQUEST_METHOD environment variable changes, which should
be safe, as the logic involving that method is only there to mitigate a
security issue that is not present if that environment variable is set
in the code.
Fixes #22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
4 files changed, 240 insertions(+), 98 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
5 comments:
Patch Set #7, Line 7:
add ProxyConfigadd ProxyConfig and NewProxyConfigFromEnvironment
Done
Patch Set #7, Line 10:
more general.
delete this last sentence and add the conventional footer line: […]
Done
Patch Set #7, Line 59:
return fmt.Errorf("format.Node internal error (%s)", err)
I think another change is leaking here? Looks like this belongs in a different change.
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
per convention, looking at $GOROOT/api/*.txt, this would be named "Err error" instead: […]
The reason for the above naming is almost certainly because those error types
need to define an Error method which would clash otherwise (rpc.Call.Error is a counterexample), but done anyway.
// defaultProxyConfig returns a Prox
is performance part of this CL? mention it in the commit message, if so. ideally with numbers.
I just copied the comment from the old code. I don't think performance is a particular part of this CL, although I suspect it might make things a tad faster (less Once.Do calls, no call to os.Getenv("REQUEST_METHOD")).
I can add new benchmarks if you think it's worth it.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
Actually, looking at this again, I don't think you need to export this field. It can be lowercase.
And if you don't even want the field at all, you could use a sentinel pointer value.
e.g.
var cgiSecurityErrorProxy = new(ProxyConfig)
And then return cgiSecurityErrorProxy from NewProxyConfigFromEnvironment, and then func (cfg *ProxyConfig) ProxyForRequest can check:
if cfg == cgiSecurityErrorProxy {
return the CGI error
}Thoughts?
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
Actually, looking at this again, I don't think you need to export this field. It can be lowercase. […]
FWIW, I'm fine with any of the above. Maybe a slight preference for the sentinel value so we add less API. (It might be confusing for users to see unexported fields in ProxyConfig.)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
FWIW, I'm fine with any of the above. […]
I'm not that keen on the sentinel idea because I think it's reasonable for users to expect that NewProxyConfigFromEnvironment will always return a new ProxyConfig instance (it's fine for them to mutate it after returning, for example, which would have unintuitive results if it's the sentinel, besides being potentially racy).
As for unexporting the error field, I'm not sure that's great either. I tend to agree with Tom that it might be confusing for users. As things are proposed currently, the behaviour of ProxyConfig.ProxyForRequest is entirely determined from its public fields. Unexporting the Error field means that it's not possible to do that. It's possible that someone might want to ignore the REQUEST_METHOD environment variable, for example, while still wanting the rest of the NewProxyConfigFromEnvironment functionality. Currently they can easily do that by setting Error to nil; the alternative with an unexported Error is to copy each field individually (but then there's always the risk that new fields remain uncopied if they're added in the future).
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
I'm not that keen on the sentinel idea because I think it's reasonable for users to expect that New […]
Well, I'm not that keen on so much new API in net/http for such a fringe use case.
Why don't we make some x/net/http/httpproxy package and move all the code there? Then net/http in std can vendor it.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
Well, I'm not that keen on so much new API in net/http for such a fringe use case. […]
I like that idea a lot. I was also not enthusiastic about the API increase but was eventually swayed when I couldn't think of something better. In retrospect, this is the obvious solution.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
I like that idea a lot. […]
That seems like a nice idea, but doesn't work exactly as is, because Transport.Proxy takes an http.Request for an argument, so there'd be a cyclic dependency. However the proxy logic doesn't actually use any fields in the request other than the URL, so how about something like this (in x/net/http/httpproxy, or perhaps just x/net/httpproxy, or even in x/net/proxy, which already exists)?
package httpproxy
// FromEnvironment returns a Config instance populated
// from environment variables.
func FromEnvironment() *Config
// Config holds configuration for HTTP proxy settings. See
// FromEnvironment for details.
type Config struct {
// HTTPProxy represents the value of the HTTP_PROXY or
// http_proxy environment variable. It will be used as the proxy
// URL for HTTP requests and HTTPS requests unless overridden by
// HTTPSProxy or NoProxy.
HTTPProxy string
// HTTPSProxy represents the HTTPS_PROXY or https_proxy
// environment variable. It will be used as the proxy URL for
// HTTPS requests unless overridden by NoProxy.
HTTPSProxy string
// NoProxy represents the NO_PROXY or no_proxy environment
// variable. It specifies URLs that should be excluded from
// proxying as a comma-separated list of domain names or a
// single asterisk (*) to indicate that no proxying should be
// done. A domain name matches that name and all subdomains. A
// domain name with a leading "." matches subdomains only. For
// example "foo.com" matches "foo.com" and "bar.foo.com";
NoProxy string
}
// ProxyForURL determines the URL to use for an HTTP request
// to the given URL.
func (cfg *Config) ProxyForURL(u *url.URL) (*url.URL, error)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
// Err, when non-nil, causes ProxyForRequest to return the
// error instead of a proxy destination, ignoring all other
// ProxyConfig field values.
Err error
That seems like a nice idea, but doesn't work exactly as is, because Transport.Proxy takes an http. […]
I just proposed https://go-review.googlesource.com/#/c/net/+/76910
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #9 to this change.
net/http: use vendored httpproxy package
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
CANNOT LAND BEFORE https://go-review.googlesource.com/c/net/+/76910
Fixes #22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
7 files changed, 470 insertions(+), 181 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #10 to this change.
net/http: use vendored httpproxy package
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
CANNOT LAND BEFORE https://go-review.googlesource.com/c/net/+/76910
Fixes #22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
7 files changed, 466 insertions(+), 180 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
/ HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the lowercase versions
// thereof). HTTPS_PROXY takes precedence over HTTP_PROXY for https
// requests.
//I just proposed https://go-review.googlesource. […]
I updated this PR to use the vendored httpproxy package (only applicable if that package lands).
I didn't use a sentinel value for httpproxy.Config because I didn't want to change behaviour - currently REQUEST_METHOD is only inspected if a HTTP_PROXY is actually used.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #11 to this change.
net/http: use vendored httpproxy package
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
CANNOT LAND BEFORE https://go-review.googlesource.com/c/net/+/76910
Fixes #22079.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
7 files changed, 561 insertions(+), 184 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
roger peppe uploaded patch set #12 to this change.
net/http: use vendored httpproxy package
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
7 files changed, 561 insertions(+), 184 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
Updated to use the latest httpproxy version landed in golang.org/x/net.
roger peppe (Gerrit)
roger peppe uploaded patch set #13 to this change.
net/http: use vendored httpproxy package
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
7 files changed, 604 insertions(+), 184 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Tom Bergan (Gerrit)
LGTM. This seems like a low-risk change for 1.10. I will let Brad approve in case he thinks it's too late.
Also, merge in https://go-review.googlesource.com/c/net/+/80140 before submitting.
Patch set 13:Run-TryBot +1Code-Review +1
1 comment:
File src/net/http/transport.go:
Patch Set #13, Line 543:
defaultProxyConfigsync with function name
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=41bfb576
Gobot Gobot (Gerrit)
Build is still in progress...
This change failed on linux-amd64:
See https://storage.googleapis.com/go-build-log/41bfb576/linux-amd64_4c62bfa9.log
Consult https://build.golang.org/ to see whether it's a new failure. Other builds still in progress; subsequent failure notices suppressed until final report.
Gobot Gobot (Gerrit)
8 of 20 TryBots failed:
Failed on linux-amd64: https://storage.googleapis.com/go-build-log/41bfb576/linux-amd64_4c62bfa9.log
Failed on linux-386: https://storage.googleapis.com/go-build-log/41bfb576/linux-386_d4cd94e4.log
Failed on darwin-amd64-10_11: https://storage.googleapis.com/go-build-log/41bfb576/darwin-amd64-10_11_f4db7857.log
Failed on linux-amd64-race: https://storage.googleapis.com/go-build-log/41bfb576/linux-amd64-race_4662854d.log
Failed on freebsd-amd64-110: https://storage.googleapis.com/go-build-log/41bfb576/freebsd-amd64-110_2373158a.log
Failed on windows-386-2008: https://storage.googleapis.com/go-build-log/41bfb576/windows-386-2008_8b4344d5.log
Failed on openbsd-amd64-60: https://storage.googleapis.com/go-build-log/41bfb576/openbsd-amd64-60_878a3416.log
Failed on windows-amd64-2016: https://storage.googleapis.com/go-build-log/41bfb576/windows-amd64-2016_ae73a3d0.log
Consult https://build.golang.org/ to see whether they are new failures.
Patch set 13:TryBot-Result -1
roger peppe (Gerrit)
roger peppe uploaded patch set #14 to this change.
net/http: use vendored httpproxy package
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/go/build/deps_test.go
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
8 files changed, 605 insertions(+), 184 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
Patch set 14:Run-TryBot +1
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=856e7265
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 14:TryBot-Result +1
Brad Fitzpatrick (Gerrit)
1 comment:
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
Whenever you update vendor directories in the main repo, you need to mention the git commit of the upstream in the commit message here.
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
roger peppe (Gerrit)
1 comment:
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
Whenever you update vendor directories in the main repo, you need to mention the git commit of the u […]
Ah, thanks, I wondered what the convention was. It would be nice to have the commits in the repo too though. Maybe we could even *gasp* use the standard go dep format...
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
1 comment:
This makes net/http use the logic factored out into
golang.org/x/net/http/httpproxy and vendored here.
Ah, thanks, I wondered what the convention was. […]
We use a mix of a few different things. I believe Russ uses dep for a few parts. It doesn't work for other parts.
And there's still the open bug for the golang_org (underscore) workaround hack which I'm sure go dep doesn't know about
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Roger, do you want to refresh this and get it in?
Brad Fitzpatrick (Gerrit)
Uploaded patch set 15.
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick uploaded patch set #15 to the change originally created by roger peppe.
net/http: vendor x/net/http/httpproxy, use it in net/http
From x/net git rev c7086645de2.
Updates #16704
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
---
M src/go/build/deps_test.go
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
8 files changed, 605 insertions(+), 184 deletions(-)
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Patch set 15:Run-TryBot +1Code-Review +2
Gobot Gobot (Gerrit)
TryBots beginning. Status page: https://farmer.golang.org/try?commit=3b0f3a2f
Gobot Gobot (Gerrit)
TryBots are happy.
Patch set 15:TryBot-Result +1
Brad Fitzpatrick (Gerrit)
Brad Fitzpatrick merged this change.
net/http: vendor x/net/http/httpproxy, use it in net/http
From x/net git rev c7086645de2.
Updates #16704
Change-Id: I4d642478fc69a52c973964845fca2fd402716e57
Reviewed-on: https://go-review.googlesource.com/68091
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
Run-TryBot: Brad Fitzpatrick <brad...@golang.org>
TryBot-Result: Gobot Gobot <go...@golang.org>
---
M src/go/build/deps_test.go
M src/net/http/export_test.go
M src/net/http/proxy_test.go
M src/net/http/transport.go
M src/net/http/transport_test.go
A src/vendor/golang_org/x/net/http/httpproxy/export_test.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy.go
A src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
8 files changed, 605 insertions(+), 184 deletions(-)
diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go
index 9aebd73..5137ccf 100644
--- a/src/go/build/deps_test.go
+++ b/src/go/build/deps_test.go
@@ -401,6 +401,7 @@
"crypto/rand",
"crypto/tls",
"golang_org/x/net/http/httpguts",
+ "golang_org/x/net/http/httpproxy",
"golang_org/x/net/http2/hpack",
"golang_org/x/net/idna",
"golang_org/x/text/unicode/norm",
diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index 1825acd..e0ceb40 100644
--- a/src/net/http/export_test.go
+++ b/src/net/http/export_test.go
@@ -76,9 +76,7 @@
}
func ResetCachedEnvironment() {
- httpProxyEnv.reset()
- httpsProxyEnv.reset()
- noProxyEnv.reset()
+ resetProxyConfig()
}
func (t *Transport) NumPendingRequestsForTesting() int {
diff --git a/src/net/http/proxy_test.go b/src/net/http/proxy_test.go
index f59a551..eef0ca8 100644
--- a/src/net/http/proxy_test.go
+++ b/src/net/http/proxy_test.go
@@ -13,37 +13,6 @@
// TODO(mattn):
// test ProxyAuth
-var UseProxyTests = []struct {
- host string
- match bool
-}{
- // Never proxy localhost:
- {"localhost", false},
- {"127.0.0.1", false},
- {"127.0.0.2", false},
- {"[::1]", false},
- {"[::2]", true}, // not a loopback address
-
- {"barbaz.net", false}, // match as .barbaz.net
- {"foobar.com", false}, // have a port but match
- {"foofoobar.com", true}, // not match as a part of foobar.com
- {"baz.com", true}, // not match as a part of barbaz.com
- {"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", false}, // match because NO_PROXY includes "foobar.com"
-}
-
-func TestUseProxy(t *testing.T) {
- ResetProxyEnv()
- os.Setenv("NO_PROXY", "foobar.com, .barbaz.net")
- for _, test := range UseProxyTests {
- if useProxy(test.host+":80") != test.match {
- t.Errorf("useProxy(%v) = %v, want %v", test.host, !test.match, test.match)
- }
- }
-}
-
var cacheKeysTests = []struct {
proxy string
scheme string
@@ -74,14 +43,8 @@
}
func ResetProxyEnv() {
- for _, v := range []string{"HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy"} {
+ for _, v := range []string{"HTTP_PROXY", "http_proxy", "NO_PROXY", "no_proxy", "REQUEST_METHOD"} {
os.Unsetenv(v)
}
ResetCachedEnvironment()
}
-
-func TestInvalidNoProxy(t *testing.T) {
- ResetProxyEnv()
- os.Setenv("NO_PROXY", ":1")
- useProxy("example.com:80") // should not panic
-}
diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index cce88ca..5bf9ff9 100644
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -29,6 +29,7 @@
"time"
"golang_org/x/net/http/httpguts"
+ "golang_org/x/net/http/httpproxy"
)
// DefaultTransport is the default implementation of Transport and is
@@ -272,39 +273,7 @@
// As a special case, if req.URL.Host is "localhost" (with or without
// a port number), then a nil URL and nil error will be returned.
func ProxyFromEnvironment(req *Request) (*url.URL, error) {
- var proxy string
- if req.URL.Scheme == "https" {
- proxy = httpsProxyEnv.Get()
- }
- if proxy == "" {
- proxy = httpProxyEnv.Get()
- if proxy != "" && os.Getenv("REQUEST_METHOD") != "" {
- return nil, errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
- }
- }
- if proxy == "" {
- return nil, nil
- }
- if !useProxy(canonicalAddr(req.URL)) {
- return nil, nil
- }
- proxyURL, err := url.Parse(proxy)
- if err != nil ||
- (proxyURL.Scheme != "http" &&
- proxyURL.Scheme != "https" &&
- proxyURL.Scheme != "socks5") {
- // proxy was bogus. Try prepending "http://" to it and
- // see if that parses correctly. If not, we fall
- // through and complain about the original one.
- if proxyURL, err := url.Parse("http://" + proxy); err == nil {
- return proxyURL, nil
- }
-
- }
- if err != nil {
- return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
- }
- return proxyURL, nil
+ return envProxyFunc()(req.URL)
}
// ProxyURL returns a proxy function (for use in a Transport)
@@ -574,44 +543,25 @@
//
var (
- httpProxyEnv = &envOnce{
- names: []string{"HTTP_PROXY", "http_proxy"},
- }
- httpsProxyEnv = &envOnce{
- names: []string{"HTTPS_PROXY", "https_proxy"},
- }
- noProxyEnv = &envOnce{
- names: []string{"NO_PROXY", "no_proxy"},
- }
+ // proxyConfigOnce guards proxyConfig
+ envProxyOnce sync.Once
+ envProxyFuncValue func(*url.URL) (*url.URL, error)
)
-// envOnce looks up an environment variable (optionally by multiple
-// names) once. It mitigates expensive lookups on some platforms
-// (e.g. Windows).
-type envOnce struct {
- names []string
- once sync.Once
- val string
+// defaultProxyConfig returns a ProxyConfig value looked up
+// from the environment. This mitigates expensive lookups
+// on some platforms (e.g. Windows).
+func envProxyFunc() func(*url.URL) (*url.URL, error) {
+ envProxyOnce.Do(func() {
+ envProxyFuncValue = httpproxy.FromEnvironment().ProxyFunc()
+ })
+ return envProxyFuncValue
}
-func (e *envOnce) Get() string {
- e.once.Do(e.init)
- return e.val
-}
-
-func (e *envOnce) init() {
- for _, n := range e.names {
- e.val = os.Getenv(n)
- if e.val != "" {
- return
- }
- }
-}
-
-// reset is used by tests
-func (e *envOnce) reset() {
- e.once = sync.Once{}
- e.val = ""
+// resetProxyConfig is used by tests.
+func resetProxyConfig() {
+ envProxyOnce = sync.Once{}
+ envProxyFuncValue = nil
}
func (t *Transport) connectMethodForRequest(treq *transportRequest) (cm connectMethod, err error) {
@@ -1235,63 +1185,6 @@
return
}
-// useProxy reports whether requests to addr should use a proxy,
-// according to the NO_PROXY or no_proxy environment variable.
-// addr is always a canonicalAddr with a host and port.
-func useProxy(addr string) bool {
- if len(addr) == 0 {
- return true
- }
- host, _, err := net.SplitHostPort(addr)
- if err != nil {
- return false
- }
- if host == "localhost" {
- return false
- }
- if ip := net.ParseIP(host); ip != nil {
- if ip.IsLoopback() {
- return false
- }
- }
-
- noProxy := noProxyEnv.Get()
- if noProxy == "*" {
- return false
- }
-
- addr = strings.ToLower(strings.TrimSpace(addr))
- if hasPort(addr) {
- addr = addr[:strings.LastIndex(addr, ":")]
- }
-
- for _, p := range strings.Split(noProxy, ",") {
- p = strings.ToLower(strings.TrimSpace(p))
- if len(p) == 0 {
- continue
- }
- if hasPort(p) {
- p = p[:strings.LastIndex(p, ":")]
- }
- if addr == p {
- return false
- }
- if len(p) == 0 {
- // There is no host part, likely the entry is malformed; ignore.
- continue
- }
- 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
- }
- }
- return true
-}
-
// connectMethod is the map key (in its String form) for keeping persistent
// TCP connections alive for subsequent HTTP requests.
//
diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 5e35812..57309bb 100644
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -2381,7 +2381,7 @@
// where HTTP_PROXY can be attacker-controlled.
{env: "http://10.1.2.3:8080", reqmeth: "POST",
want: "<nil>",
- wanterr: errors.New("net/http: refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")},
+ wanterr: errors.New("refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")},
{want: "<nil>"},
@@ -2392,28 +2392,50 @@
{noenv: ".foo.com", req: "http://example.com/", env: "proxy", want: "http://proxy"},
}
+func testProxyForRequest(t *testing.T, tt proxyFromEnvTest, proxyForRequest func(req *Request) (*url.URL, error)) {
+ t.Helper()
+ reqURL := tt.req
+ if reqURL == "" {
+ reqURL = "http://example.com"
+ }
+ req, _ := NewRequest("GET", reqURL, nil)
+ url, err := proxyForRequest(req)
+ if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e {
+ t.Errorf("%v: got error = %q, want %q", tt, g, e)
+ return
+ }
+ if got := fmt.Sprintf("%s", url); got != tt.want {
+ t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
+ }
+}
+
func TestProxyFromEnvironment(t *testing.T) {
ResetProxyEnv()
defer ResetProxyEnv()
for _, tt := range proxyFromEnvTests {
- os.Setenv("HTTP_PROXY", tt.env)
- os.Setenv("HTTPS_PROXY", tt.httpsenv)
- os.Setenv("NO_PROXY", tt.noenv)
- os.Setenv("REQUEST_METHOD", tt.reqmeth)
- ResetCachedEnvironment()
- 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("%v: got error = %q, want %q", tt, g, e)
- continue
- }
- if got := fmt.Sprintf("%s", url); got != tt.want {
- t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
- }
+ testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
+ os.Setenv("HTTP_PROXY", tt.env)
+ os.Setenv("HTTPS_PROXY", tt.httpsenv)
+ os.Setenv("NO_PROXY", tt.noenv)
+ os.Setenv("REQUEST_METHOD", tt.reqmeth)
+ ResetCachedEnvironment()
+ return ProxyFromEnvironment(req)
+ })
+ }
+}
+
+func TestProxyFromEnvironmentLowerCase(t *testing.T) {
+ ResetProxyEnv()
+ defer ResetProxyEnv()
+ for _, tt := range proxyFromEnvTests {
+ testProxyForRequest(t, tt, func(req *Request) (*url.URL, error) {
+ os.Setenv("http_proxy", tt.env)
+ os.Setenv("https_proxy", tt.httpsenv)
+ os.Setenv("no_proxy", tt.noenv)
+ os.Setenv("REQUEST_METHOD", tt.reqmeth)
+ ResetCachedEnvironment()
+ return ProxyFromEnvironment(req)
+ })
}
}
diff --git a/src/vendor/golang_org/x/net/http/httpproxy/export_test.go b/src/vendor/golang_org/x/net/http/httpproxy/export_test.go
new file mode 100644
index 0000000..36b29d2
--- /dev/null
+++ b/src/vendor/golang_org/x/net/http/httpproxy/export_test.go
@@ -0,0 +1,7 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package httpproxy
+
+var ExportUseProxy = (*Config).useProxy
diff --git a/src/vendor/golang_org/x/net/http/httpproxy/proxy.go b/src/vendor/golang_org/x/net/http/httpproxy/proxy.go
new file mode 100644
index 0000000..f82748d
--- /dev/null
+++ b/src/vendor/golang_org/x/net/http/httpproxy/proxy.go
@@ -0,0 +1,239 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package httpproxy provides support for HTTP proxy determination
+// based on environment variables, as provided by net/http's
+// ProxyFromEnvironment function.
+//
+// The API is not subject to the Go 1 compatibility promise and may change at
+// any time.
+package httpproxy
+
+import (
+ "errors"
+ "fmt"
+ "net"
+ "net/url"
+ "os"
+ "strings"
+ "unicode/utf8"
+
+ "golang_org/x/net/idna"
+)
+
+// Config holds configuration for HTTP proxy settings. See
+// FromEnvironment for details.
+type Config struct {
+ // HTTPProxy represents the value of the HTTP_PROXY or
+ // http_proxy environment variable. It will be used as the proxy
+ // URL for HTTP requests and HTTPS requests unless overridden by
+ // HTTPSProxy or NoProxy.
+ HTTPProxy string
+
+ // HTTPSProxy represents the HTTPS_PROXY or https_proxy
+ // environment variable. It will be used as the proxy URL for
+ // HTTPS requests unless overridden by NoProxy.
+ HTTPSProxy string
+
+ // NoProxy represents the NO_PROXY or no_proxy environment
+ // variable. It specifies URLs that should be excluded from
+ // proxying as a comma-separated list of domain names or a
+ // single asterisk (*) to indicate that no proxying should be
+ // done. A domain name matches that name and all subdomains. A
+ // domain name with a leading "." matches subdomains only. For
+ // example "foo.com" matches "foo.com" and "bar.foo.com";
+ // ".y.com" matches "x.y.com" but not "y.com".
+ NoProxy string
+
+ // CGI holds whether the current process is running
+ // as a CGI handler (FromEnvironment infers this from the
+ // presence of a REQUEST_METHOD environment variable).
+ // When this is set, ProxyForURL will return an error
+ // when HTTPProxy applies, because a client could be
+ // setting HTTP_PROXY maliciously. See https://golang.org/s/cgihttpproxy.
+ CGI bool
+}
+
+// FromEnvironment returns a Config instance populated from the
+// environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the
+// lowercase versions thereof). HTTPS_PROXY takes precedence over
+// HTTP_PROXY for https requests.
+//
+// The environment values may be either a complete URL or a
+// "host[:port]", in which case the "http" scheme is assumed. An error
+// is returned if the value is a different form.
+func FromEnvironment() *Config {
+ return &Config{
+ HTTPProxy: getEnvAny("HTTP_PROXY", "http_proxy"),
+ HTTPSProxy: getEnvAny("HTTPS_PROXY", "https_proxy"),
+ NoProxy: getEnvAny("NO_PROXY", "no_proxy"),
+ CGI: os.Getenv("REQUEST_METHOD") != "",
+ }
+}
+
+func getEnvAny(names ...string) string {
+ for _, n := range names {
+ if val := os.Getenv(n); val != "" {
+ return val
+ }
+ }
+ return ""
+}
+
+// ProxyFunc returns a function that determines the proxy URL to use for
+// a given request URL. Changing the contents of cfg will not affect
+// proxy functions created earlier.
+//
+// A nil URL and nil error are returned if no proxy is defined in the
+// environment, or a proxy should not be used for the given request, as
+// defined by NO_PROXY.
+//
+// As a special case, if req.URL.Host is "localhost" (with or without a
+// port number), then a nil URL and nil error will be returned.
+func (cfg *Config) ProxyFunc() func(reqURL *url.URL) (*url.URL, error) {
+ // Prevent Config changes from affecting the function calculation.
+ // TODO Preprocess proxy settings for more efficient evaluation.
+ cfg1 := *cfg
+ return cfg1.proxyForURL
+}
+
+func (cfg *Config) proxyForURL(reqURL *url.URL) (*url.URL, error) {
+ var proxy string
+ if reqURL.Scheme == "https" {
+ proxy = cfg.HTTPSProxy
+ }
+ if proxy == "" {
+ proxy = cfg.HTTPProxy
+ if proxy != "" && cfg.CGI {
+ return nil, errors.New("refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
+ }
+ }
+ if proxy == "" {
+ return nil, nil
+ }
+ if !cfg.useProxy(canonicalAddr(reqURL)) {
+ return nil, nil
+ }
+ proxyURL, err := url.Parse(proxy)
+ if err != nil ||
+ (proxyURL.Scheme != "http" &&
+ proxyURL.Scheme != "https" &&
+ proxyURL.Scheme != "socks5") {
+ // proxy was bogus. Try prepending "http://" to it and
+ // see if that parses correctly. If not, we fall
+ // through and complain about the original one.
+ if proxyURL, err := url.Parse("http://" + proxy); err == nil {
+ return proxyURL, nil
+ }
+ }
+ if err != nil {
+ return nil, fmt.Errorf("invalid proxy address %q: %v", proxy, err)
+ }
+ return proxyURL, nil
+}
+
+// useProxy reports whether requests to addr should use a proxy,
+// according to the NO_PROXY or no_proxy environment variable.
+// addr is always a canonicalAddr with a host and port.
+func (cfg *Config) useProxy(addr string) bool {
+ if len(addr) == 0 {
+ return true
+ }
+ host, _, err := net.SplitHostPort(addr)
+ if err != nil {
+ return false
+ }
+ if host == "localhost" {
+ return false
+ }
+ if ip := net.ParseIP(host); ip != nil {
+ if ip.IsLoopback() {
+ return false
+ }
+ }
+
+ noProxy := cfg.NoProxy
+ if noProxy == "*" {
+ return false
+ }
+
+ addr = strings.ToLower(strings.TrimSpace(addr))
+ if hasPort(addr) {
+ addr = addr[:strings.LastIndex(addr, ":")]
+ }
+
+ for _, p := range strings.Split(noProxy, ",") {
+ p = strings.ToLower(strings.TrimSpace(p))
+ if len(p) == 0 {
+ continue
+ }
+ if hasPort(p) {
+ p = p[:strings.LastIndex(p, ":")]
+ }
+ if addr == p {
+ return false
+ }
+ if len(p) == 0 {
+ // There is no host part, likely the entry is malformed; ignore.
+ continue
+ }
+ 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
+ }
+ }
+ return true
+}
+
+var portMap = map[string]string{
+ "http": "80",
+ "https": "443",
+ "socks5": "1080",
+}
+
+// canonicalAddr returns url.Host but always with a ":port" suffix
+func canonicalAddr(url *url.URL) string {
+ addr := url.Hostname()
+ if v, err := idnaASCII(addr); err == nil {
+ addr = v
+ }
+ port := url.Port()
+ if port == "" {
+ port = portMap[url.Scheme]
+ }
+ return net.JoinHostPort(addr, port)
+}
+
+// Given a string of the form "host", "host:port", or "[ipv6::address]:port",
+// return true if the string includes a port.
+func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") }
+
+func idnaASCII(v string) (string, error) {
+ // TODO: Consider removing this check after verifying performance is okay.
+ // Right now punycode verification, length checks, context checks, and the
+ // permissible character tests are all omitted. It also prevents the ToASCII
+ // call from salvaging an invalid IDN, when possible. As a result it may be
+ // possible to have two IDNs that appear identical to the user where the
+ // ASCII-only version causes an error downstream whereas the non-ASCII
+ // version does not.
+ // Note that for correct ASCII IDNs ToASCII will only do considerably more
+ // work, but it will not cause an allocation.
+ if isASCII(v) {
+ return v, nil
+ }
+ return idna.Lookup.ToASCII(v)
+}
+
+func isASCII(s string) bool {
+ for i := 0; i < len(s); i++ {
+ if s[i] >= utf8.RuneSelf {
+ return false
+ }
+ }
+ return true
+}
diff --git a/src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go b/src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
new file mode 100644
index 0000000..fde2514
--- /dev/null
+++ b/src/vendor/golang_org/x/net/http/httpproxy/proxy_test.go
@@ -0,0 +1,298 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package httpproxy_test
+
+import (
+ "bytes"
+ "errors"
+ "fmt"
+ "net/url"
+ "os"
+ "strings"
+ "testing"
+
+ "golang_org/x/net/http/httpproxy"
+)
+
+type proxyForURLTest struct {
+ cfg httpproxy.Config
+ req string // URL to fetch; blank means "http://example.com"
+ want string
+ wanterr error
+}
+
+func (t proxyForURLTest) String() string {
+ var buf bytes.Buffer
+ space := func() {
+ if buf.Len() > 0 {
+ buf.WriteByte(' ')
+ }
+ }
+ if t.cfg.HTTPProxy != "" {
+ fmt.Fprintf(&buf, "http_proxy=%q", t.cfg.HTTPProxy)
+ }
+ if t.cfg.HTTPSProxy != "" {
+ space()
+ fmt.Fprintf(&buf, "https_proxy=%q", t.cfg.HTTPSProxy)
+ }
+ if t.cfg.NoProxy != "" {
+ space()
+ fmt.Fprintf(&buf, "no_proxy=%q", t.cfg.NoProxy)
+ }
+ req := "http://example.com"
+ if t.req != "" {
+ req = t.req
+ }
+ space()
+ fmt.Fprintf(&buf, "req=%q", req)
+ return strings.TrimSpace(buf.String())
+}
+
+var proxyForURLTests = []proxyForURLTest{{
+ cfg: httpproxy.Config{
+ HTTPProxy: "127.0.0.1:8080",
+ },
+ want: "http://127.0.0.1:8080",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "cache.corp.example.com:1234",
+ },
+ want: "http://cache.corp.example.com:1234",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "cache.corp.example.com",
+ },
+ want: "http://cache.corp.example.com",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "https://cache.corp.example.com",
+ },
+ want: "https://cache.corp.example.com",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "http://127.0.0.1:8080",
+ },
+ want: "http://127.0.0.1:8080",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "https://127.0.0.1:8080",
+ },
+ want: "https://127.0.0.1:8080",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "socks5://127.0.0.1",
+ },
+ want: "socks5://127.0.0.1",
+}, {
+ // Don't use secure for http
+ cfg: httpproxy.Config{
+ HTTPProxy: "http.proxy.tld",
+ HTTPSProxy: "secure.proxy.tld",
+ },
+ req: "http://insecure.tld/",
+ want: "http://http.proxy.tld",
+}, {
+ // Use secure for https.
+ cfg: httpproxy.Config{
+ HTTPProxy: "http.proxy.tld",
+ HTTPSProxy: "secure.proxy.tld",
+ },
+ req: "https://secure.tld/",
+ want: "http://secure.proxy.tld",
+}, {
+ cfg: httpproxy.Config{
+ HTTPProxy: "http.proxy.tld",
+ HTTPSProxy: "https://secure.proxy.tld",
+ },
+ req: "https://secure.tld/",
+ want: "https://secure.proxy.tld",
+}, {
+ // Issue 16405: don't use HTTP_PROXY in a CGI environment,
+ // where HTTP_PROXY can be attacker-controlled.
+ cfg: httpproxy.Config{
+ HTTPProxy: "http://10.1.2.3:8080",
+ CGI: true,
+ },
+ want: "<nil>",
+ wanterr: errors.New("refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy"),
+}, {
+ // HTTPS proxy is still used even in CGI environment.
+ // (perhaps dubious but it's the historical behaviour).
+ cfg: httpproxy.Config{
+ HTTPSProxy: "https://secure.proxy.tld",
+ CGI: true,
+ },
+ req: "https://secure.tld/",
+ want: "https://secure.proxy.tld",
+}, {
+ want: "<nil>",
+}, {
+ cfg: httpproxy.Config{
+ NoProxy: "example.com",
+ HTTPProxy: "proxy",
+ },
+ req: "http://example.com/",
+ want: "<nil>",
+}, {
+ cfg: httpproxy.Config{
+ NoProxy: ".example.com",
+ HTTPProxy: "proxy",
+ },
+ req: "http://example.com/",
+ want: "<nil>",
+}, {
+ cfg: httpproxy.Config{
+ NoProxy: "ample.com",
+ HTTPProxy: "proxy",
+ },
+ req: "http://example.com/",
+ want: "http://proxy",
+}, {
+ cfg: httpproxy.Config{
+ NoProxy: "example.com",
+ HTTPProxy: "proxy",
+ },
+ req: "http://foo.example.com/",
+ want: "<nil>",
+}, {
+ cfg: httpproxy.Config{
+ NoProxy: ".foo.com",
+ HTTPProxy: "proxy",
+ },
+ req: "http://example.com/",
+ want: "http://proxy",
+}}
+
+func testProxyForURL(t *testing.T, tt proxyForURLTest) {
+ t.Helper()
+ reqURLStr := tt.req
+ if reqURLStr == "" {
+ reqURLStr = "http://example.com"
+ }
+ reqURL, err := url.Parse(reqURLStr)
+ if err != nil {
+ t.Errorf("invalid URL %q", reqURLStr)
+ return
+ }
+ cfg := tt.cfg
+ proxyForURL := cfg.ProxyFunc()
+ url, err := proxyForURL(reqURL)
+ if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e {
+ t.Errorf("%v: got error = %q, want %q", tt, g, e)
+ return
+ }
+ if got := fmt.Sprintf("%s", url); got != tt.want {
+ t.Errorf("%v: got URL = %q, want %q", tt, url, tt.want)
+ }
+
+ // Check that changing the Config doesn't change the results
+ // of the functuon.
+ cfg = httpproxy.Config{}
+ url, err = proxyForURL(reqURL)
+ if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.wanterr); g != e {
+ t.Errorf("(after mutating config) %v: got error = %q, want %q", tt, g, e)
+ return
+ }
+ if got := fmt.Sprintf("%s", url); got != tt.want {
+ t.Errorf("(after mutating config) %v: got URL = %q, want %q", tt, url, tt.want)
+ }
+}
+
+func TestProxyForURL(t *testing.T) {
+ for _, tt := range proxyForURLTests {
+ testProxyForURL(t, tt)
+ }
+}
+
+func TestFromEnvironment(t *testing.T) {
+ os.Setenv("HTTP_PROXY", "httpproxy")
+ os.Setenv("HTTPS_PROXY", "httpsproxy")
+ os.Setenv("NO_PROXY", "noproxy")
+ os.Setenv("REQUEST_METHOD", "")
+ got := httpproxy.FromEnvironment()
+ want := httpproxy.Config{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
+ }
+}
+
+func TestFromEnvironmentWithRequestMethod(t *testing.T) {
+ os.Setenv("HTTP_PROXY", "httpproxy")
+ os.Setenv("HTTPS_PROXY", "httpsproxy")
+ os.Setenv("NO_PROXY", "noproxy")
+ os.Setenv("REQUEST_METHOD", "PUT")
+ got := httpproxy.FromEnvironment()
+ want := httpproxy.Config{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ CGI: true,
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
+ }
+}
+
+func TestFromEnvironmentLowerCase(t *testing.T) {
+ os.Setenv("http_proxy", "httpproxy")
+ os.Setenv("https_proxy", "httpsproxy")
+ os.Setenv("no_proxy", "noproxy")
+ os.Setenv("REQUEST_METHOD", "")
+ got := httpproxy.FromEnvironment()
+ want := httpproxy.Config{
+ HTTPProxy: "httpproxy",
+ HTTPSProxy: "httpsproxy",
+ NoProxy: "noproxy",
+ }
+ if *got != want {
+ t.Errorf("unexpected proxy config, got %#v want %#v", got, want)
+ }
+}
+
+var UseProxyTests = []struct {
+ host string
+ match bool
+}{
+ // Never proxy localhost:
+ {"localhost", false},
+ {"127.0.0.1", false},
+ {"127.0.0.2", false},
+ {"[::1]", false},
+ {"[::2]", true}, // not a loopback address
+
+ {"barbaz.net", false}, // match as .barbaz.net
+ {"foobar.com", false}, // have a port but match
+ {"foofoobar.com", true}, // not match as a part of foobar.com
+ {"baz.com", true}, // not match as a part of barbaz.com
+ {"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", false}, // match because NO_PROXY includes "foobar.com"
+}
+
+func TestUseProxy(t *testing.T) {
+ cfg := &httpproxy.Config{
+ NoProxy: "foobar.com, .barbaz.net",
+ }
+ for _, test := range UseProxyTests {
+ if httpproxy.ExportUseProxy(cfg, test.host+":80") != test.match {
+ t.Errorf("useProxy(%v) = %v, want %v", test.host, !test.match, test.match)
+ }
+ }
+}
+
+func TestInvalidNoProxy(t *testing.T) {
+ cfg := &httpproxy.Config{
+ NoProxy: ":1",
+ }
+ ok := httpproxy.ExportUseProxy(cfg, "example.com:80") // should not panic
+ if !ok {
+ t.Errorf("useProxy unexpected return; got false; want true")
+ }
+}
To view, visit change 68091. To unsubscribe, or for help writing mail filters, visit settings.