From 4d1a6a43cd5a67ef2f8ec79f827d8619a7cc79ad Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 11 Oct 2016 15:53:14 -0700 Subject: [PATCH] client: deterministically resolve http scheme The docker client has historically used Transport.TLSClientConfig to set the scheme for the API client. A recent moved the resolution to use the http.Transport directly, rather than save the TLSClientConfig state on a client struct. This caused issues when mutliple calls made with a single client would have this field set in the http package on pre-1.7 installations. This fix detects the presence of the TLSClientConfig once and sets the scheme accordingly. We still don't know why this issue doesn't happen with Go 1.7 but it must be more deterministic in the newer version. Signed-off-by: Stephen J Day --- client.go | 14 ++++++++++++++ request.go | 8 +++----- transport.go | 17 ----------------- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/client.go b/client.go index 025eaaf9a9..75073881c7 100644 --- a/client.go +++ b/client.go @@ -63,6 +63,8 @@ const DefaultVersion string = "1.23" // Client is the API client that performs all operations // against a docker server. type Client struct { + // scheme sets the scheme for the client + scheme string // host holds the server address to connect to host string // proto holds the client protocol i.e. unix. @@ -143,7 +145,19 @@ func NewClient(host string, version string, client *http.Client, httpHeaders map } } + scheme := "http" + tlsConfig := resolveTLSConfig(client.Transport) + if tlsConfig != nil { + // TODO(stevvooe): This isn't really the right way to write clients in Go. + // `NewClient` should probably only take an `*http.Client` and work from there. + // Unfortunately, the model of having a host-ish/url-thingy as the connection + // string has us confusing protocol and transport layers. We continue doing + // this to avoid breaking existing clients but this should be addressed. + scheme = "https" + } + return &Client{ + scheme: scheme, host: host, proto: proto, addr: addr, diff --git a/request.go b/request.go index d585b46ab1..bfd62bad1e 100644 --- a/request.go +++ b/request.go @@ -100,10 +100,8 @@ func (cli *Client) sendClientRequest(ctx context.Context, method, path string, q req.Host = "docker" } - scheme := resolveScheme(cli.client.Transport) - req.URL.Host = cli.addr - req.URL.Scheme = scheme + req.URL.Scheme = cli.scheme if expectedPayload && req.Header.Get("Content-Type") == "" { req.Header.Set("Content-Type", "text/plain") @@ -111,11 +109,11 @@ func (cli *Client) sendClientRequest(ctx context.Context, method, path string, q resp, err := ctxhttp.Do(ctx, cli.client, req) if err != nil { - if scheme != "https" && strings.Contains(err.Error(), "malformed HTTP response") { + if cli.scheme != "https" && strings.Contains(err.Error(), "malformed HTTP response") { return serverResp, fmt.Errorf("%v.\n* Are you trying to connect to a TLS-enabled daemon without TLS?", err) } - if scheme == "https" && strings.Contains(err.Error(), "bad certificate") { + if cli.scheme == "https" && strings.Contains(err.Error(), "bad certificate") { return serverResp, fmt.Errorf("The server probably has client authentication (--tlsverify) enabled. Please check your TLS client certification settings: %v", err) } diff --git a/transport.go b/transport.go index 771d76f06b..f04e601649 100644 --- a/transport.go +++ b/transport.go @@ -26,20 +26,3 @@ func resolveTLSConfig(transport http.RoundTripper) *tls.Config { return nil } } - -// resolveScheme detects a tls config on the transport and returns the -// appropriate http scheme. -// -// TODO(stevvooe): This isn't really the right way to write clients in Go. -// `NewClient` should probably only take an `*http.Client` and work from there. -// Unfortunately, the model of having a host-ish/url-thingy as the connection -// string has us confusing protocol and transport layers. We continue doing -// this to avoid breaking existing clients but this should be addressed. -func resolveScheme(transport http.RoundTripper) string { - c := resolveTLSConfig(transport) - if c != nil { - return "https" - } - - return "http" -}