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) => {