diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs index 07c3fdf6af..f3d997d398 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/LinkTagHelper.cs @@ -4,14 +4,12 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Text; using System.Text.Encodings.Web; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Mvc.Razor.TagHelpers; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Mvc.TagHelpers.Internal; -using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Razor.TagHelpers; using Microsoft.Extensions.Caching.Memory; @@ -362,15 +360,21 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers firstAdded = true; } - // fallbackHrefs come from bound attributes and globbing. Must always be non-null. + // fallbackHrefs come from bound attributes (a C# context) and globbing. Must always be non-null. Debug.Assert(fallbackHrefs[i] != null); + var valueToWrite = fallbackHrefs[i]; if (AppendVersion == true) { valueToWrite = _fileVersionProvider.AddFileVersionToPath(fallbackHrefs[i]); } - builder.AppendHtml(JavaScriptEncoder.Encode(valueToWrite)); + // Must HTML-encode the href attribute value to ensure the written element is valid. Must also + // JavaScript-encode that value to ensure the doc.write() statement is valid. + valueToWrite = HtmlEncoder.Encode(valueToWrite); + valueToWrite = JavaScriptEncoder.Encode(valueToWrite); + + builder.AppendHtml(valueToWrite); builder.AppendHtml("\""); } builder.AppendHtml("]);"); diff --git a/src/Microsoft.AspNetCore.Mvc.TagHelpers/ScriptTagHelper.cs b/src/Microsoft.AspNetCore.Mvc.TagHelpers/ScriptTagHelper.cs index 96ad174303..397db49d8b 100644 --- a/src/Microsoft.AspNetCore.Mvc.TagHelpers/ScriptTagHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.TagHelpers/ScriptTagHelper.cs @@ -3,8 +3,11 @@ using System; using System.Diagnostics; +using System.IO; using System.Text.Encodings.Web; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Html; +using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.Razor.TagHelpers; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.Routing; @@ -39,6 +42,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers private const string AppendVersionAttributeName = "asp-append-version"; private static readonly Func Compare = (a, b) => a - b; private FileVersionProvider _fileVersionProvider; + private StringWriter _stringWriter; private static readonly ModeAttributes[] ModeDetails = new[] { // Regular src with file version alone @@ -174,6 +178,20 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers // Internal for ease of use when testing. protected internal GlobbingUrlBuilder GlobbingUrlBuilder { get; set; } + // Shared writer for determining the string content of a TagHelperAttribute's Value. + private StringWriter StringWriter + { + get + { + if (_stringWriter == null) + { + _stringWriter = new StringWriter(); + } + + return _stringWriter; + } + } + /// public override void Process(TagHelperContext context, TagHelperOutput output) { @@ -300,7 +318,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers if (!attribute.Name.Equals(SrcAttributeName, StringComparison.OrdinalIgnoreCase)) { var encodedKey = JavaScriptEncoder.Encode(attribute.Name); - var attributeValue = attribute.Value.ToString(); + var attributeValue = GetAttributeValue(attribute.Value); var encodedValue = JavaScriptEncoder.Encode(attributeValue); AppendAttribute(builder, encodedKey, encodedValue, escapeQuotes: true); @@ -324,6 +342,34 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } } + private string GetAttributeValue(object value) + { + string stringValue; + var htmlEncodedString = value as HtmlEncodedString; + if (htmlEncodedString != null) + { + // Value likely came from an HTML context in the .cshtml file but may still contain double quotes + // since attribute could have been enclosed in single quotes. + stringValue = htmlEncodedString.Value; + stringValue = stringValue.Replace("\"", """); + } + else + { + var writer = StringWriter; + RazorPage.WriteTo(writer, HtmlEncoder, value); + + // Value is now correctly HTML-encoded but may still contain double quotes since attribute could + // have been enclosed in single quotes and portions that were HtmlEncodedStrings are not re-encoded. + var builder = writer.GetStringBuilder(); + builder.Replace("\"", """); + + stringValue = builder.ToString(); + builder.Clear(); + } + + return stringValue; + } + private void AppendEncodedVersionedSrc( string srcName, string srcValue, @@ -337,6 +383,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers if (generateForDocumentWrite) { + // srcValue comes from a C# context and globbing. Must HTML-encode it to ensure the + // written + - + - + - + - + - + - + - + - + - + - + - + - + - + - + @@ -120,7 +120,7 @@ - + diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html index 585cf17fc6..0384f385c8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Link.html @@ -39,7 +39,7 @@ - + diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html index 433539425e..f696105479 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Script.Encoded.html @@ -1,4 +1,4 @@ - + @@ -13,27 +13,27 @@ - + - + - + - + - + - + - + - + - + - + - + "; + var context = MakeTagHelperContext( + attributes: new TagHelperAttributeList + { + { "asp-append-version", "true" }, + { "asp-fallback-href-include", "**/fallback.css" }, + { "asp-fallback-test-class", "hidden" }, + { "asp-fallback-test-property", "visibility" }, + { "asp-fallback-test-value", "hidden" }, + { "href", "/css/site.css" }, + }); + var output = MakeTagHelperOutput("link", attributes: new TagHelperAttributeList()); + var hostingEnvironment = MakeHostingEnvironment(); + var viewContext = MakeViewContext(); + var globbingUrlBuilder = new Mock( + new TestFileProvider(), + Mock.Of(), + PathString.Empty); + globbingUrlBuilder.Setup(g => g.BuildUrlList(null, "**/fallback.css", null)) + .Returns(new[] { "/fallback.css" }); + + var helper = new LinkTagHelper( + MakeHostingEnvironment(), + MakeCache(), + new HtmlTestEncoder(), + new JavaScriptTestEncoder(), + MakeUrlHelperFactory()) + { + AppendVersion = true, + Href = "/css/site.css", + FallbackHrefInclude = "**/fallback.css", + FallbackTestClass = "hidden", + FallbackTestProperty = "visibility", + FallbackTestValue = "hidden", + GlobbingUrlBuilder = globbingUrlBuilder.Object, + ViewContext = viewContext, + }; + + // Act + helper.Process(context, output); + + // Assert + Assert.Equal("link", output.TagName); + Assert.Equal("/css/site.css?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk", output.Attributes["href"].Value); + Assert.Equal(expectedPostElement, output.PostElement.GetContent()); + } + + [Fact] + public void RenderLinkTags_FallbackHref_WithFileVersion_EncodesAsExpected() + { + // Arrange + var expectedContent = "" + + Environment.NewLine + + "" + + ""; + var mixed = new DefaultTagHelperContent(); + mixed.Append("HTML encoded"); + mixed.AppendHtml(" and contains \"quotes\""); + var context = MakeTagHelperContext( + attributes: new TagHelperAttributeList + { + { "asp-append-version", "true" }, + { "asp-fallback-href-include", "**/fallback.css" }, + { "asp-fallback-test-class", "hidden" }, + { "asp-fallback-test-property", "visibility" }, + { "asp-fallback-test-value", "hidden" }, + { "encoded", new HtmlString("contains \"quotes\"") }, + { "href", "/css/site.css" }, + { "literal", "all HTML encoded" }, + { "mixed", mixed }, + }); + var output = MakeTagHelperOutput( + "link", + attributes: new TagHelperAttributeList + { + { "encoded", new HtmlString("contains \"quotes\"") }, + { "literal", "all HTML encoded" }, + { "mixed", mixed }, + }); + var hostingEnvironment = MakeHostingEnvironment(); + var viewContext = MakeViewContext(); + var globbingUrlBuilder = new Mock( + new TestFileProvider(), + Mock.Of(), + PathString.Empty); + globbingUrlBuilder.Setup(g => g.BuildUrlList(null, "**/fallback.css", null)) + .Returns(new[] { "/fallback.css" }); + + var helper = new LinkTagHelper( + MakeHostingEnvironment(), + MakeCache(), + new HtmlTestEncoder(), + new JavaScriptTestEncoder(), + MakeUrlHelperFactory()) + { + AppendVersion = true, + FallbackHrefInclude = "**/fallback.css", + FallbackTestClass = "hidden", + FallbackTestProperty = "visibility", + FallbackTestValue = "hidden", + GlobbingUrlBuilder = globbingUrlBuilder.Object, + Href = "/css/site.css", + ViewContext = viewContext, + }; + + // Act + helper.Process(context, output); + + // Assert + Assert.Equal("link", output.TagName); + Assert.Equal("/css/site.css?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk", output.Attributes["href"].Value); + var content = HtmlContentUtilities.HtmlContentToString(output, new HtmlTestEncoder()); + Assert.Equal(expectedContent, content); + } + + [Fact] + public void RendersLinkTags_GlobbedHref_WithFileVersion() { // Arrange var context = MakeTagHelperContext( @@ -848,7 +1005,10 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers tagName, attributes, getChildContentAsync: (useCachedResult, encoder) => Task.FromResult( - new DefaultTagHelperContent())); + new DefaultTagHelperContent())) + { + TagMode = TagMode.SelfClosing, + }; } private static IHostingEnvironment MakeHostingEnvironment() diff --git a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs index 95286d9a76..05c280f14a 100644 --- a/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs @@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Mvc.TagHelpers.Internal; +using Microsoft.AspNetCore.Mvc.TestCommon; using Microsoft.AspNetCore.Mvc.ViewEngines; using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Razor.TagHelpers; @@ -95,7 +96,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers [Theory] [MemberData(nameof(LinkTagHelperTest.MultiAttributeSameNameData), MemberType = typeof(LinkTagHelperTest))] - public async Task HandlesMultipleAttributesSameNameCorrectly(TagHelperAttributeList outputAttributes) + public void HandlesMultipleAttributesSameNameCorrectly(TagHelperAttributeList outputAttributes) { // Arrange var allAttributes = new TagHelperAttributeList( @@ -134,7 +135,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers expectedAttributes.Add(new TagHelperAttribute("src", "/blank.js")); // Act - await helper.ProcessAsync(tagHelperContext, output); + helper.Process(tagHelperContext, output); // Assert Assert.Equal(expectedAttributes, output.Attributes); @@ -265,7 +266,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers [Theory] [MemberData(nameof(RunsWhenRequiredAttributesArePresent_Data))] - public async Task RunsWhenRequiredAttributesArePresent( + public void RunsWhenRequiredAttributesArePresent( TagHelperAttributeList attributes, Action setProperties) { @@ -294,7 +295,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers setProperties(helper); // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.NotNull(output.TagName); @@ -362,7 +363,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers [Theory] [MemberData(nameof(RunsWhenRequiredAttributesArePresent_NoSrc_Data))] - public async Task RunsWhenRequiredAttributesArePresent_NoSrc( + public void RunsWhenRequiredAttributesArePresent_NoSrc( TagHelperAttributeList attributes, Action setProperties) { @@ -391,11 +392,12 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers setProperties(helper); // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Null(output.TagName); Assert.True(output.IsContentModified); + Assert.True(output.Content.IsEmpty); Assert.True(output.PostElement.IsModified); } @@ -498,7 +500,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } [Fact] - public async Task DoesNotRunWhenAllRequiredAttributesAreMissing() + public void DoesNotRunWhenAllRequiredAttributesAreMissing() { // Arrange var tagHelperContext = MakeTagHelperContext(); @@ -516,7 +518,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(tagHelperContext, output); + helper.Process(tagHelperContext, output); // Assert Assert.Equal("script", output.TagName); @@ -526,7 +528,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } [Fact] - public async Task PreservesOrderOfNonSrcAttributes() + public void PreservesOrderOfNonSrcAttributes() { // Arrange var tagHelperContext = MakeTagHelperContext( @@ -564,7 +566,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(tagHelperContext, output); + helper.Process(tagHelperContext, output); // Assert Assert.Equal("data-extra", output.Attributes[0].Name); @@ -573,9 +575,11 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } [Fact] - public async Task RendersScriptTagsForGlobbedSrcResults() + public void RendersScriptTagsForGlobbedSrcResults() { // Arrange + var expectedContent = "" + + ""; var context = MakeTagHelperContext( attributes: new TagHelperAttributeList { @@ -606,25 +610,46 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Equal("script", output.TagName); Assert.Equal("/js/site.js", output.Attributes["src"].Value); - Assert.Equal("", output.PostElement.GetContent()); + var content = HtmlContentUtilities.HtmlContentToString(output, new HtmlTestEncoder()); + Assert.Equal(expectedContent, content); } [Fact] - public async Task RendersScriptTagsForGlobbedSrcResults_UsesProvidedEncoder() + public void RendersScriptTagsForGlobbedSrcResults_EncodesAsExpected() { // Arrange + var expectedContent = + "" + + ""; + var mixed = new DefaultTagHelperContent(); + mixed.Append("HTML encoded"); + mixed.AppendHtml(" and contains \"quotes\""); var context = MakeTagHelperContext( attributes: new TagHelperAttributeList { - new TagHelperAttribute("src", "/js/site.js"), - new TagHelperAttribute("asp-src-include", "**/*.js") + { "asp-src-include", "**/*.js" }, + { "encoded", new HtmlString("contains \"quotes\"") }, + { "literal", "all HTML encoded" }, + { "mixed", mixed }, + { "src", "/js/site.js" }, + }); + var output = MakeTagHelperOutput( + "script", + attributes: new TagHelperAttributeList + { + { "encoded", new HtmlString("contains \"quotes\"") }, + { "literal", "all HTML encoded"}, + { "mixed", mixed}, }); - var output = MakeTagHelperOutput("script", attributes: new TagHelperAttributeList()); var hostingEnvironment = MakeHostingEnvironment(); var viewContext = MakeViewContext(); var globbingUrlBuilder = new Mock( @@ -642,22 +667,23 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers MakeUrlHelperFactory()) { GlobbingUrlBuilder = globbingUrlBuilder.Object, - ViewContext = viewContext, Src = "/js/site.js", SrcInclude = "**/*.js", + ViewContext = viewContext, }; // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Equal("script", output.TagName); Assert.Equal("/js/site.js", output.Attributes["src"].Value); - Assert.Equal("", output.PostElement.GetContent()); + var content = HtmlContentUtilities.HtmlContentToString(output, new HtmlTestEncoder()); + Assert.Equal(expectedContent, content); } [Fact] - public async Task RenderScriptTags_WithFileVersion() + public void RenderScriptTags_WithFileVersion() { // Arrange var context = MakeTagHelperContext( @@ -684,7 +710,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Equal("script", output.TagName); @@ -692,7 +718,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } [Fact] - public async Task RenderScriptTags_WithFileVersion_AndRequestPathBase() + public void RenderScriptTags_WithFileVersion_AndRequestPathBase() { // Arrange var context = MakeTagHelperContext( @@ -718,7 +744,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Equal("script", output.TagName); @@ -726,7 +752,7 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers } [Fact] - public async Task RenderScriptTags_FallbackSrc_WithFileVersion() + public void RenderScriptTags_FallbackSrc_WithFileVersion() { // Arrange var context = MakeTagHelperContext( @@ -756,20 +782,87 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Equal("script", output.TagName); Assert.Equal("/js/site.js?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk", output.Attributes["src"].Value); - Assert.Equal(Environment.NewLine + "", output.PostElement.GetContent()); + Assert.Equal(Environment.NewLine + "", output.PostElement.GetContent()); } [Fact] - public async Task RenderScriptTags_GlobbedSrc_WithFileVersion() + public void RenderScriptTags_FallbackSrc_WithFileVersion_EncodesAsExpected() { // Arrange + var expectedContent = + "" + + Environment.NewLine + + ""; + var mixed = new DefaultTagHelperContent(); + mixed.Append("HTML encoded"); + mixed.AppendHtml(" and contains \"quotes\""); + var context = MakeTagHelperContext( + attributes: new TagHelperAttributeList + { + { "asp-append-version", "true" }, + { "asp-fallback-src-include", "fallback.js" }, + { "asp-fallback-test", "isavailable()" }, + { "encoded", new HtmlString("contains \"quotes\"") }, + { "literal", "all HTML encoded" }, + { "mixed", mixed }, + { "src", "/js/site.js" }, + }); + var output = MakeTagHelperOutput( + "script", + attributes: new TagHelperAttributeList + { + { "encoded", new HtmlString("contains \"quotes\"") }, + { "literal", "all HTML encoded" }, + { "mixed", mixed }, + }); + var hostingEnvironment = MakeHostingEnvironment(); + var viewContext = MakeViewContext(); + + var helper = new ScriptTagHelper( + MakeHostingEnvironment(), + MakeCache(), + new HtmlTestEncoder(), + new JavaScriptTestEncoder(), + MakeUrlHelperFactory()) + { + AppendVersion = true, + FallbackSrc = "fallback.js", + FallbackTestExpression = "isavailable()", + Src = "/js/site.js", + ViewContext = viewContext, + }; + + // Act + helper.Process(context, output); + + // Assert + Assert.Equal("script", output.TagName); + Assert.Equal("/js/site.js?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk", output.Attributes["src"].Value); + var content = HtmlContentUtilities.HtmlContentToString(output, new HtmlTestEncoder()); + Assert.Equal(expectedContent, content); + } + + [Fact] + public void RenderScriptTags_GlobbedSrc_WithFileVersion() + { + // Arrange + var expectedContent = "" + + ""; var context = MakeTagHelperContext( attributes: new TagHelperAttributeList { @@ -802,13 +895,13 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers }; // Act - await helper.ProcessAsync(context, output); + helper.Process(context, output); // Assert Assert.Equal("script", output.TagName); Assert.Equal("/js/site.js?v=f4OxZX_x_FO5LcGBSKHWXfwtSx-j1ncoSt3SABJtkGk", output.Attributes["src"].Value); - Assert.Equal("", output.PostElement.GetContent()); + var content = HtmlContentUtilities.HtmlContentToString(output, new HtmlTestEncoder()); + Assert.Equal(expectedContent, content); } private TagHelperContext MakeTagHelperContext(