Skip to content

Commit 26869f2

Browse files
ladeakladeak
ladeak
and
ladeak
authored
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]>
1 parent 4b84751 commit 26869f2

File tree

5 files changed

+97
-102
lines changed

5 files changed

+97
-102
lines changed

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

Lines changed: 21 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System.Buffers;
45
using System.Diagnostics.CodeAnalysis;
56
using System.Text.Encodings.Web;
67
using Microsoft.AspNetCore.Html;
@@ -49,8 +50,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.TagHelpers;
4950
public class UrlResolutionTagHelper : TagHelper
5051
{
5152
// Valid whitespace characters defined by the HTML5 spec.
52-
private static readonly char[] ValidAttributeWhitespaceChars =
53-
new[] { '\t', '\n', '\u000C', '\r', ' ' };
53+
private static readonly SearchValues<char> ValidAttributeWhitespaceChars = SearchValues.Create("\t\n\u000C\r ");
54+
5455
private static readonly Dictionary<string, string[]> ElementAttributeLookups =
5556
new(StringComparer.OrdinalIgnoreCase)
5657
{
@@ -215,14 +216,11 @@ protected void ProcessUrlAttribute(string attributeName, TagHelperOutput output)
215216
protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Relative)] string url, out string? resolvedUrl)
216217
{
217218
resolvedUrl = null;
218-
var start = FindRelativeStart(url);
219-
if (start == -1)
219+
if (!TryCreateTrimmedString(url, out var trimmedUrl))
220220
{
221221
return false;
222222
}
223223

224-
var trimmedUrl = CreateTrimmedString(url, start);
225-
226224
var urlHelper = UrlHelperFactory.GetUrlHelper(ViewContext);
227225
resolvedUrl = urlHelper.Content(trimmedUrl);
228226

@@ -241,14 +239,11 @@ protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Re
241239
protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Relative)] string url, [NotNullWhen(true)] out IHtmlContent? resolvedUrl)
242240
{
243241
resolvedUrl = null;
244-
var start = FindRelativeStart(url);
245-
if (start == -1)
242+
if (!TryCreateTrimmedString(url, out var trimmedUrl))
246243
{
247244
return false;
248245
}
249246

250-
var trimmedUrl = CreateTrimmedString(url, start);
251-
252247
var urlHelper = UrlHelperFactory.GetUrlHelper(ViewContext);
253248
var appRelativeUrl = urlHelper.Content(trimmedUrl);
254249
var postTildeSlashUrlValue = trimmedUrl.Substring(2);
@@ -273,58 +268,35 @@ protected bool TryResolveUrl([StringSyntax(StringSyntaxAttribute.Uri, UriKind.Re
273268
return true;
274269
}
275270

276-
private static int FindRelativeStart(string url)
271+
private static bool TryCreateTrimmedString(string input, [NotNullWhen(true)] out string? trimmed)
277272
{
278-
if (url == null || url.Length < 2)
273+
trimmed = null;
274+
if (input == null)
279275
{
280-
return -1;
276+
return false;
281277
}
282278

283-
var maxTestLength = url.Length - 2;
284-
285-
var start = 0;
286-
for (; start < url.Length; start++)
279+
var url = input.AsSpan();
280+
var start = url.IndexOfAnyExcept(ValidAttributeWhitespaceChars);
281+
if (start < 0)
287282
{
288-
if (start > maxTestLength)
289-
{
290-
return -1;
291-
}
292-
293-
if (!IsCharWhitespace(url[start]))
294-
{
295-
break;
296-
}
297-
}
298-
299-
// Before doing more work, ensure that the URL we're looking at is app-relative.
300-
if (url[start] != '~' || url[start + 1] != '/')
301-
{
302-
return -1;
283+
return false;
303284
}
304285

305-
return start;
306-
}
286+
// Url without leading whitespace.
287+
url = url.Slice(start);
307288

308-
private static string CreateTrimmedString(string input, int start)
309-
{
310-
var end = input.Length - 1;
311-
for (; end >= start; end--)
289+
// Before doing more work, ensure that the URL we're looking at is app-relative.
290+
if (!url.StartsWith("~/"))
312291
{
313-
if (!IsCharWhitespace(input[end]))
314-
{
315-
break;
316-
}
292+
return false;
317293
}
318294

319-
var len = end - start + 1;
295+
var remainingLength = url.LastIndexOfAnyExcept(ValidAttributeWhitespaceChars) + 1;
320296

321297
// Substring returns same string if start == 0 && len == Length
322-
return input.Substring(start, len);
323-
}
324-
325-
private static bool IsCharWhitespace(char ch)
326-
{
327-
return ValidAttributeWhitespaceChars.AsSpan().IndexOf(ch) != -1;
298+
trimmed = input.Substring(start, remainingLength);
299+
return true;
328300
}
329301

330302
private sealed class EncodeFirstSegmentContent : IHtmlContent

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,19 @@ public static TheoryData ResolvableUrlData
1919
return new TheoryData<string, string>
2020
{
2121
{ "~/home/index.html", "/approot/home/index.html" },
22+
{ "~/home/index.html\r\n", "/approot/home/index.html" },
2223
{ " ~/home/index.html", "/approot/home/index.html" },
24+
{ "\u000C~/home/index.html\r\n", "/approot/home/index.html" },
25+
{ "\t ~/home/index.html\n", "/approot/home/index.html" },
26+
{ "\r\n~/home/index.html\u000C\t", "/approot/home/index.html" },
27+
{ "\r~/home/index.html\t", "/approot/home/index.html" },
28+
{ "\n~/home/index.html\u202F", "/approot/home/index.html\u202F" },
2329
{
2430
"~/home/index.html ~/secondValue/index.html",
2531
"/approot/home/index.html ~/secondValue/index.html"
2632
},
33+
{ " ~/ ", "/approot/" },
34+
{ " ~/", "/approot/" },
2735
};
2836
}
2937
}
@@ -166,6 +174,8 @@ public static TheoryData UnresolvableUrlData
166174
{ "/home/index.html ~/second/wontresolve.html" },
167175
{ " ~\\home\\index.html" },
168176
{ "~\\/home/index.html" },
177+
{ " ~" },
178+
{ " " },
169179
};
170180
}
171181
}

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

