Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Commit

Permalink
Clean up Uri.UnescapeDataString (#42225)
Browse files Browse the repository at this point in the history
- Use string.IndexOf rather than an open-coded, unsafe loop.
- Avoid an unnecessary SequenceEquals at the end: we're only at this point if a `%` was found highlighting that something escaped was found.
- Use stack memory for smaller inputs if possible, to avoid unnecessary ArrayPool interaction
- Remove an unnecessary argument to a helper function.
- Fix ValueStringBuilder.Grow to only copy the contained data.
  • Loading branch information
stephentoub authored Oct 31, 2019
1 parent d2a3440 commit 04f79d9
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 30 deletions.
2 changes: 1 addition & 1 deletion src/Common/src/CoreLib/System/Text/ValueStringBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ private void Grow(int additionalCapacityBeyondPos)

char[] poolArray = ArrayPool<char>.Shared.Rent(Math.Max(_pos + additionalCapacityBeyondPos, _chars.Length * 2));

_chars.CopyTo(poolArray);
_chars.Slice(0, _pos).CopyTo(poolArray);

char[]? toReturn = _arrayToReturnToPool;
_chars = _arrayToReturnToPool = poolArray;
Expand Down
39 changes: 15 additions & 24 deletions src/System.Private.Uri/src/System/UriExt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -546,30 +546,21 @@ public static string UnescapeDataString(string stringToUnescape)
if (stringToUnescape.Length == 0)
return string.Empty;

unsafe
{
fixed (char* pStr = stringToUnescape)
{
int position;
for (position = 0; position < stringToUnescape.Length; ++position)
if (pStr[position] == '%')
break;

if (position == stringToUnescape.Length)
return stringToUnescape;

UnescapeMode unescapeMode = UnescapeMode.Unescape | UnescapeMode.UnescapeAll;
position = 0;
ValueStringBuilder vsb = new ValueStringBuilder(stringToUnescape.Length);
UriHelper.UnescapeString(stringToUnescape, 0, stringToUnescape.Length, ref vsb, ref position,
c_DummyChar, c_DummyChar, c_DummyChar, unescapeMode, null, false);

ReadOnlySpan<char> resultSpan = vsb.AsSpan(0, position);
string result = resultSpan.SequenceEqual(stringToUnescape) ? stringToUnescape : resultSpan.ToString();
vsb.Dispose();
return result;
}
}
int position = stringToUnescape.IndexOf('%');
if (position == -1)
return stringToUnescape;

var vsb = new ValueStringBuilder(stackalloc char[256]);
vsb.EnsureCapacity(stringToUnescape.Length);

vsb.Append(stringToUnescape.AsSpan(0, position));
UriHelper.UnescapeString(
stringToUnescape, position, stringToUnescape.Length, ref vsb,
c_DummyChar, c_DummyChar, c_DummyChar,
UnescapeMode.Unescape | UnescapeMode.UnescapeAll,
syntax: null, isQuery: false);

return vsb.ToString();
}

// Where stringToEscape is intended to be a completely unescaped URI string.
Expand Down
7 changes: 2 additions & 5 deletions src/System.Private.Uri/src/System/UriHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -342,14 +342,11 @@ internal static unsafe char[] UnescapeString(char* pStr, int start, int end, cha
// For this reason it returns a char[] that is usually the same ref as the input "dest" value.
//
internal static unsafe void UnescapeString(string input, int start, int end, ref ValueStringBuilder dest,
ref int destPosition, char rsvd1, char rsvd2, char rsvd3, UnescapeMode unescapeMode, UriParser? syntax,
bool isQuery)
char rsvd1, char rsvd2, char rsvd3, UnescapeMode unescapeMode, UriParser? syntax, bool isQuery)
{
fixed (char* pStr = input)
{
UnescapeString(pStr, start, end, ref dest, rsvd1, rsvd2, rsvd3, unescapeMode,
syntax, isQuery);
destPosition = dest.Length;
UnescapeString(pStr, start, end, ref dest, rsvd1, rsvd2, rsvd3, unescapeMode, syntax, isQuery);
}
}
internal static unsafe void UnescapeString(char* pStr, int start, int end, ref ValueStringBuilder dest,
Expand Down

0 comments on commit 04f79d9

Please sign in to comment.