From 3147c1158d64268b84c417201bcde0e1ccc8486c Mon Sep 17 00:00:00 2001 From: Pranav K <prkrishn@hotmail.com> Date: Thu, 11 Nov 2021 09:29:51 -0800 Subject: [PATCH] Avoid null ref in TagHelperMatchingConvention when attribute name is null. (#38264) (#38271) As part of .NET 6, we made some tweaks to the razor parser to avoid string allocations. This change results in a null-ref for bound properties without public setters. The parser change affects building apps going back all the way to .NET 2.1, so we'd like to fix this. Fixes https://github.com/dotnet/aspnetcore/issues/38194 --- .../src/StringSegment.cs | 16 +------ .../test/StringSegmentTest.cs | 42 +++++++++++++++++++ .../test/TagHelperMatchingConventionsTest.cs | 37 +++++++++++++++- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/StringSegment.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/StringSegment.cs index b97c498f095..e680b456620 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/StringSegment.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/StringSegment.cs @@ -110,7 +110,6 @@ namespace Microsoft.AspNetCore.Razor return Equals(other, StringComparison.Ordinal); } - /// <summary> /// Indicates whether the current object is equal to another object of the same type. /// </summary> @@ -160,15 +159,7 @@ namespace Microsoft.AspNetCore.Razor /// <param name="comparisonType">One of the enumeration values that specifies the rules to use in the comparison.</param> /// <returns><code>true</code> if the specified <see cref="string"/> is equal to the current <see cref="StringSegment"/>; otherwise, <code>false</code>.</returns> public bool Equals(string text, StringComparison comparisonType) - { - var textLength = text.Length; - if (!HasValue || Length != textLength) - { - return false; - } - - return string.Compare(Buffer, Offset, text, 0, textLength, comparisonType) == 0; - } + => Equals(new StringSegment(text), comparisonType); /// <inheritdoc /> public override int GetHashCode() @@ -225,10 +216,7 @@ namespace Microsoft.AspNetCore.Razor /// <param name="comparisonType">One of the enumeration values that specifies the rules to use in the comparison.</param> /// <returns><code>true</code> if <paramref name="text"/> matches the beginning of this <see cref="StringSegment"/>; otherwise, <code>false</code>.</returns> public bool StartsWith(string text, StringComparison comparisonType) - { - var textLength = text.Length; - return string.Compare(Buffer, Offset, text, 0, textLength, comparisonType) == 0; - } + => StartsWith(new StringSegment(text), comparisonType); public bool StartsWith(StringSegment text, StringComparison comparisonType) { diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/StringSegmentTest.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/StringSegmentTest.cs index 4b202656e09..4d203c30d9a 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/StringSegmentTest.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/StringSegmentTest.cs @@ -248,6 +248,48 @@ namespace Microsoft.AspNetCore.Razor.Language Assert.Equal(expectedResult, result); } + [Fact] + public void StringSegment_Equals_NullString() + { + // Arrange + var stringSegment = new StringSegment("text"); + var @string = (string)null; + + // Act + var result = stringSegment.Equals(@string, StringComparison.Ordinal); + + // Assert + Assert.False(result); + } + + [Fact] + public void StringSegment_DefaultValue_Equals_NullString() + { + // Arrange + var stringSegment = StringSegment.Empty; + var @string = (string)null; + + // Act + var result = stringSegment.Equals(@string, StringComparison.Ordinal); + + // Assert + Assert.False(result); + } + + [Fact] + public void StringSegment_DefaultValue_Equals_EmptyString() + { + // Arrange + var stringSegment = StringSegment.Empty; + var @string = string.Empty; + + // Act + var result = stringSegment.Equals(@string, StringComparison.Ordinal); + + // Assert + Assert.True(result); + } + [Fact] public void StringSegment_StaticEquals_Valid() { diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/TagHelperMatchingConventionsTest.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/TagHelperMatchingConventionsTest.cs index c32b87a58ff..c3602b8f629 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/test/TagHelperMatchingConventionsTest.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/test/TagHelperMatchingConventionsTest.cs @@ -1,7 +1,8 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Collections.Generic; using Xunit; namespace Microsoft.AspNetCore.Razor.Language @@ -154,5 +155,39 @@ namespace Microsoft.AspNetCore.Razor.Language // Assert Assert.Equal(expectedResult, result); } + + [Fact] + public void CanSatisfyBoundAttribute_IndexerAttribute_ReturnsFalseIsNotMatching() + { + // Arrange + var tagHelperBuilder = new DefaultTagHelperDescriptorBuilder(TagHelperConventions.DefaultKind, "TestTagHelper", "Test"); + var builder = new DefaultBoundAttributeDescriptorBuilder(tagHelperBuilder, TagHelperConventions.DefaultKind); + builder.AsDictionary("asp-", typeof(Dictionary<string, string>).FullName); + + var boundAttribute = builder.Build(); + + // Act + var result = TagHelperMatchingConventions.CanSatisfyBoundAttribute("style", boundAttribute); + + // Assert + Assert.False(result); + } + + [Fact] + public void CanSatisfyBoundAttribute_IndexerAttribute_ReturnsTrueIfMatching() + { + // Arrange + var tagHelperBuilder = new DefaultTagHelperDescriptorBuilder(TagHelperConventions.DefaultKind, "TestTagHelper", "Test"); + var builder = new DefaultBoundAttributeDescriptorBuilder(tagHelperBuilder, TagHelperConventions.DefaultKind); + builder.AsDictionary("asp-", typeof(Dictionary<string, string>).FullName); + + var boundAttribute = builder.Build(); + + // Act + var result = TagHelperMatchingConventions.CanSatisfyBoundAttribute("asp-route-controller", boundAttribute); + + // Assert + Assert.True(result); + } } } -- GitLab