From d132ddfa41a708e871cb71b4beac29fcba9a0f38 Mon Sep 17 00:00:00 2001 From: Mathieu Parent Date: Tue, 5 Dec 2023 11:45:39 +0100 Subject: [PATCH] feat(httpclient): support appending the same header multiple times See https://github.com/grafana/grafana-plugin-sdk-go/issues/829 --- backend/http_settings.go | 9 +++++---- backend/http_settings_test.go | 17 ++++++++++------- .../httpclient/custom_headers_middleware.go | 10 ++++++---- .../custom_headers_middleware_test.go | 18 +++++++++--------- backend/httpclient/options.go | 2 +- experimental/authclient/authclient_test.go | 10 +++++----- 6 files changed, 36 insertions(+), 30 deletions(-) diff --git a/backend/http_settings.go b/backend/http_settings.go index 7720f6c03..5fa844cbc 100644 --- a/backend/http_settings.go +++ b/backend/http_settings.go @@ -3,6 +3,7 @@ package backend import ( "encoding/json" "fmt" + "net/http" "time" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" @@ -16,7 +17,7 @@ type HTTPSettings struct { BasicAuthEnabled bool BasicAuthUser string BasicAuthPassword string - Headers map[string]string + Header http.Header Timeout time.Duration DialTimeout time.Duration @@ -52,7 +53,7 @@ type HTTPSettings struct { // HTTPClientOptions creates and returns httpclient.Options. func (s *HTTPSettings) HTTPClientOptions() httpclient.Options { opts := httpclient.Options{ - Headers: s.Headers, + Header: s.Header, Labels: map[string]string{}, CustomOptions: map[string]interface{}{}, } @@ -104,7 +105,7 @@ func (s *HTTPSettings) HTTPClientOptions() httpclient.Options { //gocyclo:ignore func parseHTTPSettings(jsonData json.RawMessage, secureJSONData map[string]string) (*HTTPSettings, error) { s := &HTTPSettings{ - Headers: map[string]string{}, + Header: http.Header{}, } var dat map[string]interface{} @@ -273,7 +274,7 @@ func parseHTTPSettings(jsonData json.RawMessage, secureJSONData map[string]strin if key, exists := dat[headerNameSuffix]; exists { if value, exists := secureJSONData[headerValueSuffix]; exists { - s.Headers[key.(string)] = value + s.Header.Add(key.(string), value) } } else { // No (more) header values are available diff --git a/backend/http_settings_test.go b/backend/http_settings_test.go index e5835d34e..b49bd4399 100644 --- a/backend/http_settings_test.go +++ b/backend/http_settings_test.go @@ -2,6 +2,7 @@ package backend import ( "encoding/json" + "net/http" "testing" "time" @@ -44,7 +45,8 @@ func TestParseHTTPSettings(t *testing.T) { "sigV4ExternalId": "ext123", "sigV4Profile": "ghi", "httpHeaderName1": "X-HeaderOne", - "httpHeaderName2": "X-HeaderTwo" + "httpHeaderName2": "X-HeaderTwo", + "httpHeaderName3": "X-HeaderTwo" }` secureData := map[string]string{ "basicAuthPassword": "pwd", @@ -55,6 +57,7 @@ func TestParseHTTPSettings(t *testing.T) { "sigV4SecretKey": "sigV4SecretKey5", "httpHeaderValue1": "SecretOne", "httpHeaderValue2": "SecretTwo", + "httpHeaderValue3": "SecretThree", } var jsonMap map[string]interface{} err := json.Unmarshal([]byte(jsonStr), &jsonMap) @@ -68,9 +71,9 @@ func TestParseHTTPSettings(t *testing.T) { BasicAuthEnabled: true, BasicAuthUser: "user", BasicAuthPassword: "pwd", - Headers: map[string]string{ - "X-HeaderOne": "SecretOne", - "X-HeaderTwo": "SecretTwo", + Header: http.Header{ + "X-Headerone": {"SecretOne"}, + "X-Headertwo": {"SecretTwo", "SecretThree"}, }, Timeout: 10 * time.Second, DialTimeout: 10 * time.Second, @@ -108,9 +111,9 @@ func TestParseHTTPSettings(t *testing.T) { User: "user", Password: "pwd", }, - Headers: map[string]string{ - "X-HeaderOne": "SecretOne", - "X-HeaderTwo": "SecretTwo", + Header: http.Header{ + "X-Headerone": {"SecretOne"}, + "X-Headertwo": {"SecretTwo", "SecretThree"}, }, Timeouts: &httpclient.TimeoutOptions{ Timeout: 10 * time.Second, diff --git a/backend/httpclient/custom_headers_middleware.go b/backend/httpclient/custom_headers_middleware.go index 27771a2f8..85d0c4dc7 100644 --- a/backend/httpclient/custom_headers_middleware.go +++ b/backend/httpclient/custom_headers_middleware.go @@ -12,17 +12,19 @@ const CustomHeadersMiddlewareName = "CustomHeaders" // If opts.Headers is empty, next will be returned. func CustomHeadersMiddleware() Middleware { return NamedMiddlewareFunc(CustomHeadersMiddlewareName, func(opts Options, next http.RoundTripper) http.RoundTripper { - if len(opts.Headers) == 0 { + if len(opts.Header) == 0 { return next } return RoundTripperFunc(func(req *http.Request) (*http.Response, error) { - for key, value := range opts.Headers { + for key, values := range opts.Header { // According to https://pkg.go.dev/net/http#Request.Header, Host is a special case if http.CanonicalHeaderKey(key) == "Host" { - req.Host = value + req.Host = values[0] } else { - req.Header.Set(key, value) + for _, value := range values { + req.Header.Add(key, value) + } } } diff --git a/backend/httpclient/custom_headers_middleware_test.go b/backend/httpclient/custom_headers_middleware_test.go index 14b94bbfc..ac7e18e91 100644 --- a/backend/httpclient/custom_headers_middleware_test.go +++ b/backend/httpclient/custom_headers_middleware_test.go @@ -34,10 +34,10 @@ func TestCustomHeadersMiddleware(t *testing.T) { ctx := &testContext{} finalRoundTripper := ctx.createRoundTripper("final") customHeaders := CustomHeadersMiddleware() - rt := customHeaders.CreateMiddleware(Options{Headers: map[string]string{ - "X-HeaderOne": "ValueOne", - "X-HeaderTwo": "ValueTwo", - "X-HeaderThree": "ValueThree", + rt := customHeaders.CreateMiddleware(Options{Header: http.Header{ + "X-Headerone": {"ValueOne"}, + "X-Headertwo": {"ValueTwo"}, + "X-Headerthree": {"ValueThree", "ValueThreeAgain"}, }}, finalRoundTripper) require.NotNil(t, rt) middlewareName, ok := customHeaders.(MiddlewareName) @@ -55,17 +55,17 @@ func TestCustomHeadersMiddleware(t *testing.T) { require.Len(t, ctx.callChain, 1) require.ElementsMatch(t, []string{"final"}, ctx.callChain) - require.Equal(t, "ValueOne", req.Header.Get("X-HeaderOne")) - require.Equal(t, "ValueTwo", req.Header.Get("X-HeaderTwo")) - require.Equal(t, "ValueThree", req.Header.Get("X-HeaderThree")) + require.Equal(t, []string{"ValueOne"}, req.Header.Values("X-Headerone")) + require.Equal(t, []string{"ValueTwo"}, req.Header.Values("X-Headertwo")) + require.Equal(t, []string{"ValueThree", "ValueThreeAgain"}, req.Header.Values("X-Headerthree")) }) t.Run("With custom Host header set should apply Host to the request", func(t *testing.T) { ctx := &testContext{} finalRoundTripper := ctx.createRoundTripper("final") customHeaders := CustomHeadersMiddleware() - rt := customHeaders.CreateMiddleware(Options{Headers: map[string]string{ - "Host": "example.com", + rt := customHeaders.CreateMiddleware(Options{Header: http.Header{ + "Host": {"example.com"}, }}, finalRoundTripper) require.NotNil(t, rt) middlewareName, ok := customHeaders.(MiddlewareName) diff --git a/backend/httpclient/options.go b/backend/httpclient/options.go index 0c494dda9..5ec995216 100644 --- a/backend/httpclient/options.go +++ b/backend/httpclient/options.go @@ -38,7 +38,7 @@ type Options struct { ProxyOptions *proxy.Options // Headers custom headers. - Headers map[string]string + Header http.Header // CustomOptions allows custom options to be provided. CustomOptions map[string]interface{} diff --git a/experimental/authclient/authclient_test.go b/experimental/authclient/authclient_test.go index d13b86836..12bb17bd6 100644 --- a/experimental/authclient/authclient_test.go +++ b/experimental/authclient/authclient_test.go @@ -31,7 +31,7 @@ func TestNew(t *testing.T) { t.Run("client credentials", func(t *testing.T) { t.Run("valid client credentials", func(t *testing.T) { hc, err := authclient.New(httpclient.Options{ - Headers: map[string]string{"h1": "v1"}, + Header: http.Header{"H1": {"v1"}}, }, authclient.AuthOptions{ AuthMethod: authclient.AuthMethodOAuth2, OAuth2Options: &authclient.OAuth2Options{ @@ -54,7 +54,7 @@ func TestNew(t *testing.T) { }) t.Run("valid client credentials with basic auth settings", func(t *testing.T) { hc, err := authclient.New(httpclient.Options{ - Headers: map[string]string{"h1": "v1"}, + Header: http.Header{"H1": {"v1"}}, BasicAuth: &httpclient.BasicAuthOptions{User: "userFoo", Password: "pwdBar"}, }, authclient.AuthOptions{ AuthMethod: authclient.AuthMethodOAuth2, @@ -81,7 +81,7 @@ func TestNew(t *testing.T) { t.Run("invalid private key", func(t *testing.T) { privateKey := generateKey(t, true) hc, err := authclient.New(httpclient.Options{ - Headers: map[string]string{"h1": "v1"}, + Header: http.Header{"H1": {"v1"}}, }, authclient.AuthOptions{ AuthMethod: authclient.AuthMethodOAuth2, OAuth2Options: &authclient.OAuth2Options{ @@ -103,7 +103,7 @@ func TestNew(t *testing.T) { t.Run("valid private key", func(t *testing.T) { privateKey := generateKey(t, false) hc, err := authclient.New(httpclient.Options{ - Headers: map[string]string{"h1": "v1"}, + Header: http.Header{"H1": {"v1"}}, }, authclient.AuthOptions{ AuthMethod: authclient.AuthMethodOAuth2, OAuth2Options: &authclient.OAuth2Options{ @@ -134,7 +134,7 @@ func getOAuthServer(t *testing.T) *httptest.Server { return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { oAuth2TokenValue := "foo" t.Run("ensure custom headers propagated correctly", func(t *testing.T) { - require.Equal(t, "v1", r.Header.Get("h1")) + require.Equal(t, "v1", r.Header.Get("H1")) }) if r.URL.String() == handlerToken { w.Header().Set("Content-Type", "application/json")