Update NotContains match mode for Header and QueryParameters to suppo…

…rt missing/empty values (#2258) * handle empty query param value * header match logic * more readable * readable query logic * leave NotExists logic unchanged * Turn matching logic into a switch expression --------- Co-authored-by: Miha Zupan <[email protected]>
microsoft · Sep 26, 2023 · ea2ad4c · ea2ad4c
1 parent 0f491bd
commit ea2ad4c
Show file tree

Hide file tree

Showing 4 changed files with 115 additions and 51 deletions.
diff --git a/src/ReverseProxy/Routing/HeaderMatcherPolicy.cs b/src/ReverseProxy/Routing/HeaderMatcherPolicy.cs
@@ -70,34 +70,25 @@ public Task ApplyAsync(HttpContext httpContext, CandidateSet candidates)
 
             foreach (var matcher in matchers)
             {
-                var headerExistsInRequest = headers.TryGetValue(matcher.Name, out var requestHeaderValues);
-                if (headerExistsInRequest && !StringValues.IsNullOrEmpty(requestHeaderValues))
-                {
-                    if (matcher.Mode is HeaderMatchMode.Exists)
-                    {
-                        continue;
-                    }
-
-                    if (matcher.Mode is HeaderMatchMode.NotExists)
-                    {
-                        candidates.SetValidity(i, false);
-                        break;
-                    }
+                var headerExists = headers.TryGetValue(matcher.Name, out var requestHeaderValues);
+                var valueIsEmpty = StringValues.IsNullOrEmpty(requestHeaderValues);
 
-                    if (matcher.Mode is HeaderMatchMode.ExactHeader or HeaderMatchMode.HeaderPrefix
-                        ? TryMatchExactOrPrefix(matcher, requestHeaderValues)
-                        : TryMatchContainsOrNotContains(matcher, requestHeaderValues))
-                    {
-                        continue;
-                    }
-                }
-                else if (matcher.Mode is HeaderMatchMode.NotExists && !headerExistsInRequest)
+                var matched = matcher.Mode switch
                 {
-                    continue;
+                    HeaderMatchMode.Exists => !valueIsEmpty,
+                    HeaderMatchMode.NotExists => !headerExists,
+                    HeaderMatchMode.ExactHeader => !valueIsEmpty && TryMatchExactOrPrefix(matcher, requestHeaderValues),
+                    HeaderMatchMode.HeaderPrefix => !valueIsEmpty && TryMatchExactOrPrefix(matcher, requestHeaderValues),
+                    HeaderMatchMode.Contains => !valueIsEmpty && TryMatchContainsOrNotContains(matcher, requestHeaderValues),
+                    HeaderMatchMode.NotContains => valueIsEmpty || TryMatchContainsOrNotContains(matcher, requestHeaderValues),
+                    _ => false
+                };
+
+                if (!matched)
+                {
+                    candidates.SetValidity(i, false);
+                    break;
                 }
-
-                candidates.SetValidity(i, false);
-                break;
             }
         }
 

diff --git a/src/ReverseProxy/Routing/QueryParameterMatcherPolicy.cs b/src/ReverseProxy/Routing/QueryParameterMatcherPolicy.cs
@@ -69,22 +69,24 @@ public Task ApplyAsync(HttpContext httpContext, CandidateSet candidates)
 
             foreach (var matcher in matchers)
             {
-                if (query.TryGetValue(matcher.Name, out var requestQueryParameterValues) &&
-                    !StringValues.IsNullOrEmpty(requestQueryParameterValues))
+                query.TryGetValue(matcher.Name, out var requestQueryParameterValues);
+                var valueIsEmpty = StringValues.IsNullOrEmpty(requestQueryParameterValues);
+
+                var matched = matcher.Mode switch
+                {
+                    QueryParameterMatchMode.Exists => !valueIsEmpty,
+                    QueryParameterMatchMode.Exact => !valueIsEmpty && TryMatch(matcher, requestQueryParameterValues),
+                    QueryParameterMatchMode.Prefix => !valueIsEmpty && TryMatch(matcher, requestQueryParameterValues),
+                    QueryParameterMatchMode.Contains => !valueIsEmpty && TryMatch(matcher, requestQueryParameterValues),
+                    QueryParameterMatchMode.NotContains => valueIsEmpty || TryMatch(matcher, requestQueryParameterValues),
+                    _ => false
+                };
+
+                if (!matched)
                 {
-                    if (matcher.Mode is QueryParameterMatchMode.Exists)
-                    {
-                        continue;
-                    }
-
-                    if (TryMatch(matcher, requestQueryParameterValues))
-                    {
-                        continue;
-                    }
+                    candidates.SetValidity(i, false);
+                    break;
                 }
-
-                candidates.SetValidity(i, false);
-                break;
             }
         }
 

