From 86970eb92439c924945169a85278cec3b68229d9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 21 Jul 2021 18:19:16 +0200 Subject: [PATCH] sockets: add TCPProxyFromEnvironment to keep pre-go1.16 behavior TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the pre-go1.16 behavior for URLs using the 'tcp://' scheme. Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other scheme would use HTTP_PROXY. However, https://github.com/golang/net/commit/7b1cca2348c07eb09fef635269c8e01611260f9f (per a request in golang/go#40909) changed this behavior to only use HTTP_PROXY for `http://` schemes, no longer using a proxy for any other scheme. Docker uses the `tcp://` scheme as a default for API connections, to indicate that the API is not "purely" HTTP. Various parts in the code also *require* this scheme to be used. While we could change the default and allow http(s) schemes to be used, doing so will take time, taking into account that there are many installs in existence that have tcp:// configured as DOCKER_HOST. This function detects if the `tcp://` scheme is used; if it is, it creates a shallow copy of req, containing just the URL, and overrides the scheme with 'http', which should be sufficient to perform proxy detection. For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without altering the request. Signed-off-by: Sebastiaan van Stijn --- sockets/sockets.go | 34 +++++++++++- sockets/sockets_test.go | 113 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 sockets/sockets_test.go diff --git a/sockets/sockets.go b/sockets/sockets.go index 73ff9fcb7..72824dc11 100644 --- a/sockets/sockets.go +++ b/sockets/sockets.go @@ -4,6 +4,7 @@ package sockets import ( "errors" "net/http" + "net/url" "time" ) @@ -23,7 +24,38 @@ func ConfigureTransport(tr *http.Transport, proto, addr string) error { case "npipe": return configureNpipeTransport(tr, proto, addr) default: - tr.Proxy = http.ProxyFromEnvironment + tr.Proxy = TCPProxyFromEnvironment } return nil } + +// TCPProxyFromEnvironment wraps http.ProxyFromEnvironment, to preserve the +// pre-go1.16 behavior for URLs using the 'tcp://' scheme. +// +// Prior to go1.16, `https://` schemes would use HTTPS_PROXY, and any other +// scheme would use HTTP_PROXY. However, https://github.com/golang/net/commit/7b1cca2348c07eb09fef635269c8e01611260f9f +// (per a request in golang/go#40909) changed this behavior to only use +// HTTP_PROXY for `http://` schemes, no longer using a proxy for any other +// scheme. +// +// Docker uses the `tcp://` scheme as a default for API connections, to indicate +// that the API is not "purely" HTTP. Various parts in the code also *require* +// this scheme to be used. While we could change the default and allow http(s) +// schemes to be used, doing so will take time, taking into account that there +// are many installs in existence that have tcp:// configured as DOCKER_HOST. +// +// This function detects if the `tcp://` scheme is used; if it is, it creates +// a shallow copy of req, containing just the URL, and overrides the scheme with +// 'http', which should be sufficient to perform proxy detection. +// For other (non-'tcp://') schemes, http.ProxyFromEnvironment is called without +// altering the request. +func TCPProxyFromEnvironment(req *http.Request) (*url.URL, error) { + if req.URL.Scheme != "tcp" { + return http.ProxyFromEnvironment(req) + } + u := req.URL + if u.Scheme == "tcp" { + u.Scheme = "http" + } + return http.ProxyFromEnvironment(&http.Request{URL: u}) +} diff --git a/sockets/sockets_test.go b/sockets/sockets_test.go new file mode 100644 index 000000000..05460c15f --- /dev/null +++ b/sockets/sockets_test.go @@ -0,0 +1,113 @@ +package sockets + +import ( + "net/http" + "net/url" + "os" + "testing" +) + +var ( + httpProxy = "http://proxy.example.com" + httpsProxy = "https://proxy.example.com" +) + +func TestConfigureTransportProxy(t *testing.T) { + // roughly based on defaultHTTPClient in the docker client + u := &url.URL{ + Scheme: "tcp", + Host: "docker.acme.example.com", + } + transport := new(http.Transport) + err := ConfigureTransport(transport, u.Scheme, u.Host) + if err != nil { + t.Fatal(err) + } + if err := os.Setenv("HTTP_PROXY", httpProxy); err != nil { + t.Fatal(err) + } + if err := os.Setenv("HTTPS_PROXY", httpsProxy); err != nil { + t.Fatal(err) + } + defer func() { + _ = os.Unsetenv("HTTP_PROXY") + _ = os.Unsetenv("HTTPS_PROXY") + }() + + request, err := http.NewRequest(http.MethodGet, "tcp://docker.acme.example.com:2376", nil) + if err != nil { + t.Fatal(err) + } + proxyURL, err := transport.Proxy(request) + if err != nil { + t.Fatal(err) + } + if proxyURL.String() != httpProxy { + t.Fatalf("expected %s, got %s", httpProxy, proxyURL) + } +} + +func TestTCPProxyFromEnvironment(t *testing.T) { + if err := os.Setenv("HTTP_PROXY", httpProxy); err != nil { + t.Fatal(err) + } + if err := os.Setenv("HTTPS_PROXY", httpsProxy); err != nil { + t.Fatal(err) + } + defer func() { + _ = os.Unsetenv("HTTP_PROXY") + _ = os.Unsetenv("HTTPS_PROXY") + }() + + tests := []struct { + url string + expected *string + }{ + { + url: "example.com", + expected: nil, + }, + { + url: "ftp://example.com", + expected: nil, + }, + { + url: "unix:///var/run/docker.sock", + expected: nil, + }, + { + url: "tcp://example.com:2376", + expected: &httpProxy, + }, + { + url: "http://example.com:2375", + expected: &httpProxy, + }, + { + url: "https://example.com:2376", + expected: &httpsProxy, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.url, func(t *testing.T) { + request, err := http.NewRequest(http.MethodGet, tc.url, nil) + if err != nil { + t.Fatal(err) + } + + proxyURL, err := TCPProxyFromEnvironment(request) + if err != nil { + t.Fatal(err) + } + if tc.expected == nil { + if proxyURL != nil { + t.Fatalf("expected no proxy, got %s", proxyURL) + } + } else if proxyURL.String() != *tc.expected { + t.Fatalf("expected %s, got %s", *tc.expected, proxyURL) + } + }) + } +}