Skip to content

Commit

Permalink
Pin httpproxy package
Browse files Browse the repository at this point in the history
golang.org/x/net/http/httpproxy changed it's behavior regarding the
HTTP_PROXY and HTTPS_PROXY env vars.

it was to fix this issue: golang/go#40909
and this is the change: https://go-review.googlesource.com/c/net/+/249440

After this change the HTTP_PROXY env var will be ignored for HTTPS requests,
you have to also set HTTPS_PROXY in order for HTTPS requests from the agent
to be proxied. This means that if there are customers setting HTTP_PROXY
but not HTTPS_PROXY, their requests will no longer be proxied.

Pin to an older version of the httpproxy package to avoid this behavior
change.

Copying the file in-repo so that we can upgrade the rest of
golang.org/x/net without issue, and also so that any potential future
uses of x/net/http/httpproxy do not collide with maintaining this
previous desired behavior.
  • Loading branch information
sparrc committed Jan 30, 2023
1 parent b0bfdf3 commit 822c4eb
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import (
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 unless overridden by NoProxy.
// URL for HTTP requests and HTTPS requests unless overridden by
// HTTPSProxy or NoProxy.
HTTPProxy string

// HTTPSProxy represents the HTTPS_PROXY or https_proxy
Expand Down Expand Up @@ -81,7 +82,8 @@ type config struct {

// FromEnvironment returns a Config instance populated from the
// environment variables HTTP_PROXY, HTTPS_PROXY and NO_PROXY (or the
// lowercase versions thereof).
// 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
Expand Down Expand Up @@ -112,8 +114,8 @@ func getEnvAny(names ...string) string {
// 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" or a loopback address
// (with or without a port number), then a nil URL and nil error will be returned.
// 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) {
// Preprocess the Config settings for more efficient evaluation.
cfg1 := &config{
Expand All @@ -127,7 +129,8 @@ func (cfg *config) proxyForURL(reqURL *url.URL) (*url.URL, error) {
var proxy *url.URL
if reqURL.Scheme == "https" {
proxy = cfg.httpsProxy
} else if reqURL.Scheme == "http" {
}
if proxy == nil {
proxy = cfg.httpProxy
if proxy != nil && cfg.CGI {
return nil, errors.New("refusing to use HTTP_PROXY value in CGI environment; see golang.org/s/cgihttpproxy")
Expand Down Expand Up @@ -266,9 +269,6 @@ func (c *config) init() {
matchHost = true
phost = "." + phost
}
if v, err := idnaASCII(phost); err == nil {
phost = v
}
c.domainMatchers = append(c.domainMatchers, domainMatch{host: phost, port: pport, matchHost: matchHost})
}
}
Expand All @@ -292,10 +292,6 @@ func canonicalAddr(url *url.URL) string {
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
Expand Down
2 changes: 1 addition & 1 deletion agent/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ import (
"strings"

"github.com/aws/amazon-ecs-agent/agent/ecs_client/model/ecs"
"github.com/aws/amazon-ecs-agent/agent/utils/httpproxy"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/arn"
"github.com/aws/aws-sdk-go/aws/awserr"

"github.com/pkg/errors"
"golang.org/x/net/http/httpproxy"
)

func DefaultIfBlank(str string, default_value string) string {
Expand Down
1 change: 0 additions & 1 deletion agent/vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,6 @@ golang.org/x/mod/semver
golang.org/x/net/html
golang.org/x/net/html/atom
golang.org/x/net/http/httpguts
golang.org/x/net/http/httpproxy
golang.org/x/net/http2
golang.org/x/net/http2/hpack
golang.org/x/net/idna
Expand Down

0 comments on commit 822c4eb

Please sign in to comment.