Lines changed: 23 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#nullable enable
55

6+
using System.Buffers;
67
using System.Diagnostics;
78
using System.Globalization;
89
using System.Text;
@@ -19,6 +20,11 @@ namespace Microsoft.AspNetCore.Mvc.Rendering;
1920
[DebuggerDisplay("{DebuggerToString()}")]
2021
public class TagBuilder : IHtmlContent
2122
{
23+
// Note '.' is valid according to the HTML 4.01 specification. Disallowed here
24+
// to avoid confusion with CSS class selectors or when using jQuery.
25+
private static readonly SearchValues<char> _html401IdChars =
26+
SearchValues.Create("-0123456789:ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz");
27+
2228
private AttributeDictionary? _attributes;
2329
private HtmlContentBuilder? _innerHtml;
2430

@@ -154,51 +160,39 @@ public static string CreateSanitizedId(string? name, string invalidCharReplaceme
154160
}
155161

156162
// If there are no invalid characters in the string, then we don't have to create the buffer.
157-
var firstIndexOfInvalidCharacter = 1;
158-
for (; firstIndexOfInvalidCharacter < name.Length; firstIndexOfInvalidCharacter++)
163+
var indexOfInvalidCharacter = name.AsSpan(1).IndexOfAnyExcept(_html401IdChars);
164+
var firstChar = name[0];
165+
var startsWithAsciiLetter = char.IsAsciiLetter(firstChar);
166+
if (startsWithAsciiLetter && indexOfInvalidCharacter < 0)
159167
{
160-
if (!Html401IdUtil.IsValidIdCharacter(name[firstIndexOfInvalidCharacter]))
161-
{
162-
break;
163-
}
168+
return name;
164169
}
165170

166-
var firstChar = name[0];
167-
var startsWithAsciiLetter = char.IsAsciiLetter(firstChar);
168171
if (!startsWithAsciiLetter)
169172
{
170173
// The first character must be a letter according to the HTML 4.01 specification.
171174
firstChar = 'z';
172175
}
173176

174-
if (firstIndexOfInvalidCharacter == name.Length && startsWithAsciiLetter)
175-
{
176-
return name;
177-
}
178-
179177
var stringBuffer = new StringBuilder(name.Length);
180178
stringBuffer.Append(firstChar);
179+
var remainingName = name.AsSpan(1);
181180

182-
// Characters until 'firstIndexOfInvalidCharacter' have already been checked for validity.
183-
// So just copy them. This avoids running them through Html401IdUtil.IsValidIdCharacter again.
184-
for (var index = 1; index < firstIndexOfInvalidCharacter; index++)
185-
{
186-
stringBuffer.Append(name[index]);
187-
}
188-
189-
for (var index = firstIndexOfInvalidCharacter; index < name.Length; index++)
181+
// Copy values until an invalid character found. Replace the invalid character with the replacement string
182+
// and search for the next invalid character.
183+
while (remainingName.Length > 0)
190184
{
191-
var thisChar = name[index];
192-
if (Html401IdUtil.IsValidIdCharacter(thisChar))
185+
if (indexOfInvalidCharacter < 0)
193186
{
194-
stringBuffer.Append(thisChar);
195-
}
196-
else
197-
{
198-
stringBuffer.Append(invalidCharReplacement);
187+
stringBuffer.Append(remainingName);
188+
break;
199189
}
200-
}
201190

191+
stringBuffer.Append(remainingName.Slice(0, indexOfInvalidCharacter));
192+
stringBuffer.Append(invalidCharReplacement);
193+
remainingName = remainingName.Slice(indexOfInvalidCharacter + 1);
194+
indexOfInvalidCharacter = remainingName.IndexOfAnyExcept(_html401IdChars);
195+
}
202196
return stringBuffer.ToString();
203197
}
204198

