From 1f76b3c7b79e6bde573d3b3397d996a3ea2b0038 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Thu, 3 Dec 2015 15:35:42 -0800 Subject: [PATCH] Clean up remaining `HtmlString` usage - #3122 and #3123 (5 of 5) - avoid concatenating values and wrapping them in an `HtmlString` - switch from `string.Format()` to `AppendHtml()` in `LinkTagHelper` - simplify `RazorPage` and `TagHelperContentExtensions` e.g. don't use a `TagHelperContentWrapperTextWriter` - add some special cases in `UrlResolutionTagHelper` - add `TagBuilder` and `StringHtmlContent` special cases to avoid `StringWriter` use when value is an `HtmlString` - move `HtmlTextWriter` optimizations a bit lower and add missing checks for that type nits: - correct comments that incorrectly mention `HtmlString`s - update comments in `JavaScriptStringArrayEncoder` --- src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs | 24 +-- .../TagHelperContentWrapperTextWriter.cs | 1 - .../TagHelpers/UrlResolutionTagHelper.cs | 152 +++++++++++++----- .../Internal/JavaScriptResources.cs | 13 +- .../Internal/JavaScriptStringArrayEncoder.cs | 11 +- .../LinkTagHelper.cs | 22 +-- .../ScriptTagHelper.cs | 6 +- .../TagHelperContentExtensions.cs | 36 +++-- .../js/LinkTagHelper_FallbackJavaScript.js | 2 +- .../Rendering/HtmlHelperPartialExtensions.cs | 14 +- .../Rendering/IHtmlHelper.cs | 2 +- .../Rendering/TagBuilder.cs | 10 +- .../ViewFeatures/HtmlHelper.cs | 26 +-- .../ViewFeatures/StringHtmlContent.cs | 12 +- .../RazorPageTest.cs | 9 +- .../TagHelpers/UrlResolutionTagHelperTest.cs | 150 ++++++++++++++--- .../Internal/JavaScriptResourcesTest.cs | 17 +- 17 files changed, 333 insertions(+), 174 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs b/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs index 394888a517..0970302dd6 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/RazorPage.cs @@ -602,22 +602,10 @@ namespace Microsoft.AspNet.Mvc.Razor { if (!_tagHelperAttributeInfo.Suppressed) { - HtmlString htmlString; - - if (_valueBuffer != null) - { - using (var stringWriter = new StringWriter()) - { - _valueBuffer.Content.WriteTo(stringWriter, HtmlEncoder); - htmlString = new HtmlString(stringWriter.ToString()); - } - } - else - { - htmlString = HtmlString.Empty; - } - - executionContext.AddHtmlAttribute(_tagHelperAttributeInfo.Name, htmlString); + executionContext.AddHtmlAttribute( + _tagHelperAttributeInfo.Name, + _valueBuffer?.Content ?? HtmlString.Empty); + _valueBuffer = null; } } @@ -843,7 +831,7 @@ namespace Microsoft.AspNet.Mvc.Razor /// content to the . /// /// A that represents the asynchronous flush operation and on - /// completion returns a . + /// completion returns . /// The value returned is a token value that allows FlushAsync to work directly in an HTML /// section. However the value does not represent the rendered content. /// This method also writes out headers, so any modifications to headers must be done before @@ -936,7 +924,7 @@ namespace Microsoft.AspNet.Mvc.Razor /// /// Sets antiforgery cookie and X-Frame-Options header on the response. /// - /// A that returns a . + /// . /// Call this method to send antiforgery cookie token and X-Frame-Options header to client /// before flushes the headers. public virtual HtmlString SetAntiforgeryCookieAndHeader() diff --git a/src/Microsoft.AspNet.Mvc.Razor/TagHelperContentWrapperTextWriter.cs b/src/Microsoft.AspNet.Mvc.Razor/TagHelperContentWrapperTextWriter.cs index ebe22dfa42..2c58e06e7c 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/TagHelperContentWrapperTextWriter.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/TagHelperContentWrapperTextWriter.cs @@ -4,7 +4,6 @@ using System; using System.Text; using Microsoft.AspNet.Html; -using Microsoft.AspNet.Mvc.ViewFeatures; using Microsoft.AspNet.Razor.TagHelpers; namespace Microsoft.AspNet.Mvc.Razor diff --git a/src/Microsoft.AspNet.Mvc.Razor/TagHelpers/UrlResolutionTagHelper.cs b/src/Microsoft.AspNet.Mvc.Razor/TagHelpers/UrlResolutionTagHelper.cs index a30c793b89..2bba2e0cb0 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/TagHelpers/UrlResolutionTagHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/TagHelpers/UrlResolutionTagHelper.cs @@ -4,8 +4,10 @@ using System; using System.Collections.Generic; using System.ComponentModel; +using System.IO; using System.Reflection; using System.Text.Encodings.Web; +using Microsoft.AspNet.Html; using Microsoft.AspNet.Mvc.Rendering; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Mvc.ViewFeatures; @@ -160,26 +162,45 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers { foreach (var attribute in attributes) { - string resolvedUrl; - var stringValue = attribute.Value as string; if (stringValue != null) { - if (TryResolveUrl(stringValue, encodeWebRoot: false, resolvedUrl: out resolvedUrl)) + string resolvedUrl; + if (TryResolveUrl(stringValue, resolvedUrl: out resolvedUrl)) { attribute.Value = resolvedUrl; } } else { - var htmlStringValue = attribute.Value as HtmlString; - if (htmlStringValue != null && - TryResolveUrl( - htmlStringValue.ToString(), - encodeWebRoot: true, - resolvedUrl: out resolvedUrl)) + var htmlContent = attribute.Value as IHtmlContent; + if (htmlContent != null) { - attribute.Value = new HtmlString(resolvedUrl); + var htmlString = htmlContent as HtmlString; + if (htmlString != null) + { + // No need for a StringWriter in this case. + stringValue = htmlString.ToString(); + } + else + { + using (var writer = new StringWriter()) + { + htmlContent.WriteTo(writer, HtmlEncoder); + stringValue = writer.ToString(); + } + } + + IHtmlContent resolvedUrl; + if (TryResolveUrl(stringValue, resolvedUrl: out resolvedUrl)) + { + attribute.Value = resolvedUrl; + } + else if (htmlString == null) + { + // Not a ~/ URL. Just avoid re-encoding the attribute value later. + attribute.Value = new HtmlString(stringValue); + } } } } @@ -190,14 +211,12 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers /// Tries to resolve the given value relative to the application's 'webroot' setting. /// /// The URL to resolve. - /// If true, will HTML encode the expansion of '~/'. /// Absolute URL beginning with the application's virtual root. null if /// could not be resolved. /// true if the could be resolved; false otherwise. - protected bool TryResolveUrl(string url, bool encodeWebRoot, out string resolvedUrl) + protected bool TryResolveUrl(string url, out string resolvedUrl) { resolvedUrl = null; - if (url == null) { return false; @@ -205,42 +224,93 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers var trimmedUrl = url.Trim(ValidAttributeWhitespaceChars); - // Before doing more work, ensure that the URL we're looking at is app relative. + // Before doing more work, ensure that the URL we're looking at is app-relative. if (trimmedUrl.Length >= 2 && trimmedUrl[0] == '~' && trimmedUrl[1] == '/') { var urlHelper = UrlHelperFactory.GetUrlHelper(ViewContext); - var appRelativeUrl = urlHelper.Content(trimmedUrl); - - if (encodeWebRoot) - { - var postTildeSlashUrlValue = trimmedUrl.Substring(2); - - if (!appRelativeUrl.EndsWith(postTildeSlashUrlValue, StringComparison.Ordinal)) - { - throw new InvalidOperationException( - Resources.FormatCouldNotResolveApplicationRelativeUrl_TagHelper( - url, - nameof(IUrlHelper), - nameof(IUrlHelper.Content), - "removeTagHelper", - typeof(UrlResolutionTagHelper).FullName, - typeof(UrlResolutionTagHelper).GetTypeInfo().Assembly.GetName().Name)); - } - - var applicationPath = appRelativeUrl.Substring(0, appRelativeUrl.Length - postTildeSlashUrlValue.Length); - var encodedApplicationPath = HtmlEncoder.Encode(applicationPath); - - resolvedUrl = string.Concat(encodedApplicationPath, postTildeSlashUrlValue); - } - else - { - resolvedUrl = appRelativeUrl; - } + resolvedUrl = urlHelper.Content(trimmedUrl); return true; } return false; } + + /// + /// Tries to resolve the given value relative to the application's 'webroot' setting. + /// + /// The URL to resolve. + /// + /// Absolute URL beginning with the application's virtual root. null if could + /// not be resolved. + /// + /// true if the could be resolved; false otherwise. + protected bool TryResolveUrl(string url, out IHtmlContent resolvedUrl) + { + resolvedUrl = null; + if (url == null) + { + return false; + } + + var trimmedUrl = url.Trim(ValidAttributeWhitespaceChars); + + // Before doing more work, ensure that the URL we're looking at is app-relative. + if (trimmedUrl.Length >= 2 && trimmedUrl[0] == '~' && trimmedUrl[1] == '/') + { + var urlHelper = UrlHelperFactory.GetUrlHelper(ViewContext); + var appRelativeUrl = urlHelper.Content(trimmedUrl); + var postTildeSlashUrlValue = trimmedUrl.Substring(2); + + if (!appRelativeUrl.EndsWith(postTildeSlashUrlValue, StringComparison.Ordinal)) + { + throw new InvalidOperationException( + Resources.FormatCouldNotResolveApplicationRelativeUrl_TagHelper( + url, + nameof(IUrlHelper), + nameof(IUrlHelper.Content), + "removeTagHelper", + typeof(UrlResolutionTagHelper).FullName, + typeof(UrlResolutionTagHelper).GetTypeInfo().Assembly.GetName().Name)); + } + + resolvedUrl = new EncodeFirstSegmentContent( + appRelativeUrl, + appRelativeUrl.Length - postTildeSlashUrlValue.Length, + postTildeSlashUrlValue); + + return true; + } + + return false; + } + + private class EncodeFirstSegmentContent : IHtmlContent + { + private readonly string _firstSegment; + private readonly int _firstSegmentLength; + private readonly string _secondSegment; + + public EncodeFirstSegmentContent(string firstSegment, int firstSegmentLength, string secondSegment) + { + _firstSegment = firstSegment; + _firstSegmentLength = firstSegmentLength; + _secondSegment = secondSegment; + } + + public void WriteTo(TextWriter writer, HtmlEncoder encoder) + { + var htmlTextWriter = writer as HtmlTextWriter; + if (htmlTextWriter != null) + { + htmlTextWriter.Write(this); + } + else + { + encoder.Encode(writer, _firstSegment, 0, _firstSegmentLength); + writer.Write(_secondSegment); + } + } + } } } diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptResources.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptResources.cs index 3067b5860f..070fe6acd5 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptResources.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptResources.cs @@ -38,7 +38,9 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal // Load the JavaScript from embedded resource using (var resourceStream = getManifestResourceStream(key)) { - Debug.Assert(resourceStream != null, "Embedded resource missing. Ensure 'prebuild' script has run."); + Debug.Assert( + resourceStream != null, + "Embedded resource missing. Ensure 'prebuild' script has run."); using (var streamReader = new StreamReader(resourceStream)) { @@ -49,14 +51,11 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal } }); } - + private static string PrepareFormatString(string input) { - // Replace unescaped/escaped chars with their equivalent - return input.Replace("{", "{{") - .Replace("}", "}}") - .Replace("[[[", "{") - .Replace("]]]", "}"); + // Remove final ");". Those characters are in the file only to allow minification. + return input.Substring(0, input.Length - 2); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptStringArrayEncoder.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptStringArrayEncoder.cs index 104c7c7ef8..983d1c83ed 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptStringArrayEncoder.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/Internal/JavaScriptStringArrayEncoder.cs @@ -1,35 +1,34 @@ // 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 System.Collections.Generic; using System.IO; using System.Text.Encodings.Web; namespace Microsoft.AspNet.Mvc.TagHelpers.Internal { /// - /// Methods for encoding for use as a JavaScript array literal. + /// Methods for encoding string[]s for use as a JavaScript array literal. /// public static class JavaScriptStringArrayEncoder { /// - /// Encodes a .NET string array for safe use as a JavaScript array literal, including inline in an HTML file. + /// Encodes a string[] for safe use as a JavaScript array literal in many contexts, including + /// inline in an HTML file. /// public static string Encode(JavaScriptEncoder encoder, string[] values) { var writer = new StringWriter(); - - var firstAdded = false; - writer.Write('['); // Perf: Avoid allocating enumerator + var firstAdded = false; for (var i = 0; i < values.Length; i++) { if (firstAdded) { writer.Write(','); } + writer.Write('"'); encoder.Encode(writer, values[i]); writer.Write('"'); diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs index fc747bb08a..451055fba1 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/LinkTagHelper.cs @@ -12,7 +12,6 @@ using Microsoft.AspNet.Mvc.Rendering; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Mvc.TagHelpers.Internal; using Microsoft.AspNet.Mvc.TagHelpers.Logging; -using Microsoft.AspNet.Mvc.ViewFeatures; using Microsoft.AspNet.Razor.TagHelpers; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; @@ -287,7 +286,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers if (mode == Mode.Fallback) { string resolvedUrl; - if (TryResolveUrl(FallbackHref, encodeWebRoot: false, resolvedUrl: out resolvedUrl)) + if (TryResolveUrl(FallbackHref, resolvedUrl: out resolvedUrl)) { FallbackHref = resolvedUrl; } @@ -350,16 +349,17 @@ namespace Microsoft.AspNet.Mvc.TagHelpers // Build the "); + .AppendHtml(JavaScriptResources.GetEmbeddedJavaScript(FallbackJavaScriptResourceName)) + .AppendHtml("\"") + .AppendHtml(JavaScriptEncoder.Encode(FallbackTestProperty)) + .AppendHtml("\",\"") + .AppendHtml(JavaScriptEncoder.Encode(FallbackTestValue)) + .AppendHtml("\",") + .AppendHtml(JavaScriptStringArrayEncoder.Encode(JavaScriptEncoder, fallbackHrefs)) + .AppendHtml(");"); } } @@ -410,7 +410,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers builder .AppendHtml(attribute.Name) .AppendHtml("=\"") - .Append(HtmlEncoder, ViewContext.Writer.Encoding, attributeValue) + .Append(HtmlEncoder, attributeValue) .AppendHtml("\" "); } diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs index 1313059efc..dbe7cbe1a0 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/ScriptTagHelper.cs @@ -254,7 +254,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers if (mode == Mode.Fallback) { string resolvedUrl; - if (TryResolveUrl(FallbackSrc, encodeWebRoot: false, resolvedUrl: out resolvedUrl)) + if (TryResolveUrl(FallbackSrc, resolvedUrl: out resolvedUrl)) { FallbackSrc = resolvedUrl; } @@ -418,10 +418,10 @@ namespace Microsoft.AspNet.Mvc.TagHelpers } else { - // HTML-encoded the given value if necessary. + // HTML-encode the given value if necessary. content .AppendHtml("=\"") - .Append(HtmlEncoder, ViewContext.Writer.Encoding, value) + .Append(HtmlEncoder, value) .AppendHtml("\""); } } diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperContentExtensions.cs b/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperContentExtensions.cs index 3a3db98697..b24da3fe48 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperContentExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/TagHelperContentExtensions.cs @@ -3,9 +3,9 @@ using System; using System.IO; -using System.Text; using System.Text.Encodings.Web; using Microsoft.AspNet.Mvc.Razor; +using Microsoft.AspNet.Mvc.Rendering; using Microsoft.AspNet.Razor.TagHelpers; namespace Microsoft.AspNet.Mvc.TagHelpers @@ -20,7 +20,6 @@ namespace Microsoft.AspNet.Mvc.TagHelpers /// /// The to write to. /// The to use when encoding . - /// The character encoding in which the is written. /// The to write. /// after the write operation has completed. /// @@ -29,11 +28,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers /// For all other types, the encoded result of /// is written to the . /// - public static TagHelperContent Append( - this TagHelperContent content, - HtmlEncoder encoder, - Encoding encoding, - object value) + public static TagHelperContent Append(this TagHelperContent content, HtmlEncoder encoder, object value) { if (content == null) { @@ -45,25 +40,34 @@ namespace Microsoft.AspNet.Mvc.TagHelpers throw new ArgumentNullException(nameof(encoder)); } - if (encoding == null) + if (value == null) { - throw new ArgumentNullException(nameof(encoding)); + // No real action but touch content to ensure IsModified is true. + content.Append((string)null); + return content; } - using (var writer = new TagHelperContentWrapperTextWriter(encoding, content)) + string stringValue; + var htmlString = value as HtmlString; + if (htmlString != null) + { + // No need for a StringWriter in this case. + stringValue = htmlString.ToString(); + } + else { using (var stringWriter = new StringWriter()) { RazorPage.WriteTo(stringWriter, encoder, value); - - // In this case the text likely came directly from the Razor source. Since the original string is - // an attribute value that may have been quoted with single quotes, must handle any double quotes - // in the value. Writing the value out surrounded by double quotes. - var stringValue = stringWriter.ToString().Replace("\"", """); - writer.Write(stringValue); + stringValue = stringWriter.ToString(); } } + // In this case the text likely came directly from the Razor source. Since the original string is + // an attribute value that may have been quoted with single quotes, must handle any double quotes + // in the value. Writing the value out surrounded by double quotes. + content.AppendHtml(stringValue.Replace("\"", """)); + return content; } } diff --git a/src/Microsoft.AspNet.Mvc.TagHelpers/js/LinkTagHelper_FallbackJavaScript.js b/src/Microsoft.AspNet.Mvc.TagHelpers/js/LinkTagHelper_FallbackJavaScript.js index 45e5180233..bee98a7af2 100644 --- a/src/Microsoft.AspNet.Mvc.TagHelpers/js/LinkTagHelper_FallbackJavaScript.js +++ b/src/Microsoft.AspNet.Mvc.TagHelpers/js/LinkTagHelper_FallbackJavaScript.js @@ -26,4 +26,4 @@ doc.write(''); } } -})("[[[0]]]", "[[[1]]]", [[[2]]]); \ No newline at end of file +})(); \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperPartialExtensions.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperPartialExtensions.cs index ccdca37eb9..3871618a4b 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperPartialExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/HtmlHelperPartialExtensions.cs @@ -21,7 +21,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// The name of the partial view used to create the HTML markup. Must not be null. /// /// - /// A that on completion returns a new containing + /// A that on completion returns a new instance containing /// the created HTML. /// public static Task PartialAsync( @@ -50,7 +50,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// A to pass into the partial view. /// - /// A that on completion returns a new containing + /// A that on completion returns a new instance containing /// the created HTML. /// public static Task PartialAsync( @@ -80,7 +80,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// A model to pass into the partial view. /// - /// A that on completion returns a new containing + /// A that on completion returns a new instance containing /// the created HTML. /// public static Task PartialAsync( @@ -109,7 +109,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// The name of the partial view used to create the HTML markup. Must not be null. /// /// - /// Returns a new containing the created HTML. + /// Returns a new instance containing the created HTML. /// /// /// This method synchronously calls and blocks on @@ -141,7 +141,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// A to pass into the partial view. /// - /// Returns a new containing the created HTML. + /// Returns a new instance containing the created HTML. /// /// /// This method synchronously calls and blocks on @@ -174,7 +174,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// /// A model to pass into the partial view. /// - /// Returns a new containing the created HTML. + /// Returns a new instance containing the created HTML. /// /// /// This method synchronously calls and blocks on @@ -208,7 +208,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// A model to pass into the partial view. /// A to pass into the partial view. /// - /// Returns a new containing the created HTML. + /// Returns a new instance containing the created HTML. /// /// /// This method synchronously calls and blocks on diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs index d45054de75..a274501b04 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/IHtmlHelper.cs @@ -498,7 +498,7 @@ namespace Microsoft.AspNet.Mvc.Rendering /// A model to pass into the partial view. /// A to pass into the partial view. /// - /// A that on completion returns a new containing + /// A that on completion returns a new instance containing /// the created HTML. /// Task PartialAsync(string partialViewName, object model, ViewDataDictionary viewData); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/TagBuilder.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/TagBuilder.cs index c669da0414..29a7efda08 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/TagBuilder.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/Rendering/TagBuilder.cs @@ -10,7 +10,6 @@ using System.Text; using System.Text.Encodings.Web; using Microsoft.AspNet.Html; using Microsoft.AspNet.Mvc.ViewFeatures; -using Microsoft.Extensions.Internal; namespace Microsoft.AspNet.Mvc.Rendering { @@ -252,6 +251,15 @@ namespace Microsoft.AspNet.Mvc.Rendering /// public void WriteTo(TextWriter writer, HtmlEncoder encoder) { + var htmlTextWriter = writer as HtmlTextWriter; + if (htmlTextWriter != null) + { + // As a perf optimization, we can buffer this output rather than writing it + // out character by character. + htmlTextWriter.Write(this); + return; + } + switch (TagRenderMode) { case TagRenderMode.StartTag: diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs index f71e39c008..b0caa20804 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/HtmlHelper.cs @@ -866,18 +866,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures if (tagBuilder != null) { tagBuilder.TagRenderMode = TagRenderMode.StartTag; - - // As a perf optimization, we can buffer this output rather than writing it - // out character by character. - var htmlWriter = ViewContext.Writer as HtmlTextWriter; - if (htmlWriter == null) - { - tagBuilder.WriteTo(ViewContext.Writer, _htmlEncoder); - } - else - { - htmlWriter.Write(tagBuilder); - } + tagBuilder.WriteTo(ViewContext.Writer, _htmlEncoder); } return CreateForm(); @@ -920,18 +909,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures if (tagBuilder != null) { tagBuilder.TagRenderMode = TagRenderMode.StartTag; - - // As a perf optimization, we can buffer this output rather than writing it - // out character by character. - var htmlWriter = ViewContext.Writer as HtmlTextWriter; - if (htmlWriter == null) - { - tagBuilder.WriteTo(ViewContext.Writer, _htmlEncoder); - } - else - { - htmlWriter.Write(tagBuilder); - } + tagBuilder.WriteTo(ViewContext.Writer, _htmlEncoder); } return CreateForm(); diff --git a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/StringHtmlContent.cs b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/StringHtmlContent.cs index c2066b2fa1..db1592850a 100644 --- a/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/StringHtmlContent.cs +++ b/src/Microsoft.AspNet.Mvc.ViewFeatures/ViewFeatures/StringHtmlContent.cs @@ -39,7 +39,17 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures throw new ArgumentNullException(nameof(encoder)); } - encoder.Encode(writer, _input); + var htmlTextWriter = writer as HtmlTextWriter; + if (htmlTextWriter != null) + { + // As a perf optimization, we can buffer this output rather than writing it + // out character by character. + htmlTextWriter.Write(this); + } + else + { + encoder.Encode(writer, _input); + } } private string DebuggerToString() diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs index 8e8f1c1874..cf35631d21 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorPageTest.cs @@ -961,13 +961,14 @@ namespace Microsoft.AspNet.Mvc.Razor // Assert var htmlAttribute = Assert.Single(executionContext.HTMLAttributes); Assert.Equal("someattr", htmlAttribute.Name, StringComparer.Ordinal); - Assert.IsType(htmlAttribute.Value); - Assert.Equal(expectedValue, HtmlContentUtilities.HtmlContentToString((IHtmlContent)htmlAttribute.Value)); + var htmlContent = Assert.IsAssignableFrom(htmlAttribute.Value); + Assert.Equal(expectedValue, HtmlContentUtilities.HtmlContentToString(htmlContent), StringComparer.Ordinal); Assert.False(htmlAttribute.Minimized); + var allAttribute = Assert.Single(executionContext.AllAttributes); Assert.Equal("someattr", allAttribute.Name, StringComparer.Ordinal); - Assert.IsType(allAttribute.Value); - Assert.Equal(expectedValue, allAttribute.Value.ToString(), StringComparer.Ordinal); + htmlContent = Assert.IsAssignableFrom(allAttribute.Value); + Assert.Equal(expectedValue, HtmlContentUtilities.HtmlContentToString(htmlContent), StringComparer.Ordinal); Assert.False(allAttribute.Minimized); } diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/TagHelpers/UrlResolutionTagHelperTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/TagHelpers/UrlResolutionTagHelperTest.cs index 1cb2e7e20d..eebf952e69 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/TagHelpers/UrlResolutionTagHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/TagHelpers/UrlResolutionTagHelperTest.cs @@ -6,8 +6,10 @@ using System.Collections.Generic; using System.Linq; using System.Reflection; using System.Threading.Tasks; +using Microsoft.AspNet.Html; using Microsoft.AspNet.Mvc.Rendering; using Microsoft.AspNet.Mvc.Routing; +using Microsoft.AspNet.Mvc.TestCommon; using Microsoft.AspNet.Razor.TagHelpers; using Microsoft.Extensions.WebEncoders.Testing; using Moq; @@ -22,22 +24,13 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers get { // url, expectedHref - return new TheoryData + return new TheoryData { { "~/home/index.html", "/approot/home/index.html" }, { " ~/home/index.html", "/approot/home/index.html" }, - { new HtmlString("~/home/index.html"), new HtmlString("HtmlEncode[[/approot/]]home/index.html") }, { - new HtmlString(" ~/home/index.html"), - new HtmlString("HtmlEncode[[/approot/]]home/index.html") - }, - { - "~/home/index.html ~/secondValue/index.html", - "/approot/home/index.html ~/secondValue/index.html" - }, - { - new HtmlString("~/home/index.html ~/secondValue/index.html"), - new HtmlString("HtmlEncode[[/approot/]]home/index.html ~/secondValue/index.html") + "~/home/index.html ~/secondValue/index.html", + "/approot/home/index.html ~/secondValue/index.html" }, }; } @@ -45,7 +38,7 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers [Theory] [MemberData(nameof(ResolvableUrlData))] - public void Process_ResolvesTildeSlashValues(object url, object expectedHref) + public void Process_ResolvesTildeSlashValues(string url, string expectedHref) { // Arrange var tagHelperOutput = new TagHelperOutput( @@ -77,8 +70,64 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers // Assert var attribute = Assert.Single(tagHelperOutput.Attributes); Assert.Equal("href", attribute.Name, StringComparer.Ordinal); - Assert.IsType(expectedHref.GetType(), url); - Assert.Equal(expectedHref.ToString(), attribute.Value.ToString()); + var attributeValue = Assert.IsType(attribute.Value); + Assert.Equal(expectedHref, attributeValue, StringComparer.Ordinal); + Assert.False(attribute.Minimized); + } + + public static TheoryData ResolvableUrlHtmlStringData + { + get + { + // url, expectedHref + return new TheoryData + { + { new HtmlString("~/home/index.html"), "HtmlEncode[[/approot/]]home/index.html" }, + { new HtmlString(" ~/home/index.html"), "HtmlEncode[[/approot/]]home/index.html" }, + { + new HtmlString("~/home/index.html ~/secondValue/index.html"), + "HtmlEncode[[/approot/]]home/index.html ~/secondValue/index.html" + }, + }; + } + } + + [Theory] + [MemberData(nameof(ResolvableUrlHtmlStringData))] + public void Process_ResolvesTildeSlashValues_InHtmlString(object url, string expectedHref) + { + // Arrange + var tagHelperOutput = new TagHelperOutput( + tagName: "a", + attributes: new TagHelperAttributeList + { + { "href", url } + }, + getChildContentAsync: _ => Task.FromResult(null)); + var urlHelperMock = new Mock(); + urlHelperMock + .Setup(urlHelper => urlHelper.Content(It.IsAny())) + .Returns(new Func(value => "/approot" + value.Substring(1))); + var urlHelperFactory = new Mock(); + urlHelperFactory + .Setup(f => f.GetUrlHelper(It.IsAny())) + .Returns(urlHelperMock.Object); + var tagHelper = new UrlResolutionTagHelper(urlHelperFactory.Object, new HtmlTestEncoder()); + + var context = new TagHelperContext( + allAttributes: new ReadOnlyTagHelperAttributeList( + Enumerable.Empty()), + items: new Dictionary(), + uniqueId: "test"); + + // Act + tagHelper.Process(context, tagHelperOutput); + + // Assert + var attribute = Assert.Single(tagHelperOutput.Attributes); + Assert.Equal("href", attribute.Name, StringComparer.Ordinal); + var htmlContent = Assert.IsAssignableFrom(attribute.Value); + Assert.Equal(expectedHref, HtmlContentUtilities.HtmlContentToString(htmlContent), StringComparer.Ordinal); Assert.False(attribute.Minimized); } @@ -87,25 +136,20 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers get { // url - return new TheoryData + return new TheoryData { { "/home/index.html" }, { "~ /home/index.html" }, { "/home/index.html ~/second/wontresolve.html" }, { " ~\\home\\index.html" }, { "~\\/home/index.html" }, - { new HtmlString("/home/index.html") }, - { new HtmlString("~ /home/index.html") }, - { new HtmlString("/home/index.html ~/second/wontresolve.html") }, - { new HtmlString("~\\home\\index.html") }, - { new HtmlString("~\\/home/index.html") }, }; } } [Theory] [MemberData(nameof(UnresolvableUrlData))] - public void Process_DoesNotResolveNonTildeSlashValues(object url) + public void Process_DoesNotResolveNonTildeSlashValues(string url) { // Arrange var tagHelperOutput = new TagHelperOutput( @@ -123,7 +167,7 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers urlHelperFactory .Setup(f => f.GetUrlHelper(It.IsAny())) .Returns(urlHelperMock.Object); - var tagHelper = new UrlResolutionTagHelper(urlHelperFactory.Object, htmlEncoder: null); + var tagHelper = new UrlResolutionTagHelper(urlHelperFactory.Object, new HtmlTestEncoder()); var context = new TagHelperContext( allAttributes: new ReadOnlyTagHelperAttributeList( @@ -137,7 +181,63 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers // Assert var attribute = Assert.Single(tagHelperOutput.Attributes); Assert.Equal("href", attribute.Name, StringComparer.Ordinal); - Assert.Equal(url, attribute.Value); + var attributeValue = Assert.IsType(attribute.Value); + Assert.Equal(url, attributeValue, StringComparer.Ordinal); + Assert.False(attribute.Minimized); + } + + public static TheoryData UnresolvableUrlHtmlStringData + { + get + { + // url + return new TheoryData + { + { new HtmlString("/home/index.html") }, + { new HtmlString("~ /home/index.html") }, + { new HtmlString("/home/index.html ~/second/wontresolve.html") }, + { new HtmlString("~\\home\\index.html") }, + { new HtmlString("~\\/home/index.html") }, + }; + } + } + + [Theory] + [MemberData(nameof(UnresolvableUrlHtmlStringData))] + public void Process_DoesNotResolveNonTildeSlashValues_InHtmlString(HtmlString url) + { + // Arrange + var tagHelperOutput = new TagHelperOutput( + tagName: "a", + attributes: new TagHelperAttributeList + { + { "href", url } + }, + getChildContentAsync: _ => Task.FromResult(null)); + var urlHelperMock = new Mock(); + urlHelperMock + .Setup(urlHelper => urlHelper.Content(It.IsAny())) + .Returns("approot/home/index.html"); + var urlHelperFactory = new Mock(); + urlHelperFactory + .Setup(f => f.GetUrlHelper(It.IsAny())) + .Returns(urlHelperMock.Object); + var tagHelper = new UrlResolutionTagHelper(urlHelperFactory.Object, new HtmlTestEncoder()); + + var context = new TagHelperContext( + allAttributes: new ReadOnlyTagHelperAttributeList( + Enumerable.Empty()), + items: new Dictionary(), + uniqueId: "test"); + + // Act + tagHelper.Process(context, tagHelperOutput); + + // Assert + var attribute = Assert.Single(tagHelperOutput.Attributes); + Assert.Equal("href", attribute.Name, StringComparer.Ordinal); + var attributeValue = Assert.IsType(attribute.Value); + Assert.Equal(url.ToString(), attributeValue.ToString(), StringComparer.Ordinal); Assert.False(attribute.Minimized); } @@ -197,7 +297,7 @@ namespace Microsoft.AspNet.Mvc.Razor.TagHelpers urlHelperFactory .Setup(f => f.GetUrlHelper(It.IsAny())) .Returns(urlHelperMock.Object); - var tagHelper = new UrlResolutionTagHelper(urlHelperFactory.Object, htmlEncoder: null); + var tagHelper = new UrlResolutionTagHelper(urlHelperFactory.Object, new HtmlTestEncoder()); var context = new TagHelperContext( allAttributes: new ReadOnlyTagHelperAttributeList( diff --git a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/Internal/JavaScriptResourcesTest.cs b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/Internal/JavaScriptResourcesTest.cs index 3846ad3a79..b14ad29636 100644 --- a/test/Microsoft.AspNet.Mvc.TagHelpers.Test/Internal/JavaScriptResourcesTest.cs +++ b/test/Microsoft.AspNet.Mvc.TagHelpers.Test/Internal/JavaScriptResourcesTest.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal { // Arrange var resource = "window.alert('An alert');"; + var expected = resource.Substring(0, resource.Length - 2); var stream = new MemoryStream(Encoding.UTF8.GetBytes(resource)); var getManifestResourceStream = new Func(name => stream); var cache = new ConcurrentDictionary(); @@ -24,7 +25,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal var result = JavaScriptResources.GetEmbeddedJavaScript("test.js", getManifestResourceStream, cache); // Assert - Assert.Equal(resource, result); + Assert.Equal(expected, result); } [Fact] @@ -32,6 +33,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal { // Arrange var resource = "window.alert('An alert');"; + var expected = resource.Substring(0, resource.Length - 2); var stream = new MemoryStream(Encoding.UTF8.GetBytes(resource)); var getManifestResourceStream = new Func(name => stream); var cache = new ConcurrentDictionary(); @@ -43,7 +45,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal Assert.Collection(cache, kvp => { Assert.Equal("test.js", kvp.Key); - Assert.Equal(resource, kvp.Value); + Assert.Equal(expected, kvp.Value); }); } @@ -70,12 +72,13 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal } [Theory] - [InlineData("window.alert(\"[[[0]]]\")", "window.alert(\"{0}\")")] - [InlineData("var test = { a: 1 };", "var test = {{ a: 1 }};")] - [InlineData("var test = { a: 1, b: \"[[[0]]]\" };", "var test = {{ a: 1, b: \"{0}\" }};")] - public void GetEmbeddedJavaScript_PreparesJavaScriptCorrectly(string resource, string expectedResult) + [InlineData("window.alert(\"[[[0]]]\")")] + [InlineData("var test = { a: 1 };")] + [InlineData("var test = { a: 1, b: \"[[[0]]]\" };")] + public void GetEmbeddedJavaScript_PreparesJavaScriptCorrectly(string resource) { // Arrange + var expected = resource.Substring(0, resource.Length - 2); var stream = new MemoryStream(Encoding.UTF8.GetBytes(resource)); var getManifestResourceStream = new Func(name => stream); var cache = new ConcurrentDictionary(); @@ -84,7 +87,7 @@ namespace Microsoft.AspNet.Mvc.TagHelpers.Internal var result = JavaScriptResources.GetEmbeddedJavaScript("test.js", getManifestResourceStream, cache); // Assert - Assert.Equal(expectedResult, result); + Assert.Equal(expected, result); } } } \ No newline at end of file