@bradfitz ?

CC @tombergan

This sounds entirely reasonable to me. For reference, "NoProxy" is the non-trivial part of the implementation. @rogpeppe, want to send a CL?

For completeness, another option is to add http.ReloadProxyConfigFromEnvironment() to reload the environment variables. This is not a good idea because it will be hard to write that function in such a way that it won't race with concurrent calls to RoundTrip (which may be calling ProxyFromEnvironment).

@tombergan Done

https://go-review.googlesource.com/#/c/go/+/68091/

@rogpeppe, can you flesh out how you're going to use this feature? Specifically, why does ProxyConfigFromEnvironment() return the current values of the environment variables, rather than read the environment once and cache the result?

I did not read your proposed definition of ProxyConfigFromEnvironment carefully enough the first time I read your proposal. I'm not sure how you will update ProxyConfig safely when there are concurrent RoundTrip calls.

To guard against races, it's not hard to do something like:

transport.Proxy = func(req *http.Request) (*url.URL, error) {
	myType.mu.Lock()
	defer myType.mu.Unlock()
	return myType.proxyConfig.ProxyForRequest(req)
}

An alternative might be to put a mutex into ProxyConfig, unexport the fields, and provide a way of updating it. That provides a more complex API though, and I'm not sure it's worth it - there are plenty of exported fields in net/http that are vulnerable to concurrency issues if modified without care.

I agree adding a mutex to ProxyConfig is not worth it. The part that confused me is this:

// ProxyFromEnvironment is equivalent to ProxyConfigFromEnvironment().Proxy
// except that it only reads the environment variables once,
// the first time it is called.

Why does ProxyFromEnvironment need to re-read the environment variables each time it is called? It seems simpler to assume that the environment variables are fixed at startup. If the config needs to change, the caller can change the struct using a lock as in your above example.

Why does ProxyFromEnvironment need to re-read the environment variables each time it is called? It seems simpler to assume that the environment variables are fixed at startup. If the config needs to change, the caller can change the struct using a lock as in your above example.

It doesn't. I think my explanation was confusing. I've updated the explanation and added a note about the need for a mutex. PTAL.

Change https://golang.org/cl/68091 mentions this issue: net/http: add ProxyConfig and NewProxyConfigFromEnvironment

I see this is unplanned. We are hitting this problem, what is the recommended workaround, and will this PR be closed eventually? It would be nice to have this in the standard library.

@andrewrynhard, actually according to the comments above it looks like this happened:

https://godoc.org/golang.org/x/net/http/httpproxy

So closing this as done. Let me know if I missed something.