From e98b3abc27692962b79e8f0d42706d9f2e491fba Mon Sep 17 00:00:00 2001 From: Nikita Kotlyarov Date: Sun, 26 Dec 2021 15:53:49 +0400 Subject: [PATCH 1/3] Add checks for loose closing blocks Check loose closing blocks on compile time --- source/Handlebars.Test/ExceptionTests.cs | 47 +++++++++++++++- .../Lexer/Converter/BlockAccumulator.cs | 12 ++--- .../BlockAccumulatorContext.cs | 54 ++++++++++++++++++- 3 files changed, 105 insertions(+), 8 deletions(-) diff --git a/source/Handlebars.Test/ExceptionTests.cs b/source/Handlebars.Test/ExceptionTests.cs index 5aa3f6f0..434dc999 100644 --- a/source/Handlebars.Test/ExceptionTests.cs +++ b/source/Handlebars.Test/ExceptionTests.cs @@ -12,5 +12,50 @@ public void TestNonClosingBlockExpressionException() Handlebars.Compile("{{#if 0}}test")(new { }); }); } - } + + [Fact] + public void TestLooseClosingBlockExpressionException() + { + Assert.Throws(() => + { + Handlebars.Compile("{{#if 0}}test{{/if}}{{/unless}}")(new { }); + }); + } + + [Fact] + public void TestNestedLooseClosingBlockExpressionException() + { + Assert.Throws(() => + { + Handlebars.Compile("{{#if 1}}{{#unless 0}}test{{/if}}{{/unless}}{{/if}}")(new { }); + }); + } + + [Fact] + public void TestUnmatchedClosingBlockExpressionException() + { + Assert.Throws(() => + { + Handlebars.Compile("{{#if 0}}test{{/unless}}")(new { }); + }); + } + + [Fact] + public void TestLooseClosingBlockInIteratorExpressionException() + { + var data = new + { + enumerateMe = new + { + foo = "hello", + bar = "world" + } + }; + + Assert.Throws(() => + { + Handlebars.Compile("{{#each enumerateMe}}test{{/if}}{{/each}}")(data); + }); + } + } } diff --git a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulator.cs b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulator.cs index d252974e..40068729 100644 --- a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulator.cs +++ b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulator.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections.Generic; +using System.Collections.Generic; using System.Linq; using System.Linq.Expressions; @@ -27,10 +26,10 @@ public override IEnumerable ConvertTokens(IEnumerable sequence) while (enumerator.MoveNext()) { var item = (Expression)enumerator.Current; - var context = BlockAccumulatorContext.Create(item, _configuration); + var context = BlockAccumulatorContext.Create(item, null, _configuration); if (context != null) { - yield return AccumulateBlock(enumerator, context); + yield return AccumulateBlock(item, enumerator, context); } else { @@ -40,16 +39,17 @@ public override IEnumerable ConvertTokens(IEnumerable sequence) } private Expression AccumulateBlock( + Expression parentItem, IEnumerator enumerator, BlockAccumulatorContext context) { while (enumerator.MoveNext()) { var item = (Expression)enumerator.Current; - var innerContext = BlockAccumulatorContext.Create(item, _configuration); + var innerContext = BlockAccumulatorContext.Create(item, parentItem, _configuration); if (innerContext != null) { - context.HandleElement(AccumulateBlock(enumerator, innerContext)); + context.HandleElement(AccumulateBlock(item, enumerator, innerContext)); } else if (context.IsClosingElement(item)) { diff --git a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs index 195e8c6a..fc906161 100644 --- a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs +++ b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs @@ -10,7 +10,7 @@ internal abstract class BlockAccumulatorContext private static readonly HashSet ConditionHelpers = new HashSet(StringComparer.OrdinalIgnoreCase){ "#if", "#unless", "^if", "^unless" }; private static readonly HashSet IteratorHelpers = new HashSet(StringComparer.OrdinalIgnoreCase){ "#each", "^each" }; - public static BlockAccumulatorContext Create(Expression item, ICompiledHandlebarsConfiguration configuration) + public static BlockAccumulatorContext Create(Expression item, Expression parentItem, ICompiledHandlebarsConfiguration configuration) { BlockAccumulatorContext context = null; if (IsConditionalBlock(item)) @@ -29,6 +29,10 @@ public static BlockAccumulatorContext Create(Expression item, ICompiledHandlebar { context = new BlockHelperAccumulatorContext(item); } + else if (IsLooseClosingElement(item, parentItem, out var looseBlockName)) + { + throw new HandlebarsCompilerException($"Loose closing block '{looseBlockName}' was found"); + } return context; } @@ -77,6 +81,54 @@ private static bool IsPartialBlock (Expression item) } } + private static bool IsLooseClosingElement(Expression item, Expression parentItem, out string looseBlockName) + { + looseBlockName = null; + + var itemBlockName = GetBlockName(item); + + if (itemBlockName == null) return false; + + var parentBlockName = GetBlockName(parentItem); + + if (!itemBlockName.StartsWith("/")) return false; + + if (parentBlockName == null || IsClosingBlockNotMatchParentBlock(itemBlockName, parentBlockName)) + { + looseBlockName = itemBlockName; + + return true; + } + + return false; + } + + private static bool IsClosingBlockNotMatchParentBlock(string itemBlockName, string parentBlockName) + { + if (itemBlockName == null) throw new ArgumentNullException(nameof(itemBlockName)); + if (parentBlockName == null) throw new ArgumentNullException(nameof(parentBlockName)); + + if (!parentBlockName.StartsWith("#") || parentBlockName.StartsWith("#>") || parentBlockName.StartsWith("#*")) return false; + + return parentBlockName.Substring(1) != itemBlockName.Substring(1); + } + + private static string GetBlockName(Expression item) + { + item = UnwrapStatement(item); + switch( item ) + { + case PathExpression pathExpression: + return pathExpression.Path; + + case HelperExpression helperExpression: + return helperExpression.HelperName; + + default: + return null; + } + } + protected static Expression UnwrapStatement(Expression item) { if (item is StatementExpression expression) From a1d3683da7775018cea2177f87939a2c03f1777c Mon Sep 17 00:00:00 2001 From: Nikita Kotlyarov Date: Wed, 29 Dec 2021 10:50:00 +0400 Subject: [PATCH 2/3] Improve performance for substrings + fix code style --- .../BlockAccumulatorContext.cs | 33 +++++++------------ 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs index fc906161..4314b379 100644 --- a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs +++ b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq.Expressions; using HandlebarsDotNet.PathStructure; +using HandlebarsDotNet.StringUtils; namespace HandlebarsDotNet.Compiler { @@ -68,17 +69,12 @@ private static bool IsIteratorBlock(Expression item) private static bool IsPartialBlock (Expression item) { item = UnwrapStatement (item); - switch (item) + return item switch { - case PathExpression expression: - return expression.Path.StartsWith("#>"); - - case HelperExpression helperExpression: - return helperExpression.HelperName.StartsWith("#>"); - - default: - return false; - } + PathExpression expression => expression.Path.StartsWith("#>"), + HelperExpression helperExpression => helperExpression.HelperName.StartsWith("#>"), + _ => false, + }; } private static bool IsLooseClosingElement(Expression item, Expression parentItem, out string looseBlockName) @@ -110,23 +106,18 @@ private static bool IsClosingBlockNotMatchParentBlock(string itemBlockName, stri if (!parentBlockName.StartsWith("#") || parentBlockName.StartsWith("#>") || parentBlockName.StartsWith("#*")) return false; - return parentBlockName.Substring(1) != itemBlockName.Substring(1); + return new Substring(parentBlockName, 1) != new Substring(itemBlockName, 1); } private static string GetBlockName(Expression item) { item = UnwrapStatement(item); - switch( item ) + return item switch { - case PathExpression pathExpression: - return pathExpression.Path; - - case HelperExpression helperExpression: - return helperExpression.HelperName; - - default: - return null; - } + PathExpression pathExpression => pathExpression.Path, + HelperExpression helperExpression => helperExpression.HelperName, + _ => null, + }; } protected static Expression UnwrapStatement(Expression item) From d538b4cf7df2fb636cee3e18489d7887c96ee73c Mon Sep 17 00:00:00 2001 From: Nikita Kotlyarov Date: Thu, 30 Dec 2021 11:15:47 +0400 Subject: [PATCH 3/3] Update BlockAccumulatorContext.cs Update error message + make renaming --- .../BlockAccumulatorContext.cs | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs index 4314b379..8ec0682e 100644 --- a/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs +++ b/source/Handlebars/Compiler/Lexer/Converter/BlockAccumulators/BlockAccumulatorContext.cs @@ -30,9 +30,9 @@ public static BlockAccumulatorContext Create(Expression item, Expression parentI { context = new BlockHelperAccumulatorContext(item); } - else if (IsLooseClosingElement(item, parentItem, out var looseBlockName)) + else if (IsDetachedClosingElement(item, parentItem, out var closingElement)) { - throw new HandlebarsCompilerException($"Loose closing block '{looseBlockName}' was found"); + throw new HandlebarsCompilerException($"A closing element '{closingElement}' was found without a matching open element"); } return context; @@ -77,21 +77,21 @@ private static bool IsPartialBlock (Expression item) }; } - private static bool IsLooseClosingElement(Expression item, Expression parentItem, out string looseBlockName) + private static bool IsDetachedClosingElement(Expression item, Expression parentItem, out string closingElement) { - looseBlockName = null; + closingElement = null; - var itemBlockName = GetBlockName(item); + var itemElement = GetItemElement(item); - if (itemBlockName == null) return false; + if (itemElement == null) return false; - var parentBlockName = GetBlockName(parentItem); + var parentItemElement = GetItemElement(parentItem); - if (!itemBlockName.StartsWith("/")) return false; + if (!itemElement.StartsWith("/")) return false; - if (parentBlockName == null || IsClosingBlockNotMatchParentBlock(itemBlockName, parentBlockName)) + if (parentItemElement == null || IsClosingElementNotMatchOpenElement(itemElement, parentItemElement)) { - looseBlockName = itemBlockName; + closingElement = itemElement; return true; } @@ -99,17 +99,17 @@ private static bool IsLooseClosingElement(Expression item, Expression parentItem return false; } - private static bool IsClosingBlockNotMatchParentBlock(string itemBlockName, string parentBlockName) + private static bool IsClosingElementNotMatchOpenElement(string closingElement, string openElement) { - if (itemBlockName == null) throw new ArgumentNullException(nameof(itemBlockName)); - if (parentBlockName == null) throw new ArgumentNullException(nameof(parentBlockName)); + if (closingElement == null) throw new ArgumentNullException(nameof(closingElement)); + if (openElement == null) throw new ArgumentNullException(nameof(openElement)); - if (!parentBlockName.StartsWith("#") || parentBlockName.StartsWith("#>") || parentBlockName.StartsWith("#*")) return false; + if (!openElement.StartsWith("#") || openElement.StartsWith("#>") || openElement.StartsWith("#*")) return false; - return new Substring(parentBlockName, 1) != new Substring(itemBlockName, 1); + return new Substring(openElement, 1) != new Substring(closingElement, 1); } - private static string GetBlockName(Expression item) + private static string GetItemElement(Expression item) { item = UnwrapStatement(item); return item switch