Skip to content

Handle repeated headers #133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion internal/nginx/config/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"

"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/gateway-api/apis/v1alpha2"
Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions internal/nginx/config/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down
10 changes: 7 additions & 3 deletions internal/nginx/modules/src/httpmatches.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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;
}
}
Expand Down
8 changes: 8 additions & 0 deletions internal/nginx/modules/test/httpmatches.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down