Skip to content

Commit

Permalink
Update handling of query params in njs
Browse files Browse the repository at this point in the history
Updating NGINX to 1.23.1 image brought changes with njs-0.7.6, which now
supports examining all values of a query param.

The commit updates njs code for matching query params so that in the case
of multiple values of a query param, the first value is used. This is
consistent with the previous behavior and the Gateway API spec.

Additionally, the unit tests for 'paramsMatch' were fixed and updated
to cover multiple param values and different case of a param name.
  • Loading branch information
pleshakov committed Sep 7, 2022
1 parent f70133a commit 9251cbc
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 6 deletions.
15 changes: 13 additions & 2 deletions internal/nginx/modules/src/httpmatches.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,20 @@ function paramsMatch(requestParams, params) {
// Divide string into key value using the index.
let kv = [p.slice(0, idx), p.slice(idx + 1)];

const val = requestParams[kv[0]];
// val can either be a string or an array of strings.
// Also, the NGINX request's args object lookup is case-sensitive.
// For example, 'a=1&b=2&A=3&b=4' will be parsed into {a: "1", b: ["2", "4"], A: "3"}
let val = requestParams[kv[0]];
if (!val) {
return false;
}

// If val is an array, we will match against the first element in the array according to the Gateway API spec.
if (Array.isArray(val)) {
val = val[0];
}

if (!val || val !== kv[1]) {
if (val !== kv[1]) {
return false;
}
}
Expand Down
64 changes: 60 additions & 4 deletions internal/nginx/modules/test/httpmatches.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,25 +297,81 @@ describe('paramsMatch', () => {
{
name: 'returns false if one of the params is missing from request',
params: params,
requestParams: ['arg2=value2=SOME=other=value', 'arg3===value3&*1(*+'],
requestParams: {
// Arg1 is missing,
arg2: 'value2=SOME=other=value',
arg3: '==value3&*1(*+',
},
expected: false,
},
{
name: 'returns false if one of the params has an empty value',
params: params,
requestParams: {
Arg1: 'value1',
arg2: 'value2=SOME=other=value',
arg3: '', // empty value
},
expected: false,
},
{
name: 'returns false if one of the param values does not match',
params: params,
requestParams: ['Arg1=not-value-1', 'arg2=value2=SOME=other=value', 'arg3===value3&*1(*+'],
requestParams: {
Arg1: 'Arg1=not-value-1', // this value does not match
arg2: 'value2=SOME=other=value',
arg3: '==value3&*1(*+',
},
expected: false,
},
{
name: 'returns false if the case of one param values does not match',
params: params,
requestParams: ['Arg1=VALUE1', 'arg2=value2=SOME=other=value', 'arg3===value3&*1(*+'],
requestParams: {
Arg1: 'VALUE1', // this value is not the correct case
arg2: 'value2=SOME=other=value',
arg3: '==value3&*1(*+',
},
expected: false,
},
{
name: 'returns false if the case of one param name does not match',
params: params,
requestParams: {
Arg1: 'value1',
arg2: 'value2=SOME=other=value',
ARG3: '==value3&*1(*+', // this param name is not the correct case
},
expected: false,
},
{
name: 'returns true if all params match',
params: params,
requestParams: params,
requestParams: {
Arg1: 'value1',
arg2: 'value2=SOME=other=value',
arg3: '==value3&*1(*+',
},
expected: true,
},
{
name: 'returns true if all params match with one param having multiple values',
params: params,
requestParams: {
Arg1: ['value1', 'value2'], // 'value1' wins
arg2: 'value2=SOME=other=value',
arg3: '==value3&*1(*+',
},
expected: true,
},
{
name: 'returns false if one param does not match because of multiple values',
params: params,
requestParams: {
Arg1: ['value2', 'value1'], // 'value2' wins but it does not match
arg2: 'value2=SOME=other=value',
arg3: '==value3&*1(*+',
},
expected: false,
},
];
Expand Down

0 comments on commit 9251cbc

Please sign in to comment.