Use SearchValues throughout dotnet/aspnetcore (part 3) (#54878)

* TagBuilder using SearchValues

- Adding further tests and a new benchmark to compare before-after performance
- Updating the implementation of TagBuilder to use SearchValues instead manual operation

* Update comment triggering a build

* Using SearchValues in UrlResolutionTagHelper.

* Review feedback + fixing outofrange exception for unresolveable urls

* Using StartsWith instead of SequenceEqual

---------

Co-authored-by: ladeak <[email protected]>

Author: ladeak

src/Mvc/Mvc.Razor/src/TagHelpers/UrlResolutionTagHelper.cs

@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Buffers;
 using System.Diagnostics.CodeAnalysis;
 using System.Text.Encodings.Web;
 using Microsoft.AspNetCore.Html;
@@ -49,8 +50,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.TagHelpers;
 public class UrlResolutionTagHelper : TagHelper
 {
     // Valid whitespace characters defined by the HTML5 spec.
-    private static readonly char[] ValidAttributeWhitespaceChars =
-        new[] { '\t', '\n', '\u000C', '\r', ' ' };
+    private static readonly SearchValues<char> ValidAttributeWhitespaceChars = SearchValues.Create("\t\n\u000C\r ");
+
     private static readonly Dictionary<string, string[]> ElementAttributeLookups =
         new(StringComparer.OrdinalIgnoreCase)
         {
@@ -215,14 +216,11 @@ protected void ProcessUrlAttribute(string attributeName, TagHelperOutput output)
     protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Relative)] string url, out string? resolvedUrl)
     {
         resolvedUrl = null;
-        var start = FindRelativeStart(url);
-        if (start == -1)
+        if (!TryCreateTrimmedString(url, out var trimmedUrl))
         {
             return false;
         }
 
-        var trimmedUrl = CreateTrimmedString(url, start);
-
         var urlHelper = UrlHelperFactory.GetUrlHelper(ViewContext);
         resolvedUrl = urlHelper.Content(trimmedUrl);
 
@@ -241,14 +239,11 @@ protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Re
     protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Relative)] string url, [NotNullWhen(true)] out IHtmlContent? resolvedUrl)
     {
         resolvedUrl = null;
-        var start = FindRelativeStart(url);
-        if (start == -1)
+        if (!TryCreateTrimmedString(url, out var trimmedUrl))
         {
             return false;
         }
 
-        var trimmedUrl = CreateTrimmedString(url, start);
-
         var urlHelper = UrlHelperFactory.GetUrlHelper(ViewContext);
         var appRelativeUrl = urlHelper.Content(trimmedUrl);
         var postTildeSlashUrlValue = trimmedUrl.Substring(2);
@@ -273,58 +268,35 @@ protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Re
         return true;
     }
 
-    private static int FindRelativeStart(string url)
+    private static bool TryCreateTrimmedString(string input, [NotNullWhen(true)] out string? trimmed)
     {
-        if (url == null || url.Length < 2)
+        trimmed = null;
+        if (input == null)
         {
-            return -1;
+            return false;
         }
 
-        var maxTestLength = url.Length - 2;
-
-        var start = 0;
-        for (; start < url.Length; start++)
+        var url = input.AsSpan();
+        var start = url.IndexOfAnyExcept(ValidAttributeWhitespaceChars);
+        if (start < 0)
         {
-            if (start > maxTestLength)
-            {
-                return -1;
-            }
-
-            if (!IsCharWhitespace(url[start]))
-            {
-                break;
-            }
-        }
-
-        // Before doing more work, ensure that the URL we're looking at is app-relative.
-        if (url[start] != '~' || url[start + 1] != '/')
-        {
-            return -1;
+            return false;
         }
 
-        return start;
-    }
+        // Url without leading whitespace.
+        url = url.Slice(start);
 
-    private static string CreateTrimmedString(string input, int start)
-    {
-        var end = input.Length - 1;
-        for (; end >= start; end--)
+        // Before doing more work, ensure that the URL we're looking at is app-relative.
+        if (!url.StartsWith("~/"))
         {
-            if (!IsCharWhitespace(input[end]))
-            {
-                break;
-            }
+            return false;
         }
 
-        var len = end - start + 1;
+        var remainingLength = url.LastIndexOfAnyExcept(ValidAttributeWhitespaceChars) + 1;
 
         // Substring returns same string if start == 0 && len == Length
-        return input.Substring(start, len);
-    }
-
-    private static bool IsCharWhitespace(char ch)
-    {
-        return ValidAttributeWhitespaceChars.AsSpan().IndexOf(ch) != -1;
+        trimmed = input.Substring(start, remainingLength);
+        return true;
     }
 
     private sealed class EncodeFirstSegmentContent : IHtmlContent

