From f4ab03a39fafcdc7894da8bf9e0aa7acb99c5b8a Mon Sep 17 00:00:00 2001
From: Bart Koelman <bart@degreed.com>
Date: Mon, 14 Sep 2020 11:53:20 +0200
Subject: [PATCH] Fixed: splitting on comma in legacy filter notation

The original code looked at the first operator and would, unless that operator was 'in' or 'nin', split the query string parameter value on comma.

This never worked correctly in composition, because the syntax becomes ambiguous.
For example: `?filter[name]=like:a,in:b,c,d` could mean:
- like(a), in(b,c,d)
- like(a), in(b,c), equals(d)

So while this fix addresses the simple case, it still fails on combinations, including mixing with the new notation.
---
 .../FilterQueryStringParameterReader.cs       | 20 ++++++++++++++++++-
 .../Internal/LegacyFilterNotationConverter.cs | 17 ++++++++++++++++
 .../LegacyFilterParseTests.cs                 |  4 +++-
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs
index 87bb7f3554..0af5114144 100644
--- a/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs
+++ b/src/JsonApiDotNetCore/QueryStrings/Internal/FilterQueryStringParameterReader.cs
@@ -64,12 +64,30 @@ public virtual void Read(string parameterName, StringValues parameterValues)
         {
             _lastParameterName = parameterName;
 
-            foreach (string parameterValue in parameterValues)
+            foreach (string parameterValue in ExtractParameterValues(parameterName, parameterValues))
             {
                 ReadSingleValue(parameterName, parameterValue);
             }
         }
 
+        private IEnumerable<string> ExtractParameterValues(string parameterName, StringValues parameterValues)
+        {
+            foreach (string parameterValue in parameterValues)
+            {
+                if (_options.EnableLegacyFilterNotation)
+                {
+                    foreach (string condition in _legacyConverter.ExtractConditions(parameterName, parameterValue))
+                    {
+                        yield return condition;
+                    }
+                }
+                else
+                {
+                    yield return parameterValue;
+                }
+            }
+        }
+
         private void ReadSingleValue(string parameterName, string parameterValue)
         {
             try
diff --git a/src/JsonApiDotNetCore/QueryStrings/Internal/LegacyFilterNotationConverter.cs b/src/JsonApiDotNetCore/QueryStrings/Internal/LegacyFilterNotationConverter.cs
index 6234625424..efc96e435f 100644
--- a/src/JsonApiDotNetCore/QueryStrings/Internal/LegacyFilterNotationConverter.cs
+++ b/src/JsonApiDotNetCore/QueryStrings/Internal/LegacyFilterNotationConverter.cs
@@ -25,6 +25,23 @@ public sealed class LegacyFilterNotationConverter
             ["like:"] = Keywords.Contains
         };
 
+        public IEnumerable<string> ExtractConditions(string parameterName, string parameterValue)
+        {
+            if (parameterValue.StartsWith(ExpressionPrefix, StringComparison.Ordinal) ||
+                parameterValue.StartsWith(InPrefix, StringComparison.Ordinal) ||
+                parameterValue.StartsWith(NotInPrefix, StringComparison.Ordinal))
+            {
+                yield return parameterValue;
+            }
+            else
+            {
+                foreach (string condition in parameterValue.Split(','))
+                {
+                    yield return condition;
+                }
+            }
+        }
+
         public (string parameterName, string parameterValue) Convert(string parameterName, string parameterValue)
         {
             if (parameterName == null) throw new ArgumentNullException(nameof(parameterName));
diff --git a/test/UnitTests/QueryStringParameters/LegacyFilterParseTests.cs b/test/UnitTests/QueryStringParameters/LegacyFilterParseTests.cs
index b12a389a95..58a9c7ce06 100644
--- a/test/UnitTests/QueryStringParameters/LegacyFilterParseTests.cs
+++ b/test/UnitTests/QueryStringParameters/LegacyFilterParseTests.cs
@@ -56,7 +56,6 @@ public void Reader_Read_Fails(string parameterName, string parameterValue, strin
 
         [Theory]
         [InlineData("filter[caption]", "Brian O'Quote", "equals(caption,'Brian O''Quote')")]
-        [InlineData("filter[caption]", "using,comma", "equals(caption,'using,comma')")]
         [InlineData("filter[caption]", "am&per-sand", "equals(caption,'am&per-sand')")]
         [InlineData("filter[caption]", "2017-08-15T22:43:47.0156350-05:00", "equals(caption,'2017-08-15T22:43:47.0156350-05:00')")]
         [InlineData("filter[caption]", "eq:1", "equals(caption,'1')")]
@@ -77,6 +76,9 @@ public void Reader_Read_Fails(string parameterName, string parameterValue, strin
         [InlineData("filter", "expr:equals(author,null)", "equals(author,null)")]
         [InlineData("filter", "expr:has(author.articles)", "has(author.articles)")]
         [InlineData("filter", "expr:equals(count(author.articles),'1')", "equals(count(author.articles),'1')")]
+        [InlineData("filter[caption]", "using,comma", "or(equals(caption,'using'),equals(caption,'comma'))")]
+        [InlineData("filter[caption]", "like:First,Second", "or(contains(caption,'First'),equals(caption,'Second'))")]
+        [InlineData("filter[caption]", "like:First,like:Second", "or(contains(caption,'First'),contains(caption,'Second'))")]
         public void Reader_Read_Succeeds(string parameterName, string parameterValue, string expressionExpected)
         {
             // Act