diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index e9dcf36d5c..39841d4835 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/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 sequence, params HtmlSymbolType[] items) + /// + /// Verifies, that the sequence doesn't end with the "<!-" HtmlSymbols. Note, the first symbol is an opening bracket symbol + /// + internal static bool IsCommentContentDisallowed(IEnumerable sequence) { - int index = items.Length; - foreach (var previousSymbol in sequence) + var reversedSequence = sequence.Reverse(); + 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) - return false; + if (index == disallowEnding.Length) + { + return true; + } } - return index == 0; + return false; } private bool CData() diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs index 7911380d0f..01fa0b11e1 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/TokenizerBackedParser.cs +++ b/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())) + if (condition(CurrentSymbol, symbols)) { matchFound = true; break; diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs index db47d47b73..c3e0cb87e1 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs +++ b/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(); + + // Act & Assert + Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + } + private class TestHtmlMarkupParser : HtmlMarkupParser { public new HtmlSymbol PreviousSymbol diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs index 74d0c075f3..aa9ea560cc 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/TokenizerLookaheadTest.cs +++ b/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 symbols = new Stack(); var i = 3; + IEnumerable previousSymbols = null; var symbolFound = tokenizer.LookaheadUntil((s, p) => { - symbols.Push(s); + previousSymbols = p; return --i == 0; }); // Assert - Assert.Equal(3, symbols.Count); - Assert.Equal(new HtmlSymbol("fvd", HtmlSymbolType.Text), symbols.Pop()); - Assert.Equal(new HtmlSymbol("--", HtmlSymbolType.DoubleHyphen), symbols.Pop()); - Assert.Equal(new HtmlSymbol("asdf", HtmlSymbolType.Text), symbols.Pop()); + Assert.Equal(4, previousSymbols.Count()); + + // For the very first element, there will be no previous items, so null is expected + 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 symbols = new Stack(); + var symbols = new Stack(); 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 symbols = new Stack(); + var symbols = new Stack(); var symbolFound = tokenizer.LookaheadUntil((s, p) => { symbols.Push(s);