src/Mvc/Mvc.Razor/test/TagHelpers/UrlResolutionTagHelperTest.cs

@@ -19,11 +19,19 @@ public static TheoryData ResolvableUrlData
             return new TheoryData<string, string>
                 {
                    { "~/home/index.html", "/approot/home/index.html" },
+                   { "~/home/index.html\r\n", "/approot/home/index.html" },
                    { "  ~/home/index.html", "/approot/home/index.html" },
+                   { "\u000C~/home/index.html\r\n", "/approot/home/index.html" },
+                   { "\t ~/home/index.html\n", "/approot/home/index.html" },
+                   { "\r\n~/home/index.html\u000C\t", "/approot/home/index.html" },
+                   { "\r~/home/index.html\t", "/approot/home/index.html" },
+                   { "\n~/home/index.html\u202F", "/approot/home/index.html\u202F" },
                    {
                         "~/home/index.html ~/secondValue/index.html",
                         "/approot/home/index.html ~/secondValue/index.html"
                    },
+                   { "  ~/   ", "/approot/" },
+                   { "  ~/", "/approot/" },
                 };
         }
     }
@@ -166,6 +174,8 @@ public static TheoryData UnresolvableUrlData
                    { "/home/index.html ~/second/wontresolve.html" },
                    { "  ~\\home\\index.html" },
                    { "~\\/home/index.html" },
+                   { "  ~" },
+                   { "   " },
                 };
         }
     }

src/Mvc/Mvc.ViewFeatures/src/Rendering/TagBuilder.cs

@@ -3,6 +3,7 @@
 
 #nullable enable
 
+using System.Buffers;
 using System.Diagnostics;
 using System.Globalization;
 using System.Text;
@@ -19,6 +20,11 @@ namespace Microsoft.AspNetCore.Mvc.Rendering;
 [DebuggerDisplay("{DebuggerToString()}")]
 public class TagBuilder : IHtmlContent
 {
+    // Note '.' is valid according to the HTML 4.01 specification. Disallowed here
+    // to avoid confusion with CSS class selectors or when using jQuery.
+    private static readonly SearchValues<char> _html401IdChars =
+        SearchValues.Create("-0123456789:ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz");
+
     private AttributeDictionary? _attributes;
     private HtmlContentBuilder? _innerHtml;
 
@@ -154,51 +160,39 @@ public static string CreateSanitizedId(string? name, string invalidCharReplaceme
         }
 
         // If there are no invalid characters in the string, then we don't have to create the buffer.
-        var firstIndexOfInvalidCharacter = 1;
-        for (; firstIndexOfInvalidCharacter < name.Length; firstIndexOfInvalidCharacter++)
+        var indexOfInvalidCharacter = name.AsSpan(1).IndexOfAnyExcept(_html401IdChars);
+        var firstChar = name[0];
+        var startsWithAsciiLetter = char.IsAsciiLetter(firstChar);
+        if (startsWithAsciiLetter && indexOfInvalidCharacter < 0)
         {
-            if (!Html401IdUtil.IsValidIdCharacter(name[firstIndexOfInvalidCharacter]))
-            {
-                break;
-            }
+            return name;
         }
 
-        var firstChar = name[0];
-        var startsWithAsciiLetter = char.IsAsciiLetter(firstChar);
         if (!startsWithAsciiLetter)
         {
             // The first character must be a letter according to the HTML 4.01 specification.
             firstChar = 'z';
         }
 
-        if (firstIndexOfInvalidCharacter == name.Length && startsWithAsciiLetter)
-        {
-            return name;
-        }
-
         var stringBuffer = new StringBuilder(name.Length);
         stringBuffer.Append(firstChar);
+        var remainingName = name.AsSpan(1);
 
-        // Characters until 'firstIndexOfInvalidCharacter' have already been checked for validity.
-        // So just copy them. This avoids running them through Html401IdUtil.IsValidIdCharacter again.
-        for (var index = 1; index < firstIndexOfInvalidCharacter; index++)
-        {
-            stringBuffer.Append(name[index]);
-        }
-
-        for (var index = firstIndexOfInvalidCharacter; index < name.Length; index++)
+        // Copy values until an invalid character found. Replace the invalid character with the replacement string
+        // and search for the next invalid character.
+        while (remainingName.Length > 0)
         {
-            var thisChar = name[index];
-            if (Html401IdUtil.IsValidIdCharacter(thisChar))
+            if (indexOfInvalidCharacter < 0)
             {
-                stringBuffer.Append(thisChar);
-            }
-            else
-            {
-                stringBuffer.Append(invalidCharReplacement);
+                stringBuffer.Append(remainingName);
+                break;
             }
-        }
 
+            stringBuffer.Append(remainingName.Slice(0, indexOfInvalidCharacter));
+            stringBuffer.Append(invalidCharReplacement);
+            remainingName = remainingName.Slice(indexOfInvalidCharacter + 1);
+            indexOfInvalidCharacter = remainingName.IndexOfAnyExcept(_html401IdChars);
+        }
         return stringBuffer.ToString();
     }
 
