Merge pull request #51817 from corhere/client/clone-http-client

client: do not modify user-provided HTTP client

.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:

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
 	}

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{}))
+}

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
 	}