Merge pull request moby#52204 from thaJeztah/stable_customheaders

client: WithHTTPHeaders: improve doc, add test coverage, and error on duplicate headers

Author: thaJeztah

client/client_options.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/docker/go-connections/sockets"
 	"github.com/docker/go-connections/tlsconfig"
 	"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp"
@@ -194,11 +195,23 @@ func WithUserAgent(ua string) Opt {
 }
 
 // WithHTTPHeaders appends custom HTTP headers to the client's default headers.
-// It does not allow for built-in headers (such as "User-Agent", if set) to
-// be overridden. Also see [WithUserAgent].
+// It does not allow overriding built-in headers (such as "User-Agent").
+// Also see [WithUserAgent].
+//
+// It replaces any existing custom headers. Keys are case-insensitive and
+// canonicalized using [http.CanonicalHeaderKey]. If multiple entries map
+// to the same canonical key, a [cerrdefs.ErrInvalidArgument] is returned.
 func WithHTTPHeaders(headers map[string]string) Opt {
 	return func(c *clientConfig) error {
-		c.customHTTPHeaders = headers
+		c.customHTTPHeaders = make(map[string]string)
+		for k, v := range headers {
+			k = http.CanonicalHeaderKey(k)
+			_, ok := c.customHTTPHeaders[k]
+			if ok {
+				return cerrdefs.ErrInvalidArgument.WithMessage(fmt.Sprintf("duplicate custom HTTP header (%s)", k))
+			}
+			c.customHTTPHeaders[k] = v
+		}
 		return nil
 	}
 }

client/client_options_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	cerrdefs "github.com/containerd/errdefs"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
@@ -371,6 +372,38 @@ func TestWithUserAgent(t *testing.T) {
 	})
 }
 
+func TestWithHTTTPHeaders(t *testing.T) {
+	t.Run("duplicates", func(t *testing.T) {
+		c, err := New(
+			WithHTTPHeaders(map[string]string{
+				"custom-header": "custom-value-A",
+				"Custom-Header": "custom-value-B",
+				"CUSTOM-HEADER": "custom-value-C",
+			}),
+		)
+		assert.Check(t, is.ErrorIs(err, cerrdefs.ErrInvalidArgument))
+		assert.Check(t, is.Error(err, "duplicate custom HTTP header (Custom-Header)"))
+		assert.Check(t, is.Nil(c))
+	})
+	t.Run("multiple", func(t *testing.T) {
+		c, err := New(
+			WithHTTPHeaders(map[string]string{"custom-header-1": "hello"}),
+			WithHTTPHeaders(map[string]string{"custom-header-2": "custom-value-A"}),
+			WithHTTPHeaders(map[string]string{"Custom-Header-2": "custom-value-B"}),
+			WithHTTPHeaders(map[string]string{"CUSTOM-HEADER-2": "custom-value-C"}),
+			WithBaseMockClient(func(req *http.Request) (*http.Response, error) {
+				assert.Check(t, is.Equal(req.Header.Get("Custom-Header-1"), ""), "custom-header-1 should've been replaced")
+				assert.Check(t, is.Equal(req.Header.Get("Custom-Header-2"), "custom-value-C"))
+				return &http.Response{StatusCode: http.StatusOK}, nil
+			}),
+		)
+		assert.NilError(t, err)
+		_, err = c.Ping(t.Context(), PingOptions{})
+		assert.NilError(t, err)
+		assert.NilError(t, c.Close())
+	})
+}
+
 func TestWithHTTPClient(t *testing.T) {
 	cookieJar, err := cookiejar.New(nil)
 	assert.NilError(t, err)