Rewrite Uri.EscapeString

Several public methods (Uri.EscapeDataString, Uri.EscapeUriString) and a bunch of internal call sites rely on the internal EscapeString helper.  This helper has several issues with it:
- It uses unsafe code.
- It unnecessarily requires and copies through a char[] to get to a string when a string is the required result.
- It has a lot of complexity around the handling of unicode.

This PR rewrites it to utilize Span, Rune, and other newer features in a way that enables it to be both safe and efficient.  Most inputs ends up being faster, and for very long inputs, it's much, much faster.  The use of ValueStringBuilder also results in less memory allocation, in some cases significantly.

The use of Rune also fixes two arguable bugs in the existing implementation around invalid Unicode sequences, which is why a couple tests were tweaked:
- Some but not all invalid unicode patterns result in replacement characters being used: a few invalid sequences (e.g. just a high surrogate) result in an exception.  We should be standardized on using replacement characters for all such invalid sequences.
- Some patterns with invalid unicode patterns actually result in unnecessary encoding, e.g. `Uri.EscapeDataString("\uD800\uD800a")` results in `a` being encoded.

Author: stephentoub

src/System.Private.Uri/src/System.Private.Uri.csproj

@@ -37,6 +37,9 @@
     <Compile Include="System\UriPartial.cs" />
     <Compile Include="System\UriScheme.cs" />
     <Compile Include="System\UriSyntax.cs" />
+    <Compile Include="$(CommonPath)\CoreLib\System\Text\ValueStringBuilder.cs">
+      <Link>Common\CoreLib\System\Text\ValueStringBuilder.cs</Link>
+    </Compile>
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsWindows)' == 'true'">
     <Compile Include="System\Uri.Windows.cs" />

src/System.Private.Uri/src/System/Uri.cs

@@ -1373,8 +1373,8 @@ public static string HexEscape(char character)
             return string.Create(3, character, (Span<char> chars, char c) =>
             {
                 chars[0] = '%';
-                chars[1] = UriHelper.s_hexUpperChars[(c & 0xf0) >> 4];
-                chars[2] = UriHelper.s_hexUpperChars[c & 0xf];
+                chars[1] = (char)UriHelper.HexUpperChars[(c & 0xf0) >> 4];
+                chars[2] = (char)UriHelper.HexUpperChars[c & 0xf];
             });
         }
 
@@ -1897,18 +1897,9 @@ private static bool CheckForColonInFirstPathSegment(string uriString)
             return (index >= 0 && uriString[index] == ':');
         }
 
-        internal static unsafe string InternalEscapeString(string rawString)
-        {
-            if ((object)rawString == null)
-                return string.Empty;
-
-            int position = 0;
-            char[]? dest = UriHelper.EscapeString(rawString, 0, rawString.Length, null, ref position, true, '?', '#', '%');
-            if ((object?)dest == null)
-                return rawString;
-
-            return new string(dest, 0, position);
-        }
+        internal static unsafe string InternalEscapeString(string rawString) =>
+            rawString is null ? string.Empty :
+            UriHelper.EscapeString(rawString, checkExistingEscaped: true, UriHelper.UnreservedReservedTable, '?', '#');
 
         //
         //  This method is called first to figure out the scheme or a simple file path
