From f222fa4349ff0aeff80ac1dbde6cac6429d990cf Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Sun, 25 Sep 2016 00:03:45 -0700 Subject: [PATCH] Correct examination of `IHtmlHelper.Label()` return value - #5317 - previously worked only because `TagBuilder` cannot be empty and `HtmlString` overrides `ToString()` - `TagBuilder.ToString()` (now the type's `FullName`) is also never empty - copy `NullHtmlEncoder` from Razor and give it a better name (`PassThroughHtmlEncoder`) - not available in this project and (from its namespace) not intended for general use --- .../Internal/DefaultEditorTemplates.cs | 123 +++++++++++- .../project.json | 1 + .../Internal/DefaultEditorTemplatesTest.cs | 187 ++++++++++++++++-- 3 files changed, 294 insertions(+), 17 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs index 5b7181d013..a1d3b66e9f 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/DefaultEditorTemplates.cs @@ -6,6 +6,9 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; +using System.IO; +using System.Text; +using System.Text.Encodings.Web; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.Rendering; @@ -272,12 +275,16 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal if (!propertyMetadata.HideSurroundingHtml) { var label = htmlHelper.Label(propertyMetadata.PropertyName, labelText: null, htmlAttributes: null); - if (!string.IsNullOrEmpty(label.ToString())) + using (var writer = new HasContentTextWriter()) { - var labelTag = new TagBuilder("div"); - labelTag.AddCssClass("editor-label"); - labelTag.InnerHtml.SetHtmlContent(label); - content.AppendLine(labelTag); + label.WriteTo(writer, PassThroughHtmlEncoder.Default); + if (writer.HasContent) + { + var labelTag = new TagBuilder("div"); + labelTag.AddCssClass("editor-label"); + labelTag.InnerHtml.SetHtmlContent(label); + content.AppendLine(labelTag); + } } var valueDivTag = new TagBuilder("div"); @@ -433,5 +440,111 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal format: null, htmlAttributes: htmlAttributes); } + + private class HasContentTextWriter : TextWriter + { + public HasContentTextWriter() + { + } + + public bool HasContent { get; private set; } + + public override Encoding Encoding => Null.Encoding; + + public override void Write(char value) + { + HasContent = true; + } + + public override void Write(char[] buffer, int index, int count) + { + if (count > 0) + { + HasContent = true; + } + } + + public override void Write(string value) + { + if (!string.IsNullOrEmpty(value)) + { + HasContent = true; + } + } + } + + // An HTML encoder which passes through all input data. Does no encoding. + // Copied from Microsoft.AspNetCore.Razor.TagHelpers.NullHtmlEncoder. + private class PassThroughHtmlEncoder : HtmlEncoder + { + private PassThroughHtmlEncoder() + { + } + + public static new PassThroughHtmlEncoder Default { get; } = new PassThroughHtmlEncoder(); + + public override int MaxOutputCharactersPerInputCharacter => 1; + + public override string Encode(string value) + { + return value; + } + + public override void Encode(TextWriter output, char[] value, int startIndex, int characterCount) + { + if (output == null) + { + throw new ArgumentNullException(nameof(output)); + } + + if (characterCount == 0) + { + return; + } + + output.Write(value, startIndex, characterCount); + } + + public override void Encode(TextWriter output, string value, int startIndex, int characterCount) + { + if (output == null) + { + throw new ArgumentNullException(nameof(output)); + } + + if (value == null) + { + throw new ArgumentNullException(nameof(value)); + } + + if (characterCount == 0) + { + return; + } + + output.Write(value.Substring(startIndex, characterCount)); + } + + public override unsafe int FindFirstCharacterToEncode(char* text, int textLength) + { + return -1; + } + + public override unsafe bool TryEncodeUnicodeScalar( + int unicodeScalar, + char* buffer, + int bufferLength, + out int numberOfCharactersWritten) + { + numberOfCharactersWritten = 0; + + return false; + } + + public override bool WillEncode(int unicodeScalar) + { + return false; + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/project.json b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/project.json index df5f1d6182..3163542ae0 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/project.json +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/project.json @@ -12,6 +12,7 @@ ] }, "buildOptions": { + "allowUnsafe": true, "warningsAsErrors": true, "keyFile": "../../tools/Key.snk", "nowarn": [ diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs index 10d6eb53d2..fdad417bde 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Internal/DefaultEditorTemplatesTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Globalization; +using System.IO; using System.Linq; using System.Text; using System.Text.Encodings.Web; @@ -86,22 +87,108 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal } } + // label's IHtmlContent -> expected label text + public static TheoryData ObjectTemplate_ChecksWriteTo_NotToStringData + { + get + { + // Similar to HtmlString.Empty today. + var noopContentWithEmptyToString = new Mock(MockBehavior.Strict); + noopContentWithEmptyToString + .Setup(c => c.ToString()) + .Returns(string.Empty); + noopContentWithEmptyToString.Setup(c => c.WriteTo(It.IsAny(), It.IsAny())); + + // Similar to an empty StringHtmlContent today. + var noopContentWithNonEmptyToString = new Mock(MockBehavior.Strict); + noopContentWithNonEmptyToString + .Setup(c => c.ToString()) + .Returns(typeof(StringHtmlContent).FullName); + noopContentWithNonEmptyToString.Setup(c => c.WriteTo(It.IsAny(), It.IsAny())); + + // Makes noop calls on the TextWriter. + var busyNoopContentWithNonEmptyToString = new Mock(MockBehavior.Strict); + busyNoopContentWithNonEmptyToString + .Setup(c => c.ToString()) + .Returns(typeof(StringHtmlContent).FullName); + busyNoopContentWithNonEmptyToString + .Setup(c => c.WriteTo(It.IsAny(), It.IsAny())) + .Callback((writer, encoder) => + { + writer.Write(string.Empty); + writer.Write(new char[0]); + writer.Write((char[])null); + writer.Write((object)null); + writer.Write((string)null); + writer.Write(format: "{0}", arg0: null); + writer.Write(new char[] { 'a', 'b', 'c' }, index: 1, count: 0); + }); + + // Unrealistic but covers all the bases. + var writingContentWithEmptyToString = new Mock(MockBehavior.Strict); + writingContentWithEmptyToString + .Setup(c => c.ToString()) + .Returns(string.Empty); + writingContentWithEmptyToString + .Setup(c => c.WriteTo(It.IsAny(), It.IsAny())) + .Callback((writer, encoder) => writer.Write("Some string")); + + // Similar to TagBuilder today. + var writingContentWithNonEmptyToString = new Mock(MockBehavior.Strict); + writingContentWithNonEmptyToString + .Setup(c => c.ToString()) + .Returns(typeof(TagBuilder).FullName); + writingContentWithNonEmptyToString + .Setup(c => c.WriteTo(It.IsAny(), It.IsAny())) + .Callback((writer, encoder) => writer.Write("Some string")); + + // label's IHtmlContent -> expected label text + return new TheoryData + { + // Types HtmlHelper actually uses. + { HtmlString.Empty, string.Empty }, + { + new TagBuilder("label"), + "
" + Environment.NewLine + }, + + // Another IHtmlContent implementation that does not override ToString(). + { new StringHtmlContent(string.Empty), string.Empty }, + + // Mocks + { noopContentWithEmptyToString.Object, string.Empty }, + { noopContentWithNonEmptyToString.Object, string.Empty }, + { busyNoopContentWithNonEmptyToString.Object, string.Empty }, + { + writingContentWithEmptyToString.Object, + "
Some string
" + Environment.NewLine + }, + { + writingContentWithNonEmptyToString.Object, + "
Some string
" + Environment.NewLine + }, + }; + } + } + [Fact] public void ObjectTemplateEditsSimplePropertiesOnObjectByDefault() { - var expected = - "
" + Environment.NewLine - + "
Model = p1, ModelType = System.String, PropertyName = Property1," + - " SimpleDisplayText = p1 " + - "" + - "
" + Environment.NewLine - + "
" + Environment.NewLine - + "
Model = (null), ModelType = System.String, PropertyName = Property2," + - " SimpleDisplayText = (null) " + - "" + - "
" + Environment.NewLine; - // Arrange + var expected = + "
" + + Environment.NewLine + + "
Model = p1, ModelType = System.String, PropertyName = Property1, SimpleDisplayText = p1 " + + "" + + "
" + + Environment.NewLine + + "
" + + Environment.NewLine + + "
Model = (null), ModelType = System.String, PropertyName = Property2, SimpleDisplayText = (null) " + + "" + + "
" + + Environment.NewLine; + var model = new DefaultTemplatesUtilities.ObjectTemplateModel { Property1 = "p1", Property2 = null }; var html = DefaultTemplatesUtilities.GetHtmlHelper(model); @@ -112,6 +199,82 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal Assert.Equal(expected, HtmlContentUtilities.HtmlContentToString(result)); } + // Expect almost the same HTML as in ObjectTemplateEditsSimplePropertiesOnObjectByDefault(). Only difference is + // the
...
is not present for Property1. + [Fact] + public void ObjectTemplateSkipsLabel_IfDisplayNameIsEmpty() + { + // Arrange + var expected = + "
Model = p1, ModelType = System.String, PropertyName = Property1, SimpleDisplayText = p1 " + + "" + + "
" + + Environment.NewLine + + "
" + + Environment.NewLine + + "
Model = (null), ModelType = System.String, PropertyName = Property2, SimpleDisplayText = (null) " + + "" + + "
" + + Environment.NewLine; + + var provider = new TestModelMetadataProvider(); + provider + .ForProperty( + nameof(DefaultTemplatesUtilities.ObjectTemplateModel.Property1)) + .DisplayDetails(dd => dd.DisplayName = () => string.Empty); + + var model = new DefaultTemplatesUtilities.ObjectTemplateModel { Property1 = "p1", Property2 = null }; + var html = DefaultTemplatesUtilities.GetHtmlHelper(model, provider); + + // Act + var result = DefaultEditorTemplates.ObjectTemplate(html); + + // Assert + Assert.Equal(expected, HtmlContentUtilities.HtmlContentToString(result)); + } + + [Theory] + [MemberData(nameof(ObjectTemplate_ChecksWriteTo_NotToStringData))] + public void ObjectTemplate_ChecksWriteTo_NotToString(IHtmlContent labelContent, string expectedLabel) + { + // Arrange + var expected = + expectedLabel + + "
Model = (null), ModelType = System.String, PropertyName = Property1, SimpleDisplayText = (null) " + + "
" + + Environment.NewLine + + expectedLabel + + "
Model = (null), ModelType = System.String, PropertyName = Property2, SimpleDisplayText = (null) " + + "
" + + Environment.NewLine; + + var helperToCopy = DefaultTemplatesUtilities.GetHtmlHelper(); + var helperMock = new Mock(MockBehavior.Strict); + helperMock.SetupGet(h => h.ViewContext).Returns(helperToCopy.ViewContext); + helperMock.SetupGet(h => h.ViewData).Returns(helperToCopy.ViewData); + helperMock + .Setup(h => h.Label( + It.Is(s => string.Equals("Property1", s, StringComparison.Ordinal) || + string.Equals("Property2", s, StringComparison.Ordinal)), + null, // labelText + null)) // htmlAttributes + .Returns(labelContent); + helperMock + .Setup(h => h.ValidationMessage( + It.Is(s => string.Equals("Property1", s, StringComparison.Ordinal) || + string.Equals("Property2", s, StringComparison.Ordinal)), + null, // message + null, // htmlAttributes + null)) // tag + .Returns(HtmlString.Empty); + + // Act + var result = DefaultEditorTemplates.ObjectTemplate(helperMock.Object); + + // Assert + Assert.Equal(expected, HtmlContentUtilities.HtmlContentToString(result)); + } + [Fact] public void ObjectTemplateDisplaysNullDisplayTextWithNullModelAndTemplateDepthGreaterThanOne() {