diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs index 9f9886164c..6c37427ca7 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/BlockKindInternal.cs @@ -19,7 +19,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy // Special Comment = 8, Tag = 9, - HtmlComment = 10 } } diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs index 48c79c83e1..a23adbb28f 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlMarkupParser.cs @@ -12,7 +12,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { private const string ScriptTagName = "script"; - private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { new HtmlSymbol("-", HtmlSymbolType.Text), new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; + private static readonly HtmlSymbol[] nonAllowedHtmlCommentEnding = new[] { HtmlSymbol.Hyphen, new HtmlSymbol("!", HtmlSymbolType.Bang), new HtmlSymbol("<", HtmlSymbolType.OpenAngle) }; + private static readonly HtmlSymbol[] singleHyphenArray = new[] { HtmlSymbol.Hyphen }; private static readonly char[] ValidAfterTypeAttributeNameCharacters = { ' ', '\t', '\r', '\n', '\f', '=' }; private SourceLocation _lastTagStart = SourceLocation.Zero; @@ -506,7 +507,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy while (!EndOfFile) { SkipToAndParseCode(HtmlSymbolType.DoubleHyphen); - var lastDoubleHyphen = AcceptAllButLastDoubleHypens(); + var lastDoubleHyphen = AcceptAllButLastDoubleHyphens(); if (At(HtmlSymbolType.CloseAngle)) { @@ -543,7 +544,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return false; } - protected HtmlSymbol AcceptAllButLastDoubleHypens() + protected HtmlSymbol AcceptAllButLastDoubleHyphens() { var lastDoubleHyphen = CurrentSymbol; AcceptWhile(s => @@ -558,9 +559,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy }); NextToken(); - EnsureCurrent(); - if (At(HtmlSymbolType.Text) && IsDashSymbol(CurrentSymbol)) + if (At(HtmlSymbolType.Text) && IsHyphen(CurrentSymbol)) { // Doing this here to maintain the order of symbols if (!NextIs(HtmlSymbolType.CloseAngle)) @@ -575,9 +575,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return lastDoubleHyphen; } - internal static bool IsDashSymbol(HtmlSymbol symbol) + internal static bool IsHyphen(HtmlSymbol symbol) { - return string.Equals(symbol.Content, "-", StringComparison.Ordinal); + return symbol.Equals(HtmlSymbol.Hyphen); } protected bool IsHtmlCommentAhead() @@ -589,7 +589,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy * 1. The string "", or "--!>" + * 2.2 nor contain the strings + * 2.2.1 "" // As we will be treating this as a comment ending, there is no need to handle this case at all. + * 2.2.3 "--!>" * 2.3 nor end with the string "" * @@ -601,37 +604,42 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } // Check condition 2.1 - if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsDashSymbol(next) && NextIs(HtmlSymbolType.CloseAngle))) + if (NextIs(HtmlSymbolType.CloseAngle) || NextIs(next => IsHyphen(next) && NextIs(HtmlSymbolType.CloseAngle))) { return false; } // Check condition 2.2 var isValidComment = false; - LookaheadUntil((s, p) => + LookaheadUntil((symbol, prevSymbols) => { - if (s.Type == HtmlSymbolType.DoubleHyphen) + if (symbol.Type == HtmlSymbolType.DoubleHyphen) { 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 = !IsCommentContentDisallowed(p); + isValidComment = !IsCommentContentEndingInvalid(prevSymbols); return true; } - else if (NextIs(ns => IsDashSymbol(ns) && NextIs(HtmlSymbolType.CloseAngle))) + else if (NextIs(ns => IsHyphen(ns) && NextIs(HtmlSymbolType.CloseAngle))) { - // This is also a valid closing comment case, as the dashes lookup is treated with DoubleHyphen symbols first. + // Check condition 2.3: we're at the end of a comment, which has an extra dash. + // Need to treat the dash as part of the content and check the ending. + // However, that case would have already been checked as part of check from 2.2.1 which + // would already fail this iteration and we wouldn't get here isValidComment = true; return true; } else if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.CloseAngle))) { + // This is condition 2.2.3 isValidComment = false; return true; } } - else if (s.Type == HtmlSymbolType.OpenAngle) + else if (symbol.Type == HtmlSymbolType.OpenAngle) { + // Checking condition 2.2.1 if (NextIs(ns => ns.Type == HtmlSymbolType.Bang && NextIs(HtmlSymbolType.DoubleHyphen))) { isValidComment = false; @@ -648,7 +656,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy /// /// 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) + internal static bool IsCommentContentEndingInvalid(IEnumerable sequence) { var reversedSequence = sequence.Reverse(); var index = 0; diff --git a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs index f41f323d1d..217e293704 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/Legacy/HtmlSymbol.cs @@ -8,6 +8,8 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy { internal class HtmlSymbol : SymbolBase { + internal static readonly HtmlSymbol Hyphen = new HtmlSymbol("-", HtmlSymbolType.Text); + public HtmlSymbol(string content, HtmlSymbolType type) : base(content, type, RazorDiagnostic.EmptyArray) { diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs index a40ec2e422..cc12a33914 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/Legacy/HtmlMarkupParserTests.cs @@ -24,23 +24,23 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy [Theory] [MemberData(nameof(NonDashSymbols))] - public void IsDashSymbol_ReturnsFalseForNonDashSymbol(object symbol) + public void IsHyphen_ReturnsFalseForNonDashSymbol(object symbol) { // Arrange var convertedSymbol = (HtmlSymbol)symbol; // Act & Assert - Assert.False(HtmlMarkupParser.IsDashSymbol(convertedSymbol)); + Assert.False(HtmlMarkupParser.IsHyphen(convertedSymbol)); } [Fact] - public void IsDashSymbol_ReturnsTrueForADashSymbol() + public void IsHyphen_ReturnsTrueForADashSymbol() { // Arrange var dashSymbol = new HtmlSymbol("-", HtmlSymbolType.Text); // Act & Assert - Assert.True(HtmlMarkupParser.IsDashSymbol(dashSymbol)); + Assert.True(HtmlMarkupParser.IsHyphen(dashSymbol)); } [Fact] @@ -50,7 +50,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy var sut = CreateTestParserForContent("-->"); // Act - var symbol = sut.AcceptAllButLastDoubleHypens(); + var symbol = sut.AcceptAllButLastDoubleHyphens(); // Assert Assert.Equal(doubleHyphenSymbol, symbol); @@ -65,12 +65,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy var sut = CreateTestParserForContent("--->"); // Act - var symbol = sut.AcceptAllButLastDoubleHypens(); + var symbol = sut.AcceptAllButLastDoubleHyphens(); // Assert Assert.Equal(doubleHyphenSymbol, symbol); Assert.True(sut.At(HtmlSymbolType.CloseAngle)); - Assert.True(HtmlMarkupParser.IsDashSymbol(sut.PreviousSymbol)); + Assert.True(HtmlMarkupParser.IsHyphen(sut.PreviousSymbol)); } [Fact] @@ -103,6 +103,16 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy Assert.True(sut.IsHtmlCommentAhead()); } + [Fact] + public void IsHtmlCommentAhead_ReturnsFalseForContentWithBadEndingAndExtraDash() + { + // Arrange + var sut = CreateTestParserForContent("-- Some comment content in here "); + + // Act & Assert + Assert.False(sut.IsHtmlCommentAhead()); + } + [Fact] public void IsHtmlCommentAhead_ReturnsTrueForValidCommentTagWithExtraInfoAfter() { @@ -144,36 +154,36 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy } [Fact] - public void IsCommentContentDisallowed_ReturnsFalseForAllowedContent() + public void IsCommentContentEndingInvalid_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)); + Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence)); } [Fact] - public void IsCommentContentDisallowed_ReturnsTrueForDisallowedContent() + public void IsCommentContentEndingInvalid_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)); + Assert.True(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence)); } [Fact] - public void IsCommentContentDisallowed_ReturnsFalseForEmptyContent() + public void IsCommentContentEndingInvalid_ReturnsFalseForEmptyContent() { // Arrange var expectedSymbol1 = new HtmlSymbol("a", HtmlSymbolType.Text); var sequence = Array.Empty(); // Act & Assert - Assert.False(HtmlMarkupParser.IsCommentContentDisallowed(sequence)); + Assert.False(HtmlMarkupParser.IsCommentContentEndingInvalid(sequence)); } private class TestHtmlMarkupParser : HtmlMarkupParser @@ -193,9 +203,9 @@ namespace Microsoft.AspNetCore.Razor.Language.Test.Legacy this.EnsureCurrent(); } - public new HtmlSymbol AcceptAllButLastDoubleHypens() + public new HtmlSymbol AcceptAllButLastDoubleHyphens() { - return base.AcceptAllButLastDoubleHypens(); + return base.AcceptAllButLastDoubleHyphens(); } public override void BuildSpan(SpanBuilder span, SourceLocation start, string content)