Merge pull request #126 from danielearwicker/safeLiteralFilterValues

Safe literal filter values

server/dotnet/FlowerBI.Engine.Tests/FilterParametersTests.cs

@@ -0,0 +1,78 @@
+namespace FlowerBI.Engine.Tests;
+
+using FluentAssertions;
+using Xunit;
+
+public class FilterParametersTests
+{
+    private static Filter MakeFilter(object val)
+        => new(new LabelledColumn("x", new Column<string>("c1")), "=", val, null);
+
+    [Fact]
+    public void DapperFilterParameters_String_GeneratesActualParam()
+    {   
+        var p = new DapperFilterParameters();
+        var result = p[MakeFilter("hi")];
+        result.Should().Be("@filter0");
+        p.DapperParams.ParameterNames.Should().BeEquivalentTo(["filter0"]);
+    }
+
+    [Fact]
+    public void DapperFilterParameters_ListWithString_GeneratesActualParam()
+    {   
+        var p = new DapperFilterParameters();
+        var result = p[MakeFilter("hi")];
+        result.Should().Be("@filter0");
+        p.DapperParams.ParameterNames.Should().BeEquivalentTo(["filter0"]);
+    }
+
+    [Theory]
+    [InlineData(42, "42")]
+    [InlineData((short)42, "42")]
+    [InlineData((long)42, "42")]
+    [InlineData(3.14, "3.14")]
+    [InlineData((float)3.14, "3.14")]
+    [InlineData(true, "1")]
+    [InlineData(false, "0")]
+    public void DapperFilterParameters_SimpleNumber_GeneratesLiteral(object val, string expected)
+    {   
+        var p = new DapperFilterParameters();
+        var result = p[MakeFilter(val)];
+        result.Should().Be(expected);
+        p.DapperParams.ParameterNames.Should().BeEmpty();
+    }
+
+    [Fact]
+    public void DapperFilterParameters_Decimal_GeneratesLiteral()
+    {   
+        var p = new DapperFilterParameters();
+        var result = p[MakeFilter(3.14m)];
+        result.Should().Be("3.14");
+        p.DapperParams.ParameterNames.Should().BeEmpty();
+    }
+
+    [Theory]
+    [InlineData(42, "(42, 42)")]
+    [InlineData((short)42, "(42, 42)")]
+    [InlineData((long)42, "(42, 42)")]
+    [InlineData(3.14, "(3.14, 3.14)")]
+    [InlineData((float)3.14, "(3.14, 3.14)")]
+    [InlineData(true, "(1, 1)")]
+    [InlineData(false, "(0, 0)")]
+    public void DapperFilterParameters_SimpleNumberList_GeneratesLiteral(object val, string expected)
+    {   
+        var p = new DapperFilterParameters();
+        var result = p[MakeFilter(new[] {val, val})];
+        result.Should().Be(expected);
+        p.DapperParams.ParameterNames.Should().BeEmpty();
+    }
+
+    [Fact]
+    public void DapperFilterParameters_DecimalList_GeneratesLiteral()
+    {   
+        var p = new DapperFilterParameters();
+        var result = p[MakeFilter(new object[] {3.14, 8, (short)3})];
+        result.Should().Be("(3.14, 8, 3)");
+        p.DapperParams.ParameterNames.Should().BeEmpty();
+    }
+}

server/dotnet/FlowerBI.Engine/QueryGeneration/FilterParameters.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using Dapper;
+using YamlDotNet.Serialization;
 
 namespace FlowerBI
 {
@@ -60,10 +61,56 @@ private static string FormatValue(object val)
         override public string ToString() =>
             string.Join(", ", _names.Select(x => $"{x.Value} = {FormatValue(x.Key.Value)}"));
 
+        private static bool TryGetSafeLiteral(object val, out string literal)
+        {
+            if (val is int or long or short or decimal or float or double)
+            {
+                literal = $"{val}";
+                return true;
+            }
+
+            if (val is bool b)
+            {
+                literal = b ? "1" : "0";
+                return true;
+            }
+
+            if (val is IEnumerable<object> seq)
+            {
+                var parts = new List<string>();
+                foreach (var item in seq)
+                {
+                    if (TryGetSafeLiteral(item, out var part))
+                    {
+                        parts.Add(part);
+                    }
+                    else
+                    {
+                        literal = string.Empty;
+                        return false;
+                    }
+                }
+
+                literal = $"({string.Join(", ", parts)})";
+                return true;
+            }
+
+            literal = string.Empty;
+            return false;
+        }
+
         public string this[Filter filter]
         {
             get
             {
+                // Numeric and boolean values (and lists of) are not a SQL injection risk,
+                // while embedding them as constants can help the DB query planner match to
+                // filtered or indexed views etc.
+                if (TryGetSafeLiteral(filter.Value, out var literal))
+                {
+                    return literal;
+                }
+
                 if (!_names.TryGetValue(filter, out var name))
                 {
                     var plainName = $"filter{_names.Count}";