From 0665d30e190d809115c0bc84fc3a0e6255dfeb45 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Wed, 21 Feb 2018 22:31:23 +0000 Subject: [PATCH] In RazorCompiler, reject malformed documents with mismatching tags --- .../Cli/Commands/BuildRazorCommand.cs | 1 + .../BlazorIntermediateNodeWriter.cs | 58 +++++++++------- .../RazorCompilerDiagnostic.cs | 4 +- .../RazorCompilerException.cs | 6 +- .../ScopeStack.cs | 68 +++++++++++++++++++ .../RazorCompilerTest.cs | 40 +++++++++++ 6 files changed, 149 insertions(+), 28 deletions(-) rename src/{Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation => Microsoft.AspNetCore.Blazor.Razor.Extensions}/RazorCompilerDiagnostic.cs (91%) rename src/{Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation => Microsoft.AspNetCore.Blazor.Razor.Extensions}/RazorCompilerException.cs (80%) create mode 100644 src/Microsoft.AspNetCore.Blazor.Razor.Extensions/ScopeStack.cs diff --git a/src/Microsoft.AspNetCore.Blazor.Build/Cli/Commands/BuildRazorCommand.cs b/src/Microsoft.AspNetCore.Blazor.Build/Cli/Commands/BuildRazorCommand.cs index 467a5878e6..13b3ff60d0 100644 --- a/src/Microsoft.AspNetCore.Blazor.Build/Cli/Commands/BuildRazorCommand.cs +++ b/src/Microsoft.AspNetCore.Blazor.Build/Cli/Commands/BuildRazorCommand.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation; +using Microsoft.AspNetCore.Blazor.Razor; using Microsoft.Extensions.CommandLineUtils; using System; using System.Collections.Generic; diff --git a/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorIntermediateNodeWriter.cs b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorIntermediateNodeWriter.cs index dd5b10580e..769244aa28 100644 --- a/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorIntermediateNodeWriter.cs +++ b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/BlazorIntermediateNodeWriter.cs @@ -27,6 +27,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor new[] { "area", "base", "br", "col", "embed", "hr", "img", "input", "link", "meta", "param", "source", "track", "wbr" }, StringComparer.OrdinalIgnoreCase); + private readonly ScopeStack _scopeStack = new ScopeStack(); private string _unconsumedHtml; private IList _currentAttributeValues; private IDictionary _currentElementAttributes = new Dictionary(); @@ -75,6 +76,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor { if (node.Children[i] is IntermediateToken token && token.IsCSharp) { + _scopeStack.IncrementCurrentScopeChildCount(); context.CodeWriter.Write(token.Content); } else @@ -118,6 +120,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor // Since we're not in the middle of writing an element, this must evaluate as some // text to display + _scopeStack.IncrementCurrentScopeChildCount(); context.CodeWriter .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.AddContent)}") .Write((_sourceSequence++).ToString()) @@ -207,6 +210,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor case HtmlTokenType.Character: { // Text node + _scopeStack.IncrementCurrentScopeChildCount(); codeWriter .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.AddContent)}") .Write((_sourceSequence++).ToString()) @@ -225,6 +229,7 @@ namespace Microsoft.AspNetCore.Blazor.Razor if (nextToken.Type == HtmlTokenType.StartTag) { + _scopeStack.IncrementCurrentScopeChildCount(); if (isComponent) { codeWriter @@ -241,40 +246,47 @@ namespace Microsoft.AspNetCore.Blazor.Razor .WriteStringLiteral(nextTag.Data) .WriteEndMethodInvocation(); } - } - foreach (var attribute in nextTag.Attributes) - { - WriteAttribute(codeWriter, attribute.Key, attribute.Value); - } - - if (_currentElementAttributes.Count > 0) - { - foreach (var pair in _currentElementAttributes) + foreach (var attribute in nextTag.Attributes) { - WriteAttribute(codeWriter, pair.Key, pair.Value.AttributeValue); + WriteAttribute(codeWriter, attribute.Key, attribute.Value); } - _currentElementAttributes.Clear(); - } - if (_currentElementAttributeTokens.Count > 0) - { - foreach (var token in _currentElementAttributeTokens) + if (_currentElementAttributes.Count > 0) { - codeWriter - .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.AddAttribute)}") - .Write((_sourceSequence++).ToString()) - .WriteParameterSeparator() - .Write(token.AttributeValue.Content) - .WriteEndMethodInvocation(); + foreach (var pair in _currentElementAttributes) + { + WriteAttribute(codeWriter, pair.Key, pair.Value.AttributeValue); + } + _currentElementAttributes.Clear(); } - _currentElementAttributeTokens.Clear(); + + if (_currentElementAttributeTokens.Count > 0) + { + foreach (var token in _currentElementAttributeTokens) + { + codeWriter + .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.AddAttribute)}") + .Write((_sourceSequence++).ToString()) + .WriteParameterSeparator() + .Write(token.AttributeValue.Content) + .WriteEndMethodInvocation(); + } + _currentElementAttributeTokens.Clear(); + } + + _scopeStack.OpenScope( + tagName: isComponent ? tagNameOriginalCase : nextTag.Data, + isComponent: isComponent); } if (nextToken.Type == HtmlTokenType.EndTag || nextTag.IsSelfClosing - || htmlVoidElementsLookup.Contains(nextTag.Data)) + || (!isComponent && htmlVoidElementsLookup.Contains(nextTag.Data))) { + _scopeStack.CloseScope( + tagName: isComponent ? tagNameOriginalCase : nextTag.Data, + isComponent: isComponent); var closeMethodName = isComponent ? nameof(RenderTreeBuilder.CloseComponent) : nameof(RenderTreeBuilder.CloseElement); diff --git a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompilerDiagnostic.cs b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/RazorCompilerDiagnostic.cs similarity index 91% rename from src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompilerDiagnostic.cs rename to src/Microsoft.AspNetCore.Blazor.Razor.Extensions/RazorCompilerDiagnostic.cs index 0e632d82cb..f5b41a545a 100644 --- a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompilerDiagnostic.cs +++ b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/RazorCompilerDiagnostic.cs @@ -1,7 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation +namespace Microsoft.AspNetCore.Blazor.Razor { public class RazorCompilerDiagnostic { @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation public int Column { get; } public string Message { get; } - internal RazorCompilerDiagnostic( + public RazorCompilerDiagnostic( DiagnosticType type, string sourceFilePath, int line, diff --git a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompilerException.cs b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/RazorCompilerException.cs similarity index 80% rename from src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompilerException.cs rename to src/Microsoft.AspNetCore.Blazor.Razor.Extensions/RazorCompilerException.cs index 519f5df09f..9061ad1aed 100644 --- a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompilerException.cs +++ b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/RazorCompilerException.cs @@ -3,15 +3,15 @@ using System; -namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation +namespace Microsoft.AspNetCore.Blazor.Razor { /// /// Represents a fatal error during the transformation of a Blazor component from /// Razor source code to C# source code. /// - internal class RazorCompilerException : Exception + public class RazorCompilerException : Exception { - public RazorCompilerException(string message): base(message) + public RazorCompilerException(string message) : base(message) { } diff --git a/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/ScopeStack.cs b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/ScopeStack.cs new file mode 100644 index 0000000000..70b1eb28b6 --- /dev/null +++ b/src/Microsoft.AspNetCore.Blazor.Razor.Extensions/ScopeStack.cs @@ -0,0 +1,68 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Razor.Language; +using System; +using System.Collections.Generic; + +namespace Microsoft.AspNetCore.Blazor.Razor +{ + /// + /// Keeps track of the nesting of elements/containers while writing out the C# source code + /// for a component. This allows us to detect mismatched start/end tags, as well as inject + /// additional C# source to capture component descendants in a lambda. + /// + internal class ScopeStack + { + private readonly Stack _stack = new Stack(); + + public void OpenScope(string tagName, bool isComponent) + { + _stack.Push(new ScopeEntry(tagName, isComponent)); + } + + public void CloseScope(string tagName, bool isComponent) + { + if (_stack.Count == 0) + { + throw new RazorCompilerException( + $"Unexpected closing tag '{tagName}' with no matching start tag."); + } + + var expected = _stack.Pop(); + if (!tagName.Equals(expected.TagName, StringComparison.Ordinal)) + { + throw new RazorCompilerException( + $"Mismatching closing tag. Found '{tagName}' but expected '{expected.TagName}'."); + } + + // Note: there's no unit test to cover the following, because there's no known way of + // triggering it from user code (i.e., Razor source code). But the test is here anyway + // just in case one day it turns out there is some way of causing this error. + if (isComponent != expected.IsComponent) + { + throw new RazorCompilerException( + $"Mismatching closing tag. Found '{tagName}' of type '{(isComponent ? "component" : "element")}' but expected type '{(expected.IsComponent ? "component" : "element")}'."); + } + } + + public void IncrementCurrentScopeChildCount() + { + + } + + private class ScopeEntry + { + public readonly string TagName; + public readonly bool IsComponent; + public int ChildCount; + + public ScopeEntry(string tagName, bool isComponent) + { + TagName = tagName; + IsComponent = isComponent; + ChildCount = 0; + } + } + } +} diff --git a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs index 597ba8cd5d..dc5934df37 100644 --- a/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs @@ -4,6 +4,7 @@ using Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation; using Microsoft.AspNetCore.Blazor.Components; using Microsoft.AspNetCore.Blazor.Layouts; +using Microsoft.AspNetCore.Blazor.Razor; using Microsoft.AspNetCore.Blazor.Rendering; using Microsoft.AspNetCore.Blazor.RenderTree; using Microsoft.AspNetCore.Blazor.Test.Helpers; @@ -503,6 +504,38 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test diagnostic => Assert.Contains("'invalidVar'", diagnostic.GetMessage())); } + [Fact] + public void RejectsEndTagWithNoStartTag() + { + // Arrange/Act + var result = CompileToCSharp( + "Line1\nLine2\nLine3"); + + // Assert + Assert.Collection(result.Diagnostics, + item => + { + Assert.Equal(RazorCompilerDiagnostic.DiagnosticType.Error, item.Type); + Assert.StartsWith("Unexpected closing tag 'mytag' with no matching start tag.", item.Message); + }); + } + + [Fact] + public void RejectsEndTagWithDifferentNameToStartTag() + { + // Arrange/Act + var result = CompileToCSharp( + "textmore text"); + + // Assert + Assert.Collection(result.Diagnostics, + item => + { + Assert.Equal(RazorCompilerDiagnostic.DiagnosticType.Error, item.Type); + Assert.StartsWith("Mismatching closing tag. Found 'root' but expected 'child'.", item.Message); + }); + } + private static RenderTreeFrame[] GetRenderTree(IComponent component) { var renderer = new TestRenderer(); @@ -575,6 +608,13 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test } } + private static CompileToCSharpResult CompileToCSharp(string cshtmlContent) + => CompileToCSharp( + GetArbitraryPlatformValidDirectoryPath(), + "test.cshtml", + cshtmlContent, + "TestNamespace"); + private static CompileToCSharpResult CompileToCSharp(string cshtmlRootPath, string cshtmlRelativePath, string cshtmlContent, string outputNamespace) { using (var resultStream = new MemoryStream())