From ed7e3c906ab7ab8a91b5ce4d4345c46e4a351d2a Mon Sep 17 00:00:00 2001
From: Aziz Rmadi <46684200+armadi1809@users.noreply.github.com>
Date: Sun, 21 Jan 2024 20:36:44 -0600
Subject: [PATCH] matchers: `query` now ANDs multiple keys (#6054)

Co-authored-by: Francis Lavoie <lavofr@gmail.com>
---
 modules/caddyhttp/matchers.go      | 27 ++++++++++++++++------
 modules/caddyhttp/matchers_test.go | 36 ++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/modules/caddyhttp/matchers.go b/modules/caddyhttp/matchers.go
index b3859797..93211b22 100644
--- a/modules/caddyhttp/matchers.go
+++ b/modules/caddyhttp/matchers.go
@@ -33,6 +33,7 @@ import (
 	"github.com/google/cel-go/cel"
 	"github.com/google/cel-go/common/types"
 	"github.com/google/cel-go/common/types/ref"
+	"golang.org/x/exp/slices"
 
 	"github.com/caddyserver/caddy/v2"
 	"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
@@ -789,6 +790,12 @@ func (m *MatchQuery) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
 
 // Match returns true if r matches m. An empty m matches an empty query string.
 func (m MatchQuery) Match(r *http.Request) bool {
+	// If no query keys are configured, this only
+	// matches an empty query string.
+	if len(m) == 0 {
+		return len(r.URL.Query()) == 0
+	}
+
 	repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
 
 	// parse query string just once, for efficiency
@@ -806,19 +813,25 @@ func (m MatchQuery) Match(r *http.Request) bool {
 		return false
 	}
 
+	// Count the amount of matched keys, to ensure we AND
+	// between all configured query keys; all keys must
+	// match at least one value.
+	matchedKeys := 0
 	for param, vals := range m {
 		param = repl.ReplaceAll(param, "")
 		paramVal, found := parsed[param]
-		if found {
-			for _, v := range vals {
-				v = repl.ReplaceAll(v, "")
-				if paramVal[0] == v || v == "*" {
-					return true
-				}
+		if !found {
+			return false
+		}
+		for _, v := range vals {
+			v = repl.ReplaceAll(v, "")
+			if slices.Contains(paramVal, v) || v == "*" {
+				matchedKeys++
+				break
 			}
 		}
 	}
-	return len(m) == 0 && len(r.URL.Query()) == 0
+	return matchedKeys == len(m)
 }
 
 // CELLibrary produces options that expose this matcher for use in CEL
diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go
index 10081309..5f76a36b 100644
--- a/modules/caddyhttp/matchers_test.go
+++ b/modules/caddyhttp/matchers_test.go
@@ -763,6 +763,42 @@ func TestQueryMatcher(t *testing.T) {
 			input:    "/?somekey=1",
 			expect:   true,
 		},
+		{
+			scenario: "do not match when not all query params are present",
+			match:    MatchQuery{"debug": []string{"1"}, "foo": []string{"bar"}},
+			input:    "/?debug=1",
+			expect:   false,
+		},
+		{
+			scenario: "match when all query params are present",
+			match:    MatchQuery{"debug": []string{"1"}, "foo": []string{"bar"}},
+			input:    "/?debug=1&foo=bar",
+			expect:   true,
+		},
+		{
+			scenario: "do not match when the value of a query param does not match",
+			match:    MatchQuery{"debug": []string{"1"}, "foo": []string{"bar"}},
+			input:    "/?debug=2&foo=bar",
+			expect:   false,
+		},
+		{
+			scenario: "do not match when all the values the query params do not match",
+			match:    MatchQuery{"debug": []string{"1"}, "foo": []string{"bar"}},
+			input:    "/?debug=2&foo=baz",
+			expect:   false,
+		},
+		{
+			scenario: "match against two values for the same key",
+			match:    MatchQuery{"debug": []string{"1"}},
+			input:    "/?debug=1&debug=2",
+			expect:   true,
+		},
+		{
+			scenario: "match against two values for the same key",
+			match:    MatchQuery{"debug": []string{"2", "1"}},
+			input:    "/?debug=2&debug=1",
+			expect:   true,
+		},
 	} {
 
 		u, _ := url.Parse(tc.input)