fix (Parsing): Fix 2/5 of the parsing errors reported in sebastienros…

…#148 . Filter expression guards are in place, but will throw a runtime error, ideally this should materialize as a template validation error.
jafin · Sep 7, 2019 · b8d45f7 · b8d45f7
1 parent 273a6f6
commit b8d45f7
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 3 deletions.
diff --git a/Fluid.Tests/Fluid.Tests.csproj b/Fluid.Tests/Fluid.Tests.csproj
@@ -7,6 +7,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.Extensions.FileProviders.Abstractions" Version="1.1.1" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.8.0" />
+    <PackageReference Include="Shouldly" Version="3.0.2" />
     <PackageReference Include="xunit" Version="2.4.0" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.4.0" />
     <PackageReference Include="xunit.analyzers" Version="0.10.0" />

diff --git a/Fluid.Tests/Issues/Issue148ExceptionsTests.cs b/Fluid.Tests/Issues/Issue148ExceptionsTests.cs
@@ -0,0 +1,62 @@
+using System.Collections.Generic;
+using Shouldly;
+using Xunit;
+
+namespace Fluid.Tests.Issues
+{
+    public class Issue148ExceptionsTests
+    {
+        private FluidTemplate _fluidTemplate;
+        private IEnumerable<string> _errors;
+        private readonly User _model;
+
+        public Issue148ExceptionsTests()
+        {
+            _model = new User
+            {
+                String = "ABC",
+                Integer = 123,
+                Doubles = new List<double> {1.1, 2.2, 3.3}
+            };
+        }
+
+
+        [Theory]
+        [InlineData("{%endfor%}", "No start tag 'for' found for tag 'endfor'")]
+        [InlineData("{%comment%}{%", "End tag 'endcomment' was not found")]
+        public void Issue148ParsingErrors(string template, string expectedErrorString)
+        {
+            Run(_model, template);
+            _errors.ShouldContain(expectedErrorString);
+        }
+
+        [Theory(Skip =
+            "Fix the exceptions throw at RenderAsync time, need to be caught earlier (BuildFilterExpression?)")]
+        [InlineData("{{0|remove}}", "'remove' requires string argument.  i.e.  'remove \"word_to_remove\"")]
+        [InlineData("{{0|modulo}}", "'modulo' requires one numeric argument.  i.e.  'modulo: 5'")]
+        [InlineData("<p>{{false|divided_by|modulo|urlode}}<<",
+            "'modulo' requires one numeric argument.  i.e.  'modulo: 5'")]
+        public void Issue148RuntimeErrors(string template, string expectedErrorString)
+        {
+            Run(_model, template);
+            _errors.ShouldContain(expectedErrorString);
+        }
+
+
+        public class User
+        {
+            public string String { get; set; }
+            public int Integer { get; set; }
+            public List<double> Doubles { get; set; }
+        }
+
+        private void Run(User model, string template)
+        {
+            if (FluidTemplate.TryParse(template, out _fluidTemplate, out _errors))
+            {
+                TemplateContext.GlobalMemberAccessStrategy.Register<User>();
+                _fluidTemplate.Render(new TemplateContext {Model = model});
+            }
+        }
+    }
+}
diff --git a/Fluid/DefaultFluidParser.cs b/Fluid/DefaultFluidParser.cs
@@ -237,9 +237,7 @@ private StringSegment ConsumeTag(StringSegment segment, int start, string endTag
             }
 
             // We reached the end of the segment without finding the matched tag.
-            // Ideally we could return a parsing error, right now we just return the text.
-            end = segment.Length - 1;
-            return segment.Subsegment(start, index - start);
+            throw new ParseException($"End tag '{endTag}' was not found");
         }
 
         /// <summary>
@@ -794,6 +792,10 @@ public virtual UnlessStatement BuildUnlessStatement(BlockContext context)
 
         public static Statement BuildForStatement(BlockContext context)
         {
+            if (context.Tag == null)
+            {
+                throw new ParseException($"No start tag 'for' found for tag 'endfor'");
+            }
             if (context.Tag.Term.Name != "for")
             {
                 throw new ParseException($"Unexpected tag: '{context.Tag.Term.Name}' not matching 'for' tag.");

diff --git a/Fluid/Filters/NumberFilters.cs b/Fluid/Filters/NumberFilters.cs
@@ -1,4 +1,5 @@
 using System;
+using Fluid.Guards;
 using Fluid.Values;
 
 namespace Fluid.Filters
@@ -79,6 +80,7 @@ public static FluidValue Minus(FluidValue input, FilterArguments arguments, Temp
 
         public static FluidValue Modulo(FluidValue input, FilterArguments arguments, TemplateContext context)
         {
+            Guard.AgainstNullOrEmptyArguments(arguments, $"'modulo' requires one numeric argument.  i.e.  'modulo: 5'");
             return NumberValue.Create(Convert.ToInt32(input.ToNumberValue()) % Convert.ToInt32(arguments.At(0).ToNumberValue()));
         }
 

diff --git a/Fluid/Filters/StringFilters.cs b/Fluid/Filters/StringFilters.cs
@@ -1,4 +1,5 @@
 using System;
+using Fluid.Guards;
 using Fluid.Values;
 
 namespace Fluid.Filters
@@ -93,6 +94,7 @@ public static FluidValue RemoveFirst(FluidValue input, FilterArguments arguments
 
         public static FluidValue Remove(FluidValue input, FilterArguments arguments, TemplateContext context)
         {
+            Guard.AgainstNullOrEmptyArguments(arguments, $"'remove' requires string argument.  i.e.  'remove \"word_to_remove\"");
             return new StringValue(input.ToStringValue().Replace(arguments.At(0).ToStringValue(), ""));
         }
 

diff --git a/Fluid/Guards/GuardExtensions.cs b/Fluid/Guards/GuardExtensions.cs
@@ -0,0 +1,13 @@
+namespace Fluid.Guards
+{
+    public static class Guard
+    {
+        public static void AgainstNullOrEmptyArguments(FilterArguments argument, string customMessage)
+        {
+            if (argument == null || argument.Count == 0)
+            {
+                throw new ParseException(customMessage);
+            }
+        }
+    }
+}