From 9ebbf652bdc86cc94d103c6b738a70da38f94e22 Mon Sep 17 00:00:00 2001 From: Cory Snider Date: Tue, 6 Jan 2026 15:52:33 -0500 Subject: [PATCH] client: do not modify user-provided HTTP client The http.Client passed into client.WithHTTPClient() is modified by the constructor in-place: the value of its Transport field is mutated and wrapped in an OpenTelemetry decorator. This can lead to very surprising behaviour when a second client is constructed reusing the same http.Client value. If the http.Client is configured for TLS, the second client will fail to detect that and will incorrectly dial the Engine API socket as cleartext HTTP. Copy the provided http.Client so our modifications don't leak out to unexpected places. Signed-off-by: Cory Snider --- .golangci.yml | 5 ++++ client/client_options.go | 11 ++++++++- client/client_options_test.go | 23 +++++++++++++++++++ .../moby/moby/client/client_options.go | 11 ++++++++- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 2f4a07be1d..7dbb047082 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -293,6 +293,11 @@ linters: linters: - gosec + - text: "^G402: " # Look for bad TLS connection settings + source: "cmpopts\\.Ignore" + linters: + - gosec + # FIXME: ignoring unused assigns to ctx for now; too many hits in libnetwork/xxx functions that setup traces - text: "assigned to ctx, but never used afterwards" linters: diff --git a/client/client_options.go b/client/client_options.go index ac0b9c90f9..ae80c4e5cf 100644 --- a/client/client_options.go +++ b/client/client_options.go @@ -149,7 +149,16 @@ func WithHostFromEnv() Opt { func WithHTTPClient(client *http.Client) Opt { return func(c *clientConfig) error { if client != nil { - c.client = client + // Make a clone of client so modifications do not affect + // the caller's client. Clone here instead of in New() + // as other options (WithHost) also mutate c.client. + // Cloned clients share the same CookieJar as the + // original. + hc := *client + if ht, ok := hc.Transport.(*http.Transport); ok { + hc.Transport = ht.Clone() + } + c.client = &hc } return nil } diff --git a/client/client_options_test.go b/client/client_options_test.go index 7271926578..6f4cdc643c 100644 --- a/client/client_options_test.go +++ b/client/client_options_test.go @@ -1,12 +1,15 @@ package client import ( + "crypto/tls" "net/http" + "net/http/cookiejar" "runtime" "strings" "testing" "time" + "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -367,3 +370,23 @@ func TestWithUserAgent(t *testing.T) { assert.NilError(t, c.Close()) }) } + +func TestWithHTTPClient(t *testing.T) { + cookieJar, err := cookiejar.New(nil) + assert.NilError(t, err) + pristineHTTPClient := func() *http.Client { + return &http.Client{ + Timeout: 42 * time.Second, + Jar: cookieJar, + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ServerName: "example.com", MinVersion: tls.VersionTLS12}, + }, + } + } + hc := pristineHTTPClient() + _, err = New(WithHTTPClient(hc), WithHost("tcp://example.com:443")) + assert.NilError(t, err) + assert.DeepEqual(t, hc, pristineHTTPClient(), + cmpopts.IgnoreUnexported(http.Transport{}, tls.Config{}), + cmpopts.EquateComparable(&cookiejar.Jar{})) +} diff --git a/vendor/github.com/moby/moby/client/client_options.go b/vendor/github.com/moby/moby/client/client_options.go index ac0b9c90f9..ae80c4e5cf 100644 --- a/vendor/github.com/moby/moby/client/client_options.go +++ b/vendor/github.com/moby/moby/client/client_options.go @@ -149,7 +149,16 @@ func WithHostFromEnv() Opt { func WithHTTPClient(client *http.Client) Opt { return func(c *clientConfig) error { if client != nil { - c.client = client + // Make a clone of client so modifications do not affect + // the caller's client. Clone here instead of in New() + // as other options (WithHost) also mutate c.client. + // Cloned clients share the same CookieJar as the + // original. + hc := *client + if ht, ok := hc.Transport.(*http.Transport); ok { + hc.Transport = ht.Clone() + } + c.client = &hc } return nil }