- Clarified the code where the comment content ending is checked to be allowed or not.

- Added more unit tests

src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs

@@ -613,7 +613,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
                     if (NextIs(HtmlSymbolType.CloseAngle))
                     {
                         // Check condition 2.3: We're at the end of a comment. Check to make sure the text ending is allowed.
-                        isValidComment = !SymbolSequenceEndsWithItems(p, HtmlSymbolType.OpenAngle, HtmlSymbolType.Bang, HtmlSymbolType.DoubleHyphen);
+                        isValidComment = !IsCommentContentDisallowed(p);
                         return true;
                     }
                     else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle)))
@@ -643,21 +643,28 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
             return isValidComment;
         }
 
-        internal static bool SymbolSequenceEndsWithItems(IEnumerable<HtmlSymbol> sequence, params HtmlSymbolType[] items)
+        /// <summary>
+        /// Verifies, that the sequence doesn't end with the "&lt;!-" HtmlSymbols. Note, the first symbol is an opening bracket symbol
+        /// </summary>
+        internal static bool IsCommentContentDisallowed(IEnumerable<HtmlSymbol> sequence)
         {
-            int index = items.Length;
+            var reversedSequence = sequence.Reverse();
-            foreach (var previousSymbol in sequence)
+            var disallowEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) };
+            var index = 0;
+            foreach (var item in reversedSequence)
             {
-                if (index == 0)
+                if (!item.Equals(disallowEnding[index++]))
                 {
-                    break;
+                    return false;
                 }
 
-                if (items[--index] != previousSymbol.Type)
+                if (index == disallowEnding.Length)
-                    return false;
+                {
+                    return true;
+                }
             }
 
-            return index == 0;
+            return false;
         }
 
         private bool CData()

src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs

@@ -135,7 +135,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
                 }
 
                 symbols.Add(CurrentSymbol);
-                if (condition(CurrentSymbol, symbols.Reverse<TSymbol>()))
+                if (condition(CurrentSymbol, symbols))
                 {
                     matchFound = true;
                     break;

test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs

@@ -1,4 +1,6 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
+using System.Linq;
 using Microsoft.AspNetCore.Razor.Language.Legacy;
 using Xunit;
 
@@ -25,7 +27,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsDashSymbol_ReturnsFalseForNonDashSymbol(object symbol)
         {
             // Arrange
-            HtmlSymbol convertedSymbol = (HtmlSymbol)symbol;
+            var convertedSymbol = (HtmlSymbol)symbol;
 
             // Act & Assert
             Assert.False(HtmlMarkupParser.IsDashSymbol(convertedSymbol));
@@ -35,7 +37,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsDashSymbol_ReturnsTrueForADashSymbol()
         {
             // Arrange
-            HtmlSymbol dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text);
+            var dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text);
 
             // Act & Assert
             Assert.True(HtmlMarkupParser.IsDashSymbol(dashSymbol));
@@ -45,10 +47,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void AcceptAllButLastDoubleHypens_ReturnsTheOnlyDoubleHyphenSymbol()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("-->");
+            var sut = CreateTestParserForContent("-->");
 
             // Act
-            HtmlSymbol symbol = sut.AcceptAllButLastDoubleHypens();
+            var symbol = sut.AcceptAllButLastDoubleHypens();
 
             // Assert
             Assert.Equal(doubleHyphenSymbol, symbol);
@@ -60,10 +62,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void AcceptAllButLastDoubleHypens_ReturnsTheDoubleHyphenSymbolAfterAcceptingTheDash()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("--->");
+            var sut = CreateTestParserForContent("--->");
 
             // Act
-            HtmlSymbol symbol = sut.AcceptAllButLastDoubleHypens();
+            var symbol = sut.AcceptAllButLastDoubleHypens();
 
             // Assert
             Assert.Equal(doubleHyphenSymbol, symbol);
@@ -75,7 +77,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsHtmlCommentAhead_ReturnsTrueForEmptyCommentTag()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("---->");
+            var sut = CreateTestParserForContent("---->");
 
             // Act & Assert
             Assert.True(sut.IsHtmlCommentAhead());
@@ -85,7 +87,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTag()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("-- Some comment content in here -->");
+            var sut = CreateTestParserForContent("-- Some comment content in here -->");
 
             // Act & Assert
             Assert.True(sut.IsHtmlCommentAhead());
@@ -95,7 +97,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraDashesAtClosingTag()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("-- Some comment content in here ----->");
+            var sut = CreateTestParserForContent("-- Some comment content in here ----->");
 
             // Act & Assert
             Assert.True(sut.IsHtmlCommentAhead());
@@ -105,7 +107,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("-- comment --> the first part is a valid comment without the Open angle and bang symbols");
+            var sut = CreateTestParserForContent("-- comment --> the first part is a valid comment without the Open angle and bang symbols");
 
             // Act & Assert
             Assert.True(sut.IsHtmlCommentAhead());
@@ -115,7 +117,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsHtmlCommentAhead_ReturnsFalseForNotClosedComment()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("-- not closed comment");
+            var sut = CreateTestParserForContent("-- not closed comment");
 
             // Act & Assert
             Assert.False(sut.IsHtmlCommentAhead());
@@ -125,12 +127,45 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy
         public void IsHtmlCommentAhead_ReturnsFalseForCommentWithoutLastClosingAngle()
         {
             // Arrange
-            TestHtmlMarkupParser sut = CreateTestParserForContent("-- not closed comment--");
+            var sut = CreateTestParserForContent("-- not closed comment--");
 
             // Act & Assert
             Assert.False(sut.IsHtmlCommentAhead());
         }
 
+        [Fact]
+        public void IsCommentContentDisallowed_ReturnsFalseForAllowedContent()
+        {
+            // Arrange
+            var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text);
+            var sequence = Enumerable.Range((int)'a', 26).Select(item => new HtmlSymbol(((char)item).ToString(), HtmlSymbolType.Text));
+
+            // Act & Assert
+            Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence));
+        }
+
+        [Fact]
+        public void IsCommentContentDisallowed_ReturnsTrueForDisallowedContent()
+        {
+            // Arrange
+            var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text);
+            var sequence = new[] { new HtmlSymbol("<", HtmlSymbolType.OpenAngle), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("-", HtmlSymbolType.Text) };
+
+            // Act & Assert
+            Assert.True(HtmlMarkupParser.IsCommentContentDisallowed(sequence));
+        }
+
+        [Fact]
+        public void IsCommentContentDisallowed_ReturnsFalseForEmptyContent()
+        {
+            // Arrange
+            var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text);
+            var sequence = Array.Empty<HtmlSymbol>();
+
+            // Act & Assert
+            Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence));
+        }
+
         private class TestHtmlMarkupParser : HtmlMarkupParser
         {
             public new HtmlSymbol PreviousSymbol

test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs

@@ -3,7 +3,7 @@
 
 using System;
 using System.Collections.Generic;
-using System.IO;
+using System.Linq;
 using System.Text;
 using Xunit;
 
@@ -57,25 +57,29 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
         }
 
         [Fact]