@@ -418,28 +412,4 @@ public void WriteTo(TextWriter writer, HtmlEncoder encoder)
418412
TagBuilder.WriteTo(_tagBuilder, writer, encoder, _tagRenderMode);
419413
}
420414
}
421-
422-
private static class Html401IdUtil
423-
{
424-
public static bool IsValidIdCharacter(char testChar)
425-
{
426-
return char.IsAsciiLetterOrDigit(testChar) || IsAllowableSpecialCharacter(testChar);
427-
}
428-
429-
private static bool IsAllowableSpecialCharacter(char testChar)
430-
{
431-
switch (testChar)
432-
{
433-
case '-':
434-
case '_':
435-
case ':':
436-
// Note '.' is valid according to the HTML 4.01 specification. Disallowed here to avoid
437-
// confusion with CSS class selectors or when using jQuery.
438-
return true;
439-
440-
default:
441-
return false;
442-
}
443-
}
444-
}
445415
}

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ public void WriteTo_IgnoresIdAttributeCase(TagRenderMode renderingMode, string e
9595
[InlineData("0", "z")]
9696
[InlineData("-", "z")]
9797
[InlineData(",", "z")]
98+
[InlineData(",.", "z-")]
99+
[InlineData("a\uD83D\uDE0A", "a--")]
100+
[InlineData("a\uD83D\uDE0A.", "a---")]
101+
[InlineData("\uD83D\uDE0A", "z-")]
102+
[InlineData("\uD83D\uDE0A.", "z--")]
103+
[InlineData(",a", "za")]
104+
[InlineData("a,", "a-")]
105+
[InlineData("a0,", "a0-")]
106+
[InlineData("a,0,", "a-0-")]
107+
[InlineData("a,0", "a-0")]
98108
[InlineData("00Hello,World", "z0Hello-World")]
99109
[InlineData(",,Hello,,World,,", "z-Hello--World--")]
100110
[InlineData("-_:Hello-_:Hello-_:", "z_:Hello-_:Hello-_:")]
@@ -110,6 +120,23 @@ public void CreateSanitizedIdCreatesId(string input, string output)
110120
Assert.Equal(output, result);
111121
}
112122

123+
[Fact]
124+
public void CreateSanitizedIdCreatesIdReplacesAllInvalidCharacters()
125+
{
126+
foreach (char c in Enumerable.Range(char.MinValue, char.MaxValue))
127+
{
128+
var result = TagBuilder.CreateSanitizedId($"a{c}", "z");
129+
if (char.IsAsciiLetterOrDigit(c) || c == '-' || c == '_' || c == ':')
130+
{
131+
Assert.Equal($"a{c}", result);
132+
}
133+
else
134+
{
135+
Assert.Equal("az", result);
136+
}
137+
}
138+
}
139+
113140
[Theory]
114141
[InlineData("attribute", "value", "<p attribute=\"HtmlEncode[[value]]\"></p>")]
115142
[InlineData("attribute", null, "<p attribute=\"\"></p>")]

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using BenchmarkDotNet.Attributes;
5+
using Microsoft.AspNetCore.Mvc.Rendering;
6+
7+
namespace Microsoft.AspNetCore.Mvc.Microbenchmarks;
8+
9+
public class TagBuilderBenchmark
10+
{
11+
[Benchmark]
12+
public string ValidFieldName() => TagBuilder.CreateSanitizedId("LongIdForFieldWithMaxLength", "z");
13+
14+
[Benchmark]
15+
public string InvalidFieldName() => TagBuilder.CreateSanitizedId("LongIdForField$WithMaxLength", "z");
16+
}

0 commit comments

Comments
 (0)