Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 18 additions & 49 deletions src/Mvc/Mvc.Razor/src/TagHelpers/UrlResolutionTagHelper.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);

Expand All @@ -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);
Expand All @@ -273,58 +268,32 @@ 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 || input.Length < 2)
{
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;
}
return false;
}

// Before doing more work, ensure that the URL we're looking at is app-relative.
if (url[start] != '~' || url[start + 1] != '/')
{
return -1;
}

return start;
}

private static string CreateTrimmedString(string input, int start)
{
var end = input.Length - 1;
for (; end >= start; end--)
if (!url.Slice(start, 2).SequenceEqual("~/"))
ladeak marked this conversation as resolved.
Show resolved Hide resolved
{
if (!IsCharWhitespace(input[end]))
{
break;
}
return false;
}

var len = end - start + 1;
var end = url.Slice(start).LastIndexOfAnyExcept(ValidAttributeWhitespaceChars);

// 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, end + 1);
amcasey marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

private sealed class EncodeFirstSegmentContent : IHtmlContent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,17 @@ public static TheoryData ResolvableUrlData
{
{ "~/home/index.html", "/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/" },
};
}
}
Expand Down
76 changes: 23 additions & 53 deletions src/Mvc/Mvc.ViewFeatures/src/Rendering/TagBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#nullable enable

using System.Buffers;
using System.Diagnostics;
using System.Globalization;
using System.Text;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
ladeak marked this conversation as resolved.
Show resolved Hide resolved
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();
}

Expand Down Expand Up @@ -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;
}
}
}
}
27 changes: 27 additions & 0 deletions src/Mvc/Mvc.ViewFeatures/test/Rendering/TagBuilderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ public void WriteTo_IgnoresIdAttributeCase(TagRenderMode renderingMode, string e
[InlineData("0", "z")]
[InlineData("-", "z")]
[InlineData(",", "z")]
[InlineData(",.", "z-")]
[InlineData("a😊", "a--")]
ladeak marked this conversation as resolved.
Show resolved Hide resolved
[InlineData("a😊.", "a---")]
[InlineData("😊", "z-")]
[InlineData("😊.", "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-_:")]
Expand All @@ -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>")]
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
}
Loading