From 5a735264b30d29f27bcf28f5e3a908b0711e1d39 Mon Sep 17 00:00:00 2001
From: Kenneth Jenkins <51246568+kenjenkins@users.noreply.github.com>
Date: Wed, 18 Oct 2023 13:55:48 -0700
Subject: [PATCH] config: do not add route headers to global map (#4629)

Currently the GetSetResponseHeadersForPolicy() method may add entries to
the global SetResponseHeaders map, which can lead to one route's headers
being applied to other routes.

Instead, make a copy of the SetResponseHeaders map before adding any
route-specific response header entries.

Add additional unit tests for GetSetResponseHeaders() and
GetSetResponseHeadersForPolicy().
---
 config/options.go      |  9 ++++---
 config/options_test.go | 56 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/config/options.go b/config/options.go
index f48af08ce..d651c5b1f 100644
--- a/config/options.go
+++ b/config/options.go
@@ -1121,9 +1121,12 @@ func (o *Options) GetSetResponseHeaders() map[string]string {
 
 // GetSetResponseHeadersForPolicy gets the SetResponseHeaders for a policy.
 func (o *Options) GetSetResponseHeadersForPolicy(policy *Policy) map[string]string {
-	hdrs := o.SetResponseHeaders
-	if hdrs == nil {
-		hdrs = make(map[string]string)
+	hdrs := make(map[string]string)
+	for k, v := range o.SetResponseHeaders {
+		hdrs[k] = v
+	}
+
+	if o.SetResponseHeaders == nil {
 		for k, v := range defaultSetResponseHeaders {
 			hdrs[k] = v
 		}
diff --git a/config/options_test.go b/config/options_test.go
index 9f1dd54c7..e770cdce6 100644
--- a/config/options_test.go
+++ b/config/options_test.go
@@ -978,6 +978,18 @@ func TestOptions_GetSetResponseHeaders(t *testing.T) {
 		options.SetResponseHeaders = map[string]string{DisableHeaderKey: "1", "x-other": "xyz"}
 		assert.Equal(t, map[string]string{}, options.GetSetResponseHeaders())
 	})
+	t.Run("empty", func(t *testing.T) {
+		options := NewDefaultOptions()
+		options.SetResponseHeaders = map[string]string{}
+		assert.Equal(t, map[string]string{}, options.GetSetResponseHeaders())
+	})
+	t.Run("no partial defaults", func(t *testing.T) {
+		options := NewDefaultOptions()
+		options.Cert = "CERT"
+		options.SetResponseHeaders = map[string]string{"X-Frame-Options": "DENY"}
+		assert.Equal(t, map[string]string{"X-Frame-Options": "DENY"},
+			options.GetSetResponseHeaders())
+	})
 }
 
 func TestOptions_GetSetResponseHeadersForPolicy(t *testing.T) {
@@ -989,6 +1001,50 @@ func TestOptions_GetSetResponseHeadersForPolicy(t *testing.T) {
 		}
 		assert.Equal(t, map[string]string{"x": "y"}, options.GetSetResponseHeadersForPolicy(policy))
 	})
+	t.Run("global defaults plus policy", func(t *testing.T) {
+		options := NewDefaultOptions()
+		options.Cert = "CERT"
+		policy := &Policy{
+			SetResponseHeaders: map[string]string{"Route": "xyz"},
+		}
+		assert.Equal(t, map[string]string{
+			"Route":                     "xyz",
+			"Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload",
+			"X-Frame-Options":           "SAMEORIGIN",
+			"X-XSS-Protection":          "1; mode=block",
+		}, options.GetSetResponseHeadersForPolicy(policy))
+	})
+	t.Run("global defaults partial override", func(t *testing.T) {
+		options := NewDefaultOptions()
+		options.Cert = "CERT"
+		policy := &Policy{
+			SetResponseHeaders: map[string]string{"X-Frame-Options": "DENY"},
+		}
+		assert.Equal(t, map[string]string{
+			"Strict-Transport-Security": "max-age=31536000; includeSubDomains; preload",
+			"X-Frame-Options":           "DENY",
+			"X-XSS-Protection":          "1; mode=block",
+		}, options.GetSetResponseHeadersForPolicy(policy))
+	})
+	t.Run("multiple policies", func(t *testing.T) {
+		options := NewDefaultOptions()
+		options.SetResponseHeaders = map[string]string{"global": "foo"}
+		p1 := &Policy{
+			SetResponseHeaders: map[string]string{"route-1": "bar"},
+		}
+		p2 := &Policy{
+			SetResponseHeaders: map[string]string{"route-2": "baz"},
+		}
+		assert.Equal(t, map[string]string{
+			"global":  "foo",
+			"route-1": "bar",
+		}, options.GetSetResponseHeadersForPolicy(p1))
+		assert.Equal(t, map[string]string{
+			"global":  "foo",
+			"route-2": "baz",
+		}, options.GetSetResponseHeadersForPolicy(p2))
+		assert.Equal(t, map[string]string{"global": "foo"}, options.GetSetResponseHeaders())
+	})
 }
 
 func TestOptions_GetSharedKey(t *testing.T) {