From 9251cbc0d8d2e4b456f7812f8a3e8c9a8c13ef60 Mon Sep 17 00:00:00 2001
From: Michael Pleshakov <michael@nginx.com>
Date: Thu, 1 Sep 2022 15:48:35 -0500
Subject: [PATCH] Update handling of query params in njs

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.
---
 internal/nginx/modules/src/httpmatches.js     | 15 ++++-
 .../nginx/modules/test/httpmatches.test.js    | 64 +++++++++++++++++--
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/internal/nginx/modules/src/httpmatches.js b/internal/nginx/modules/src/httpmatches.js
index 2ff435196d..54a9efcc17 100644
--- a/internal/nginx/modules/src/httpmatches.js
+++ b/internal/nginx/modules/src/httpmatches.js
@@ -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;
     }
   }
diff --git a/internal/nginx/modules/test/httpmatches.test.js b/internal/nginx/modules/test/httpmatches.test.js
index 69c144e5a6..669638b819 100644
--- a/internal/nginx/modules/test/httpmatches.test.js
+++ b/internal/nginx/modules/test/httpmatches.test.js
@@ -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,
     },
   ];