From 462356cb4fa259570793939ea18762492645e2bb Mon Sep 17 00:00:00 2001 From: Cameron Sparr Date: Fri, 27 Jan 2023 16:16:35 -0800 Subject: [PATCH] Pin httpproxy package 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: https://github.com/golang/go/issues/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. --- .../x/net/http => utils}/httpproxy/proxy.go | 20 ++++++++----------- agent/utils/utils.go | 2 +- agent/vendor/modules.txt | 1 - 3 files changed, 9 insertions(+), 14 deletions(-) rename agent/{vendor/golang.org/x/net/http => utils}/httpproxy/proxy.go (94%) diff --git a/agent/vendor/golang.org/x/net/http/httpproxy/proxy.go b/agent/utils/httpproxy/proxy.go similarity index 94% rename from agent/vendor/golang.org/x/net/http/httpproxy/proxy.go rename to agent/utils/httpproxy/proxy.go index c3bd9a1eeb..889ea57106 100644 --- a/agent/vendor/golang.org/x/net/http/httpproxy/proxy.go +++ b/agent/utils/httpproxy/proxy.go @@ -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 @@ -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 @@ -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{ @@ -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") @@ -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}) } } @@ -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 diff --git a/agent/utils/utils.go b/agent/utils/utils.go index c5faa53c8f..95abb93364 100644 --- a/agent/utils/utils.go +++ b/agent/utils/utils.go @@ -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 { diff --git a/agent/vendor/modules.txt b/agent/vendor/modules.txt index c71c4dc662..685e07b409 100644 --- a/agent/vendor/modules.txt +++ b/agent/vendor/modules.txt @@ -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