QueryStringEnumerable API (#33910)

* Add QueryStringEnumerable

* Use QueryStringEnumerable in QueryFeature

* Move decoding logic into QueryStringEnumerable

* Simplify parsing logic. Add test and support for another edge case.

Previously, the logic would trim leading whitespace from keys, but only for keys with a value. I don't know why we did this, why we didn't trim trailing whitespace, and why we didn't do it for keys with empty value. Also it was inconsistent - we did it in QueryHelper, but not in QueryFeature.

* Make into a public API

* More unit test cases

* Typo

* Add unit test to clarify QueryHelpers whitespace handling

* Fix access modifier

* CR: Remove unwanted test

* Optimize: avoid allocations in common cases

Author: SteveSandersonMS

src/Http/Http/src/Features/QueryFeature.cs

@@ -2,13 +2,10 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Buffers;
 using System.Collections.Generic;
 using System.Runtime.CompilerServices;
-using System.Runtime.InteropServices;
-using System.Runtime.Intrinsics;
-using System.Runtime.Intrinsics.X86;
 using Microsoft.AspNetCore.Internal;
+using Microsoft.AspNetCore.WebUtilities;
 using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.Http.Features
@@ -19,7 +16,7 @@ namespace Microsoft.AspNetCore.Http.Features
     public class QueryFeature : IQueryFeature
     {
         // Lambda hoisted to static readonly field to improve inlining https://github.com/dotnet/roslyn/issues/13624
-        private readonly static Func<IFeatureCollection, IHttpRequestFeature?> _nullRequestFeature = f => null;
+        private static readonly Func<IFeatureCollection, IHttpRequestFeature?> _nullRequestFeature = f => null;
 
         private FeatureReferences<IHttpRequestFeature> _features;
 
@@ -113,48 +110,10 @@ public IQueryCollection Query
             }
 
             var accumulator = new KvpAccumulator();
-            var query = queryString.AsSpan();
-
-            if (query[0] == '?')
+            var enumerable = new QueryStringEnumerable(queryString.AsSpan());
+            foreach (var pair in enumerable)
             {
-                query = query[1..];
-            }
-
-            while (!query.IsEmpty)
-            {
-                var delimiterIndex = query.IndexOf('&');
-
-                var querySegment = delimiterIndex >= 0
-                    ? query.Slice(0, delimiterIndex)
-                    : query;
-
-                var equalIndex = querySegment.IndexOf('=');
-
-                if (equalIndex >= 0)
-                {
-                    var name = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(0, equalIndex));
-                    var value = SpanHelper.ReplacePlusWithSpace(querySegment.Slice(equalIndex + 1));
-
-                    accumulator.Append(
-                        Uri.UnescapeDataString(name),
-                        Uri.UnescapeDataString(value));
-                }
-                else
-                {
-                    if (!querySegment.IsEmpty)
-                    {
-                        var name = SpanHelper.ReplacePlusWithSpace(querySegment);
-
-                        accumulator.Append(Uri.UnescapeDataString(name));
-                    }
-                }
-
-                if (delimiterIndex < 0)
-                {
-                    break;
-                }
-
-                query = query.Slice(delimiterIndex + 1);
+                accumulator.Append(pair.DecodeName(), pair.DecodeValue());
             }
 
             return accumulator.HasValues
@@ -171,8 +130,8 @@ internal struct KvpAccumulator
             private AdaptiveCapacityDictionary<string, StringValues> _accumulator;
             private AdaptiveCapacityDictionary<string, List<string>> _expandingAccumulator;
 
-            public void Append(ReadOnlySpan<char> key, ReadOnlySpan<char> value = default)
-                => Append(key.ToString(), value.IsEmpty ? string.Empty : value.ToString());
+            public void Append(ReadOnlySpan<char> key, ReadOnlySpan<char> value)
+                => Append(key.ToString(), value.ToString());
 
             /// <summary>
             /// This API supports infrastructure and is not intended to be used
@@ -263,58 +222,5 @@ public AdaptiveCapacityDictionary<string, StringValues> GetResults()
                 return _accumulator ?? new AdaptiveCapacityDictionary<string, StringValues>(0, StringComparer.OrdinalIgnoreCase);
             }
         }
-
-        private static class SpanHelper
-        {
-            private static readonly SpanAction<char, IntPtr> s_replacePlusWithSpace = ReplacePlusWithSpaceCore;
-
-            [MethodImpl(MethodImplOptions.AggressiveInlining)]
-            public static unsafe string ReplacePlusWithSpace(ReadOnlySpan<char> span)
-            {
-                fixed (char* ptr = &MemoryMarshal.GetReference(span))
-                {
-                    return string.Create(span.Length, (IntPtr)ptr, s_replacePlusWithSpace);
-                }
-            }
-
-            private static unsafe void ReplacePlusWithSpaceCore(Span<char> buffer, IntPtr state)
-            {
-                fixed (char* ptr = &MemoryMarshal.GetReference(buffer))
-                {
-                    var input = (ushort*)state.ToPointer();
-                    var output = (ushort*)ptr;
-
-                    var i = (nint)0;
-                    var n = (nint)(uint)buffer.Length;
-
-                    if (Sse41.IsSupported && n >= Vector128<ushort>.Count)
-                    {
-                        var vecPlus = Vector128.Create((ushort)'+');
-                        var vecSpace = Vector128.Create((ushort)' ');
-
-                        do
-                        {
-                            var vec = Sse2.LoadVector128(input + i);
-                            var mask = Sse2.CompareEqual(vec, vecPlus);
-                            var res = Sse41.BlendVariable(vec, vecSpace, mask);
-                            Sse2.Store(output + i, res);
-                            i += Vector128<ushort>.Count;
-                        } while (i <= n - Vector128<ushort>.Count);
-                    }
-
-                    for (; i < n; ++i)
-                    {
-                        if (input[i] != '+')
-                        {
-                            output[i] = input[i];
-                        }
-                        else
-                        {
-                            output[i] = ' ';
-                        }
-                    }
-                }
-            }
-        }
     }
 }