@@ -418,28 +412,4 @@ public void WriteTo(TextWriter writer, HtmlEncoder encoder)
             TagBuilder.WriteTo(_tagBuilder, writer, encoder, _tagRenderMode);
         }
     }
-
-    private static class Html401IdUtil
-    {
-        public static bool IsValidIdCharacter(char testChar)
-        {
-            return char.IsAsciiLetterOrDigit(testChar) || IsAllowableSpecialCharacter(testChar);
-        }
-
-        private static bool IsAllowableSpecialCharacter(char testChar)
-        {
-            switch (testChar)
-            {
-                case '-':
-                case '_':
-                case ':':
-                    // Note '.' is valid according to the HTML 4.01 specification. Disallowed here to avoid
-                    // confusion with CSS class selectors or when using jQuery.
-                    return true;
-
-                default:
-                    return false;
-            }
-        }
-    }
 }

src/Mvc/Mvc.ViewFeatures/test/Rendering/TagBuilderTest.cs

@@ -95,6 +95,16 @@ public void WriteTo_IgnoresIdAttributeCase(TagRenderMode renderingMode, string e
     [InlineData("0", "z")]
     [InlineData("-", "z")]
     [InlineData(",", "z")]
+    [InlineData(",.", "z-")]
+    [InlineData("a\uD83D\uDE0A", "a--")]
+    [InlineData("a\uD83D\uDE0A.", "a---")]
+    [InlineData("\uD83D\uDE0A", "z-")]
+    [InlineData("\uD83D\uDE0A.", "z--")]
+    [InlineData(",a", "za")]
+    [InlineData("a,", "a-")]
+    [InlineData("a0,", "a0-")]
+    [InlineData("a,0,", "a-0-")]
+    [InlineData("a,0", "a-0")]
     [InlineData("00Hello,World", "z0Hello-World")]
     [InlineData(",,Hello,,World,,", "z-Hello--World--")]
     [InlineData("-_:Hello-_:Hello-_:", "z_:Hello-_:Hello-_:")]
@@ -110,6 +120,23 @@ public void CreateSanitizedIdCreatesId(string input, string output)
         Assert.Equal(output, result);
     }
 
+    [Fact]
+    public void CreateSanitizedIdCreatesIdReplacesAllInvalidCharacters()
+    {
+        foreach (char c in Enumerable.Range(char.MinValue, char.MaxValue))
+        {
+            var result = TagBuilder.CreateSanitizedId($"a{c}", "z");
+            if (char.IsAsciiLetterOrDigit(c) || c == '-' || c == '_' || c == ':')
+            {
+                Assert.Equal($"a{c}", result);
+            }
+            else
+            {
+                Assert.Equal("az", result);
+            }
+        }
+    }
+
     [Theory]
     [InlineData("attribute", "value", "<p attribute=\"HtmlEncode[[value]]\"></p>")]
     [InlineData("attribute", null, "<p attribute=\"\"></p>")]

src/Mvc/perf/Microbenchmarks/Microsoft.AspNetCore.Mvc/TagBuilderBenchmark.cs

@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using BenchmarkDotNet.Attributes;
+using Microsoft.AspNetCore.Mvc.Rendering;
+
+namespace Microsoft.AspNetCore.Mvc.Microbenchmarks;
+
+public class TagBuilderBenchmark
+{
+    [Benchmark]
+    public string ValidFieldName() => TagBuilder.CreateSanitizedId("LongIdForFieldWithMaxLength", "z");
+
+    [Benchmark]
+    public string InvalidFieldName() => TagBuilder.CreateSanitizedId("LongIdForField$WithMaxLength", "z");
+}