-        public void LookaheadUntil_PassesThePreviousSymbolsInReverseOrder()
+        public void LookaheadUntil_PassesThePreviousSymbolsInTheSameOrder()
         {
             // Arrange
             var tokenizer = CreateContentTokenizer("asdf--fvd--<");
 
             // Act
-            Stack<HtmlSymbol> symbols = new Stack<HtmlSymbol>();
             var i = 3;
+            IEnumerable<HtmlSymbol> previousSymbols = null;
             var symbolFound = tokenizer.LookaheadUntil((s, p) =>
             {
-                symbols.Push(s);
+                previousSymbols = p;
                 return --i == 0;
             });
 
             // Assert
-            Assert.Equal(3, symbols.Count);
+            Assert.Equal(4, previousSymbols.Count());
-            Assert.Equal(new HtmlSymbol("fvd", HtmlSymbolType.Text), symbols.Pop());
+
-            Assert.Equal(new HtmlSymbol("--", HtmlSymbolType.DoubleHyphen), symbols.Pop());
+            // For the very first element, there will be no previous items, so null is expected
-            Assert.Equal(new HtmlSymbol("asdf", HtmlSymbolType.Text), symbols.Pop());
+            var orderIndex = 0;
+            Assert.Null(previousSymbols.ElementAt(orderIndex++));
+            Assert.Equal(new HtmlSymbol("asdf", HtmlSymbolType.Text), previousSymbols.ElementAt(orderIndex++));
+            Assert.Equal(new HtmlSymbol("--", HtmlSymbolType.DoubleHyphen), previousSymbols.ElementAt(orderIndex++));
+            Assert.Equal(new HtmlSymbol("fvd", HtmlSymbolType.Text), previousSymbols.ElementAt(orderIndex++));
         }
 
         [Fact]
@@ -85,7 +89,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
             var tokenizer = CreateContentTokenizer("asdf--fvd");
 
             // Act
-            Stack<HtmlSymbol> symbols = new Stack<HtmlSymbol>();
+            var symbols = new Stack<HtmlSymbol>();
             var symbolFound = tokenizer.LookaheadUntil((s, p) =>
             {
                 symbols.Push(s);
@@ -107,7 +111,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
             var tokenizer = CreateContentTokenizer("asdf--fvd");
 
             // Act
-            Stack<HtmlSymbol> symbols = new Stack<HtmlSymbol>();
+            var symbols = new Stack<HtmlSymbol>();
             var symbolFound = tokenizer.LookaheadUntil((s, p) =>
             {
                 symbols.Push(s);