src/Http/WebUtilities/src/Microsoft.AspNetCore.WebUtilities.csproj

@@ -4,7 +4,7 @@
     <Description>ASP.NET Core utilities, such as for working with forms, multipart messages, and query strings.</Description>
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <IsAspNetCoreApp>true</IsAspNetCoreApp>
-    <DefineConstants>$(DefineConstants);WebEncoders_In_WebUtilities</DefineConstants>
+    <DefineConstants>$(DefineConstants);WebEncoders_In_WebUtilities;QueryStringEnumerable_In_WebUtilities</DefineConstants>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore</PackageTags>
@@ -13,6 +13,7 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <Compile Include="$(SharedSourceRoot)QueryStringEnumerable.cs" />
     <Compile Include="$(SharedSourceRoot)WebEncoders\**\*.cs" />
     <Compile Include="$(SharedSourceRoot)UrlDecoder\**\*.cs" />
   </ItemGroup>

src/Http/WebUtilities/src/PublicAPI.Unshipped.txt

@@ -2,6 +2,17 @@
 *REMOVED*static Microsoft.AspNetCore.WebUtilities.QueryHelpers.ParseNullableQuery(string! queryString) -> System.Collections.Generic.Dictionary<string!, Microsoft.Extensions.Primitives.StringValues>?
 Microsoft.AspNetCore.WebUtilities.FileBufferingReadStream.MemoryThreshold.get -> int
 Microsoft.AspNetCore.WebUtilities.FileBufferingWriteStream.MemoryThreshold.get -> int
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.DecodeName() -> System.ReadOnlySpan<char>
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.DecodeValue() -> System.ReadOnlySpan<char>
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.EncodedName.get -> System.ReadOnlySpan<char>
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.EncodedValue.get -> System.ReadOnlySpan<char>
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.Enumerator
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.Enumerator.Current.get -> Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.Enumerator.MoveNext() -> bool
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.GetEnumerator() -> Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.Enumerator
+Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.QueryStringEnumerable(System.ReadOnlySpan<char> queryString) -> void
 override Microsoft.AspNetCore.WebUtilities.BufferedReadStream.ReadAsync(System.Memory<byte> buffer, System.Threading.CancellationToken cancellationToken) -> System.Threading.Tasks.ValueTask<int>
 override Microsoft.AspNetCore.WebUtilities.FileBufferingWriteStream.WriteAsync(System.ReadOnlyMemory<byte> buffer, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask
 static Microsoft.AspNetCore.WebUtilities.QueryHelpers.ParseNullableQuery(string? queryString) -> System.Collections.Generic.Dictionary<string!, Microsoft.Extensions.Primitives.StringValues>?

src/Http/WebUtilities/src/QueryHelpers.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using System.Text;
 using System.Text.Encodings.Web;
+using Microsoft.AspNetCore.Internal;
 using Microsoft.Extensions.Primitives;
 
 namespace Microsoft.AspNetCore.WebUtilities
@@ -172,59 +173,11 @@ public static Dictionary<string, StringValues> ParseQuery(string? queryString)
         public static Dictionary<string, StringValues>? ParseNullableQuery(string? queryString)
         {
             var accumulator = new KeyValueAccumulator();
+            var enumerable = new QueryStringEnumerable(queryString);
 
-            if (string.IsNullOrEmpty(queryString) || queryString == "?")
+            foreach (var pair in enumerable)
             {
-                return null;
-            }
-
-            int scanIndex = 0;
-            if (queryString[0] == '?')
-            {
-                scanIndex = 1;
-            }
-
-            int textLength = queryString.Length;
-            int equalIndex = queryString.IndexOf('=');
-            if (equalIndex == -1)
-            {
-                equalIndex = textLength;
-            }
-            while (scanIndex < textLength)
-            {
-                int delimiterIndex = queryString.IndexOf('&', scanIndex);
-                if (delimiterIndex == -1)
-                {
-                    delimiterIndex = textLength;
-                }
-                if (equalIndex < delimiterIndex)
-                {
-                    while (scanIndex != equalIndex && char.IsWhiteSpace(queryString[scanIndex]))
-                    {
-                        ++scanIndex;
-                    }
-                    string name = queryString.Substring(scanIndex, equalIndex - scanIndex);
-                    string value = queryString.Substring(equalIndex + 1, delimiterIndex - equalIndex - 1);
-                    accumulator.Append(
-                        Uri.UnescapeDataString(name.Replace('+', ' ')),
-                        Uri.UnescapeDataString(value.Replace('+', ' ')));
-                    equalIndex = queryString.IndexOf('=', delimiterIndex);
-                    if (equalIndex == -1)
-                    {
-                        equalIndex = textLength;
-                    }
-                }
-                else
-                {
-                    if (delimiterIndex > scanIndex)
-                    {
-                        string name = queryString.Substring(scanIndex, delimiterIndex - scanIndex);
-                        accumulator.Append(
-                            Uri.UnescapeDataString(name.Replace('+', ' ')),
-                            string.Empty);
-                    }
-                }
-                scanIndex = delimiterIndex + 1;
+                accumulator.Append(pair.DecodeName().ToString(), pair.DecodeValue().ToString());
             }
 
             if (!accumulator.HasValues)