From fe600235c92a351d8278120b86123292da4df909 Mon Sep 17 00:00:00 2001
From: Kate Osborn <k.osborn@f5.com>
Date: Fri, 17 Jun 2022 12:37:03 -0600
Subject: [PATCH] Handle duplicate header names and values

If an HTTPHeaderMatch contains entries with equivalent header names, we will only configure
the first entry. We will ignore all other entries with that name. Header names are
case-insensitive, so "Foo" is equivalent to "foo".

If an HTTP request has repeated headers, nginx will merge the values into a comma delimited string.
For example, -H "foo:bar" -H "foo:baz" becomes {"foo":"bar,baz"} in the headersIn object.
In this case, a request satisfies the header match {"foo":"bar"} if one of the values specified
in the request header "foo" is "bar".
---
 internal/nginx/config/generator.go            | 10 +++++++++-
 internal/nginx/config/generator_test.go       | 20 +++++++++++++++++++
 internal/nginx/modules/src/httpmatches.js     | 10 +++++++---
 .../nginx/modules/test/httpmatches.test.js    |  8 ++++++++
 4 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/internal/nginx/config/generator.go b/internal/nginx/config/generator.go
index ce2937501f..77d5236eef 100644
--- a/internal/nginx/config/generator.go
+++ b/internal/nginx/config/generator.go
@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
@@ -193,11 +194,18 @@ func createHTTPMatch(match v1alpha2.HTTPRouteMatch, redirectPath string) httpMat
 
 	if match.Headers != nil {
 		headers := make([]string, 0, len(match.Headers))
+		headerNames := make(map[string]struct{})
 
 		// FIXME(kate-osborn): For now we only support type "Exact".
 		for _, h := range match.Headers {
 			if *h.Type == v1alpha2.HeaderMatchExact {
-				headers = append(headers, createHeaderKeyValString(h))
+				// duplicate header names are not permitted by the spec
+				// only configure the first entry for every header name (case-insensitive)
+				lowerName := strings.ToLower(string(h.Name))
+				if _, ok := headerNames[lowerName]; !ok {
+					headers = append(headers, createHeaderKeyValString(h))
+					headerNames[lowerName] = struct{}{}
+				}
 			}
 		}
 		hm.Headers = headers
diff --git a/internal/nginx/config/generator_test.go b/internal/nginx/config/generator_test.go
index 0b698b539b..a27630195b 100644
--- a/internal/nginx/config/generator_test.go
+++ b/internal/nginx/config/generator_test.go
@@ -640,6 +640,16 @@ func TestCreateHTTPMatch(t *testing.T) {
 			Value: "val-3",
 		},
 	}
+
+	testDuplicateHeaders := make([]v1alpha2.HTTPHeaderMatch, 0, 5)
+	duplicateHeaderMatch := v1alpha2.HTTPHeaderMatch{
+		Type:  helpers.GetHeaderMatchTypePointer(v1alpha2.HeaderMatchExact),
+		Name:  "HEADER-2", // header names are case-insensitive
+		Value: "val-2",
+	}
+	testDuplicateHeaders = append(testDuplicateHeaders, testHeaderMatches...)
+	testDuplicateHeaders = append(testDuplicateHeaders, duplicateHeaderMatch)
+
 	testQueryParamMatches := []v1alpha2.HTTPQueryParamMatch{
 		{
 			Type:  helpers.GetQueryParamMatchTypePointer(v1alpha2.QueryParamMatchExact),
@@ -763,6 +773,16 @@ func TestCreateHTTPMatch(t *testing.T) {
 			},
 			msg: "method, headers, and query params match",
 		},
+		{
+			match: v1alpha2.HTTPRouteMatch{
+				Headers: testDuplicateHeaders,
+			},
+			expected: httpMatch{
+				Headers:      expectedHeaders,
+				RedirectPath: testPath,
+			},
+			msg: "duplicate header names",
+		},
 	}
 	for _, tc := range tests {
 		result := createHTTPMatch(tc.match, testPath)
diff --git a/internal/nginx/modules/src/httpmatches.js b/internal/nginx/modules/src/httpmatches.js
index 56adf29415..2ff435196d 100644
--- a/internal/nginx/modules/src/httpmatches.js
+++ b/internal/nginx/modules/src/httpmatches.js
@@ -129,8 +129,6 @@ function testMatch(r, match) {
   return true;
 }
 
-// FIXME(osborn): Need to add special handling for repeated headers.
-// Should follow guidance from https://www.rfc-editor.org/rfc/rfc7230.html#section-3.2.2.
 function headersMatch(requestHeaders, headers) {
   for (let i = 0; i < headers.length; i++) {
     const h = headers[i];
@@ -144,7 +142,13 @@ function headersMatch(requestHeaders, headers) {
     // This means that requestHeaders['FOO'] is equivalent to requestHeaders['foo'].
     let val = requestHeaders[kv[0]];
 
-    if (!val || val !== kv[1]) {
+    if (!val) {
+      return false;
+    }
+
+    // split on comma because nginx uses commas to delimit multiple header values
+    const values = val.split(',');
+    if (!values.includes(kv[1])) {
       return false;
     }
   }
diff --git a/internal/nginx/modules/test/httpmatches.test.js b/internal/nginx/modules/test/httpmatches.test.js
index cb7a6c7550..eec8199b1a 100644
--- a/internal/nginx/modules/test/httpmatches.test.js
+++ b/internal/nginx/modules/test/httpmatches.test.js
@@ -253,6 +253,14 @@ describe('headersMatch', () => {
       },
       expected: true,
     },
+    {
+      name: 'returns true if request has multiple values for a header name and one value matches ',
+      headers: ['multiValueHeader:val3'],
+      requestHeaders: {
+        multiValueHeader: 'val1,val2,val3,val4,val5',
+      },
+      expected: true,
+    },
   ];
 
   tests.forEach((test) => {