@@ -2469,11 +2460,7 @@ private unsafe void CreateHostString()
                         flags |= Flags.E_HostNotCanonical;
                         if (NotAny(Flags.UserEscaped))
                         {
-                            int position = 0;
-                            char[]? dest = UriHelper.EscapeString(host, 0, host.Length, null, ref position, true, '?',
-                                '#', IsImplicitFile ? c_DummyChar : '%');
-                            if ((object?)dest != null)
-                                host = new string(dest, 0, position);
+                            host = UriHelper.EscapeString(host, checkExistingEscaped: !IsImplicitFile, UriHelper.UnreservedReservedTable, '?', '#');
                         }
                         else
                         {
@@ -2771,8 +2758,10 @@ private string ReCreateParts(UriComponents parts, ushort nonCanonical, UriFormat
                         case UriFormat.UriEscaped:
                             if (NotAny(Flags.UserEscaped))
                             {
-                                chars = UriHelper.EscapeString(_string, _info.Offset.User, _info.Offset.Host, chars,
-                                    ref count, true, '?', '#', '%')!; // TODO-NULLABLE: Remove ! when [NotNullIfNotNull] respected
+                                chars = UriHelper.EscapeString(
+                                    _string.AsSpan(_info.Offset.User, _info.Offset.Host - _info.Offset.User),
+                                    chars, ref count,
+                                    checkExistingEscaped: true, '?', '#');
                             }
                             else
                             {
@@ -2938,8 +2927,12 @@ private string ReCreateParts(UriComponents parts, ushort nonCanonical, UriFormat
                         case UriFormat.UriEscaped:
                             //Can Assert IsImplicitfile == false
                             if (NotAny(Flags.UserEscaped))
-                                chars = UriHelper.EscapeString(_string, delimiterAwareIndex, _info.Offset.Fragment, chars,
-                                    ref count, true, '#', c_DummyChar, '%')!; // TODO-NULLABLE: Remove ! when [NotNullIfNotNull] respected
+                            {
+                                chars = UriHelper.EscapeString(
+                                    _string.AsSpan(delimiterAwareIndex, _info.Offset.Fragment - delimiterAwareIndex),
+                                    chars, ref count,
+                                    checkExistingEscaped: true, '#');
+                            }
                             else
                             {
                                 UriHelper.UnescapeString(_string, delimiterAwareIndex, _info.Offset.Fragment, chars,
@@ -2991,8 +2984,12 @@ private string ReCreateParts(UriComponents parts, ushort nonCanonical, UriFormat
                     {
                         case UriFormat.UriEscaped:
                             if (NotAny(Flags.UserEscaped))
-                                chars = UriHelper.EscapeString(_string, delimiterAwareIndex, _info.Offset.End, chars,
-                                    ref count, true, c_DummyChar, c_DummyChar, '%')!; // TODO-NULLABLE: Remove ! when [NotNullIfNotNull] respected
+                            {
+                                chars = UriHelper.EscapeString(
+                                    _string.AsSpan(delimiterAwareIndex, _info.Offset.End - delimiterAwareIndex),
+                                    chars, ref count,
+                                    checkExistingEscaped: true);
+                            }
                             else
                             {
                                 UriHelper.UnescapeString(_string, delimiterAwareIndex, _info.Offset.End, chars,
@@ -4623,8 +4620,11 @@ private unsafe char[] GetCanonicalPath(char[] dest, ref int pos, UriFormat forma
                             str = str.Remove(dosPathIdx + _info.Offset.Path - 1, 1);
                             str = str.Insert(dosPathIdx + _info.Offset.Path - 1, ":");
                         }
-                        dest = UriHelper.EscapeString(str, _info.Offset.Path, _info.Offset.Query, dest, ref end, true,
-                            '?', '#', IsImplicitFile ? c_DummyChar : '%')!; // TODO-NULLABLE: Remove ! when [NotNullIfNotNull] respected
+
+                        dest = UriHelper.EscapeString(
+                            str.AsSpan(_info.Offset.Path, _info.Offset.Query - _info.Offset.Path),
+                            dest, ref end,
+                            checkExistingEscaped: !IsImplicitFile, '?', '#');
                     }
                     else
                     {
@@ -4636,8 +4636,7 @@ private unsafe char[] GetCanonicalPath(char[] dest, ref int pos, UriFormat forma
                 // On Unix, escape '\\' in path of file uris to '%5C' canonical form.
                 if (!IsWindowsSystem && InFact(Flags.BackslashInPath) && _syntax.NotAny(UriSyntaxFlags.ConvertPathSlashes) && _syntax.InFact(UriSyntaxFlags.FileLikeUri) && !IsImplicitFile)
                 {
-                    string str = new string(dest, pos, end - pos);
-                    dest = UriHelper.EscapeString(str, 0, str.Length, dest, ref pos, true, '\\', c_DummyChar, '%')!; // TODO-NULLABLE: Remove ! when [NotNullIfNotNull] respected
+                    dest = UriHelper.EscapeString(new string(dest, pos, end - pos), dest, ref pos, checkExistingEscaped: true, '\\');
                     end = pos;
                 }
             }
@@ -4685,9 +4684,7 @@ private unsafe char[] GetCanonicalPath(char[] dest, ref int pos, UriFormat forma
                 if (formatAs == UriFormat.UriEscaped && NotAny(Flags.UserEscaped) && InFact(Flags.E_PathNotCanonical))
                 {
                     //Note: Flags.UserEscaped check is solely based on trusting the user
-                    string srcString = new string(dest, pos, end - pos);
-                    dest = UriHelper.EscapeString(srcString, 0, end - pos, dest, ref pos, true, '?', '#',
-                        IsImplicitFile ? c_DummyChar : '%')!; // TODO-NULLABLE: Remove ! when [NotNullIfNotNull] respected
+                    dest = UriHelper.EscapeString(new string(dest, pos, end - pos), dest, ref pos, checkExistingEscaped: !IsImplicitFile, '?', '#');
                     end = pos;
                 }
             }
@@ -5337,22 +5334,9 @@ protected virtual string Unescape(string path)
         }
 
         [Obsolete("The method has been deprecated. Please use GetComponents() or static EscapeUriString() to escape a Uri component or a string. https://go.microsoft.com/fwlink/?linkid=14202")]
-        protected static string EscapeString(string? str)
-        {
-            // This method just does not make sense as protected
-            // It should go public static asap
-
-            if (str == null)
-            {
-                return string.Empty;
-            }
-
-            int destStart = 0;
-            char[]? dest = UriHelper.EscapeString(str, 0, str.Length, null, ref destStart, true, '?', '#', '%');
-            if (dest == null)
-                return str;
-            return new string(dest, 0, destStart);
-        }
+        protected static string EscapeString(string? str) =>
+            str is null ? string.Empty :
+            UriHelper.EscapeString(str, checkExistingEscaped: true, UriHelper.UnreservedReservedTable, '?', '#');
 
         //
         // CheckSecurity

src/System.Private.Uri/src/System/UriBuilder.cs

@@ -242,11 +242,9 @@ public string Path
             }
             set
             {
-                if ((value == null) || (value.Length == 0))
-                {
-                    value = "/";
-                }
-                _path = Uri.InternalEscapeString(value.Replace('\\', '/'));
+                _path = string.IsNullOrEmpty(value) ?
+                    "/" :
+                    Uri.InternalEscapeString(value.Replace('\\', '/'));
                 _changed = true;
             }
         }

src/System.Private.Uri/src/System/UriExt.cs

@@ -3,8 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Globalization;
-using System.Text;
-using System.Runtime.InteropServices;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 
@@ -250,7 +248,7 @@ private unsafe bool CheckForEscapedUnreserved(string data)
                         && tempPtr[i + 1] >= '0' && tempPtr[i + 1] <= '7') // max 0x7F
                     {
                         char ch = UriHelper.EscapedAscii(tempPtr[i + 1], tempPtr[i + 2]);
-                        if (ch != c_DummyChar && UriHelper.Is3986Unreserved(ch))
+                        if (ch != c_DummyChar && UriHelper.IsUnreserved(ch))
                         {
                             return true;
                         }
@@ -569,46 +567,15 @@ public static string UnescapeDataString(string stringToUnescape)
             }
         }
 
-        //
         // Where stringToEscape is intended to be a completely unescaped URI string.
         // This method will escape any character that is not a reserved or unreserved character, including percent signs.
-        // Note that EscapeUriString will also do not escape a '#' sign.
-        //
-        public static string EscapeUriString(string stringToEscape)
-        {
-            if ((object)stringToEscape == null)
-                throw new ArgumentNullException(nameof(stringToEscape));
-
-            if (stringToEscape.Length == 0)
-                return string.Empty;
-
-            int position = 0;
-            char[]? dest = UriHelper.EscapeString(stringToEscape, 0, stringToEscape.Length, null, ref position, true,
-                c_DummyChar, c_DummyChar, c_DummyChar);
-            if ((object?)dest == null)
-                return stringToEscape;
-            return new string(dest, 0, position);
-        }
+        public static string EscapeUriString(string stringToEscape) =>
+            UriHelper.EscapeString(stringToEscape, checkExistingEscaped: false, UriHelper.UnreservedReservedTable);
 
-        //
         // Where stringToEscape is intended to be URI data, but not an entire URI.
         // This method will escape any character that is not an unreserved character, including percent signs.
-        //
-        public static string EscapeDataString(string stringToEscape)
-        {
-            if ((object)stringToEscape == null)
-                throw new ArgumentNullException(nameof(stringToEscape));
-
-            if (stringToEscape.Length == 0)
-                return string.Empty;
-
-            int position = 0;
-            char[]? dest = UriHelper.EscapeString(stringToEscape, 0, stringToEscape.Length, null, ref position, false,
-                c_DummyChar, c_DummyChar, c_DummyChar);
-            if (dest == null)
-                return stringToEscape;
-            return new string(dest, 0, position);
-        }
+        public static string EscapeDataString(string stringToEscape) =>
+            UriHelper.EscapeString(stringToEscape, checkExistingEscaped: false, UriHelper.UnreservedTable);
 
         //
         // Cleans up the specified component according to Iri rules
@@ -781,19 +748,12 @@ private unsafe string GetRelativeSerializationString(UriFormat format)
         {
             if (format == UriFormat.UriEscaped)
             {
-                if (_string.Length == 0)
-                    return string.Empty;
-                int position = 0;
-                char[]? dest = UriHelper.EscapeString(_string, 0, _string.Length, null, ref position, true,
-                    c_DummyChar, c_DummyChar, '%');
-                if ((object?)dest == null)
-                    return _string;
-                return new string(dest, 0, position);
+                return UriHelper.EscapeString(_string, checkExistingEscaped: true, UriHelper.UnreservedReservedTable);
             }
-
             else if (format == UriFormat.Unescaped)
+            {
                 return UnescapeDataString(_string);
-
+            }
             else if (format == UriFormat.SafeUnescaped)
             {
                 if (_string.Length == 0)
@@ -806,7 +766,9 @@ private unsafe string GetRelativeSerializationString(UriFormat format)
                 return new string(dest, 0, position);
             }
             else
+            {
                 throw new ArgumentOutOfRangeException(nameof(format));
+            }
         }
 
         //