diff --git a/test/ReverseProxy.Tests/Routing/HeaderMatcherPolicyTests.cs b/test/ReverseProxy.Tests/Routing/HeaderMatcherPolicyTests.cs
@@ -276,12 +276,14 @@ public async Task ApplyAsync_MatchingScenarios_AnyHeaderValue(string incomingHea
     [InlineData("abc", HeaderMatchMode.Contains, false, "abc\"", true)]
     [InlineData("abc", HeaderMatchMode.Contains, false, "\"abc\"", true)]
     [InlineData("abc", HeaderMatchMode.Contains, false, "ab\"c", false)]
-    [InlineData("abc", HeaderMatchMode.NotContains, false, "", false)]
+    [InlineData("abc", HeaderMatchMode.NotContains, false, null, true)]
+    [InlineData("abc", HeaderMatchMode.NotContains, false, "", true)]
     [InlineData("abc", HeaderMatchMode.NotContains, false, "ababc", false)]
     [InlineData("abc", HeaderMatchMode.NotContains, false, "zaBCz", false)]
     [InlineData("abc", HeaderMatchMode.NotContains, false, "dcbaabcd", false)]
     [InlineData("abc", HeaderMatchMode.NotContains, false, "ababab", true)]
-    [InlineData("abc", HeaderMatchMode.NotContains, true, "", false)]
+    [InlineData("abc", HeaderMatchMode.NotContains, true, null, true)]
+    [InlineData("abc", HeaderMatchMode.NotContains, true, "", true)]
     [InlineData("abc", HeaderMatchMode.NotContains, true, "abcc", false)]
     [InlineData("abc", HeaderMatchMode.NotContains, true, "aaaBC", true)]
     [InlineData("abc", HeaderMatchMode.NotContains, true, "bbabcdb", false)]
@@ -412,16 +414,16 @@ public async Task ApplyAsync_MatchingScenarios_OneHeaderValue(
     [InlineData("abc", "def", HeaderMatchMode.Contains, true, "\"abc, def\"", true)]
     [InlineData("abc", "def", HeaderMatchMode.Contains, true, "\"abc\", def\"", true)]
     [InlineData("abc", "def", HeaderMatchMode.Contains, true, "ab\"cde\"f", false)]
-    [InlineData("abc", "def", HeaderMatchMode.NotContains, false, null, false)]
-    [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "", false)]
+    [InlineData("abc", "def", HeaderMatchMode.NotContains, false, null, true)]
+    [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "", true)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "aabc", false)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "baBc", false)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "ababcd", false)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "dcabcD", false)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "def", false)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, false, "ghi", true)]
-    [InlineData("abc", "def", HeaderMatchMode.NotContains, true, null, false)]
-    [InlineData("abc", "def", HeaderMatchMode.NotContains, true, "", false)]
+    [InlineData("abc", "def", HeaderMatchMode.NotContains, true, null, true)]
+    [InlineData("abc", "def", HeaderMatchMode.NotContains, true, "", true)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, true, "cabca", false)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, true, "aBCa", true)]
     [InlineData("abc", "def", HeaderMatchMode.NotContains, true, "CaBCdd", true)]
@@ -461,6 +463,42 @@ public async Task ApplyAsync_MatchingScenarios_TwoHeaderValues(
         Assert.Equal(shouldMatch, candidates.IsValidCandidate(0));
     }
 
+    [Theory]
+    [InlineData(HeaderMatchMode.Contains, true, false)]
+    [InlineData(HeaderMatchMode.Contains, false, false)]
+    [InlineData(HeaderMatchMode.NotContains, true, true)]
+    [InlineData(HeaderMatchMode.NotContains, false, true)]
+    [InlineData(HeaderMatchMode.HeaderPrefix, true, false)]
+    [InlineData(HeaderMatchMode.HeaderPrefix, false, false)]
+    [InlineData(HeaderMatchMode.ExactHeader, true, false)]
+    [InlineData(HeaderMatchMode.ExactHeader, false, false)]
+    [InlineData(HeaderMatchMode.NotExists, true, true)]
+    [InlineData(HeaderMatchMode.NotExists, false, true)]
+    [InlineData(HeaderMatchMode.Exists, true, false)]
+    [InlineData(HeaderMatchMode.Exists, false, false)]
+    public async Task ApplyAsync_MatchingScenarios_MissingHeader(
+        HeaderMatchMode headerValueMatchMode,
+        bool isCaseSensitive,
+        bool shouldMatch)
+    {
+        var context = new DefaultHttpContext();
+
+        var headerValues = new[] { "bar" };
+        if (headerValueMatchMode == HeaderMatchMode.Exists
+            || headerValueMatchMode == HeaderMatchMode.NotExists)
+        {
+            headerValues = null;
+        }
+
+        var endpoint = CreateEndpoint("foo", headerValues, headerValueMatchMode, isCaseSensitive);
+        var candidates = new CandidateSet(new[] { endpoint }, new RouteValueDictionary[1], new int[1]);
+        var sut = new HeaderMatcherPolicy();
+
+        await sut.ApplyAsync(context, candidates);
+
+        Assert.Equal(shouldMatch, candidates.IsValidCandidate(0));
+    }
+
     [Theory]
     [InlineData("Foo", "abc", HeaderMatchMode.ExactHeader, "ab, abc", true)]
     [InlineData("Foo", "abc", HeaderMatchMode.ExactHeader, "ab; abc", false)]

diff --git a/test/ReverseProxy.Tests/Routing/QueryMatcherPolicyTests.cs b/test/ReverseProxy.Tests/Routing/QueryMatcherPolicyTests.cs
@@ -214,12 +214,12 @@ public async Task ApplyAsync_MatchingScenarios_AnyQueryParamValue(string incomin
     [InlineData("abc", QueryParameterMatchMode.Prefix, true, "abcd", true)]
     [InlineData("abc", QueryParameterMatchMode.Prefix, true, "aBCd", false)]
     [InlineData("abc", QueryParameterMatchMode.Prefix, true, "ab", false)]
-    [InlineData("abc", QueryParameterMatchMode.NotContains, false, "", false)]
+    [InlineData("abc", QueryParameterMatchMode.NotContains, false, "", true)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, false, "aabc", false)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, false, "zaBCz", false)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, false, "sabcd", false)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, false, "aaab", true)]
-    [InlineData("abc", QueryParameterMatchMode.NotContains, true, "", false)]
+    [InlineData("abc", QueryParameterMatchMode.NotContains, true, "", true)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, true, "abcaa", false)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, true, "cbcaBC", true)]
     [InlineData("abc", QueryParameterMatchMode.NotContains, true, "ababcd", false)]
@@ -325,7 +325,7 @@ public async Task ApplyAsync_MatchingScenarios_OneQueryParamValue(
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "aaa", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "Abc", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "def", false)]
-    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "", false)]
+    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.Contains, true, "cabca", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.Contains, true, "aBCa", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.Contains, true, "CaBCdd", false)]
@@ -343,17 +343,17 @@ public async Task ApplyAsync_MatchingScenarios_OneQueryParamValue(
     [InlineData("abc", "ABC", QueryParameterMatchMode.Contains, true, "ABC;d", true)]
     [InlineData("abc", "ABC", QueryParameterMatchMode.Contains, true, "abC;d", false)]
     [InlineData("abc", "ABC", QueryParameterMatchMode.Contains, true, "abcABC;d", true)]
-    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, null, false)]
+    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, null, true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "aaa", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "Abc", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "def", false)]
-    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "", false)]
+    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "aabc", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "baBc", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "ababcd", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "dcabcD", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, false, "ghi", true)]
-    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, null, false)]
+    [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, null, true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "cabca", false)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "aBCa", true)]
     [InlineData("abc", "def", QueryParameterMatchMode.NotContains, true, "CaBCdd", true)]
@@ -394,6 +394,39 @@ public async Task ApplyAsync_MatchingScenarios_TwoQueryParamValues(
         Assert.Equal(shouldMatch, candidates.IsValidCandidate(0));
     }
 
+    [Theory]
+    [InlineData(QueryParameterMatchMode.NotContains, true, true)]
+    [InlineData(QueryParameterMatchMode.NotContains, false, true)]
+    [InlineData(QueryParameterMatchMode.Exists, true, false)]
+    [InlineData(QueryParameterMatchMode.Exists, false, false)]
+    [InlineData(QueryParameterMatchMode.Contains, true, false)]
+    [InlineData(QueryParameterMatchMode.Contains, false, false)]
+    [InlineData(QueryParameterMatchMode.Exact, true, false)]
+    [InlineData(QueryParameterMatchMode.Exact, false, false)]
+    [InlineData(QueryParameterMatchMode.Prefix, true, false)]
+    [InlineData(QueryParameterMatchMode.Prefix, false, false)]
+    public async Task ApplyAsync_MatchingScenarios_MissingParam(
+        QueryParameterMatchMode queryParamValueMatchMode,
+        bool isCaseSensitive,
+        bool shouldMatch)
+    {
+        var context = new DefaultHttpContext();
+
+        var queryParamValues = new[] { "bar" };
+        if (queryParamValueMatchMode == QueryParameterMatchMode.Exists)
+        {
+            queryParamValues = null;
+        }
+
+        var endpoint = CreateEndpoint("foo", queryParamValues, queryParamValueMatchMode, isCaseSensitive);
+        var candidates = new CandidateSet(new[] { endpoint }, new RouteValueDictionary[1], new int[1]);
+        var sut = new QueryParameterMatcherPolicy();
+
+        await sut.ApplyAsync(context, candidates);
+
+        Assert.Equal(shouldMatch, candidates.IsValidCandidate(0));
+    }
+
     [Theory]
     [InlineData(false, false, false)]
     [InlineData(false, true, false)]