Skip to content

QueryStringEnumerable now works in terms of ReadOnlyMemory<char> #34001

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 1, 2021
Merged

QueryStringEnumerable now works in terms of ReadOnlyMemory<char> #34001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95d53bd
QueryStringEnumerable now works in terms of ReadOnlyMemory<char>
SteveSandersonMS Jul 1, 2021
41c3ec1
CR updates
SteveSandersonMS Jul 1, 2021
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
4 changes: 2 additions & 2 deletions src/Http/Http/src/Features/QueryFeature.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ public IQueryCollection Query
}

var accumulator = new KvpAccumulator();
var enumerable = new QueryStringEnumerable(queryString.AsSpan());
var enumerable = new QueryStringEnumerable(queryString);
foreach (var pair in enumerable)
{
accumulator.Append(pair.DecodeName(), pair.DecodeValue());
accumulator.Append(pair.DecodeName().Span, pair.DecodeValue().Span);
}

return accumulator.HasValues
Expand Down
11 changes: 6 additions & 5 deletions src/Http/WebUtilities/src/PublicAPI.Unshipped.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ Microsoft.AspNetCore.WebUtilities.FileBufferingReadStream.MemoryThreshold.get ->
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.EncodedNameValuePair.DecodeName() -> System.ReadOnlyMemory<char>
Copy link
Contributor

@pranavkm pranavkm Jul 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@halter73 to follow up with the .NET API review folks to figure out if returning ReadOnlyMemory is acceptable. API tentatively approved by ASP.NET Core review.

Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.DecodeValue() -> System.ReadOnlyMemory<char>
Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.EncodedName.get -> System.ReadOnlyMemory<char>
Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.EncodedNameValuePair.EncodedValue.get -> System.ReadOnlyMemory<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
Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.QueryStringEnumerable(System.ReadOnlyMemory<char> queryString) -> void
Microsoft.AspNetCore.WebUtilities.QueryStringEnumerable.QueryStringEnumerable(string? 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>?
Expand Down
47 changes: 28 additions & 19 deletions src/Shared/QueryStringEnumerable.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,24 @@ namespace Microsoft.AspNetCore.Internal
#else
internal
#endif
readonly ref struct QueryStringEnumerable
readonly struct QueryStringEnumerable
{
private readonly ReadOnlySpan<char> _queryString;
private readonly ReadOnlyMemory<char> _queryString;

/// <summary>
/// Constructs an instance of <see cref="QueryStringEnumerable"/>.
/// </summary>
/// <param name="queryString">The query string.</param>
public QueryStringEnumerable(ReadOnlySpan<char> queryString)
public QueryStringEnumerable(string? queryString)
: this(queryString.AsMemory())
{
}

/// <summary>
/// Constructs an instance of <see cref="QueryStringEnumerable"/>.
/// </summary>
/// <param name="queryString">The query string.</param>
public QueryStringEnumerable(ReadOnlyMemory<char> queryString)
{
_queryString = queryString;
}
Expand All @@ -45,21 +54,21 @@ public Enumerator GetEnumerator()
/// <summary>
/// Represents a single name/value pair extracted from a query string during enumeration.
/// </summary>
public readonly ref struct EncodedNameValuePair
public readonly struct EncodedNameValuePair
{
/// <summary>
/// Gets the name from this name/value pair in its original encoded form.
/// To get the decoded string, call <see cref="DecodeName"/>.
/// </summary>
public readonly ReadOnlySpan<char> EncodedName { get; }
public readonly ReadOnlyMemory<char> EncodedName { get; }

/// <summary>
/// Gets the value from this name/value pair in its original encoded form.
/// To get the decoded string, call <see cref="DecodeValue"/>.
/// </summary>
public readonly ReadOnlySpan<char> EncodedValue { get; }
public readonly ReadOnlyMemory<char> EncodedValue { get; }

internal EncodedNameValuePair(ReadOnlySpan<char> encodedName, ReadOnlySpan<char> encodedValue)
internal EncodedNameValuePair(ReadOnlyMemory<char> encodedName, ReadOnlyMemory<char> encodedValue)
{
EncodedName = encodedName;
EncodedValue = encodedValue;
Expand All @@ -69,37 +78,37 @@ internal EncodedNameValuePair(ReadOnlySpan<char> encodedName, ReadOnlySpan<char>
/// Decodes the name from this name/value pair.
/// </summary>
/// <returns>Characters representing the decoded name.</returns>
public ReadOnlySpan<char> DecodeName()
public ReadOnlyMemory<char> DecodeName()
=> Decode(EncodedName);

/// <summary>
/// Decodes the value from this name/value pair.
/// </summary>
/// <returns>Characters representing the decoded value.</returns>
public ReadOnlySpan<char> DecodeValue()
public ReadOnlyMemory<char> DecodeValue()
=> Decode(EncodedValue);

private static ReadOnlySpan<char> Decode(ReadOnlySpan<char> chars)
private static ReadOnlyMemory<char> Decode(ReadOnlyMemory<char> chars)
{
// If the value is short, it's cheap to check up front if it really needs decoding. If it doesn't,
// then we can save some allocations.
return chars.Length < 16 && chars.IndexOfAny('%', '+') < 0
return chars.Length < 16 && chars.Span.IndexOfAny('%', '+') < 0
? chars
: Uri.UnescapeDataString(SpanHelper.ReplacePlusWithSpace(chars));
: Uri.UnescapeDataString(SpanHelper.ReplacePlusWithSpace(chars.Span)).AsMemory();
}
}

/// <summary>
/// An enumerator that supplies the name/value pairs from a URI query string.
/// </summary>
public ref struct Enumerator
public struct Enumerator
{
private ReadOnlySpan<char> _query;
private ReadOnlyMemory<char> _query;

internal Enumerator(ReadOnlySpan<char> query)
internal Enumerator(ReadOnlyMemory<char> query)
{
Current = default;
_query = query.IsEmpty || query[0] != '?'
_query = query.IsEmpty || query.Span[0] != '?'
? query
: query.Slice(1);
}
Expand All @@ -118,8 +127,8 @@ public bool MoveNext()
while (!_query.IsEmpty)
{
// Chomp off the next segment
ReadOnlySpan<char> segment;
var delimiterIndex = _query.IndexOf('&');
ReadOnlyMemory<char> segment;
var delimiterIndex = _query.Span.IndexOf('&');
if (delimiterIndex >= 0)
{
segment = _query.Slice(0, delimiterIndex);
Expand All @@ -132,7 +141,7 @@ public bool MoveNext()
}

// If it's nonempty, emit it
var equalIndex = segment.IndexOf('=');
var equalIndex = segment.Span.IndexOf('=');
if (equalIndex >= 0)
{
Current = new EncodedNameValuePair(
Expand Down
8 changes: 3 additions & 5 deletions src/Shared/test/Shared.Tests/QueryStringEnumerableTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,12 @@ public void DecodingWorks(string queryString, string expectedDecodedName, string
}

[Fact]
public void DecodingRetainsSpansIfDecodingNotNeeded()
public void DecodingReusesMemoryIfDecodingNotNeeded()
{
foreach (var kvp in new QueryStringEnumerable("?key=value"))
{
Assert.True(MemoryExtensions.Overlaps(kvp.EncodedName, kvp.DecodeName(), out var nameOffset));
Assert.True(MemoryExtensions.Overlaps(kvp.EncodedValue, kvp.DecodeValue(), out var valueOffset));
Assert.Equal(0, nameOffset);
Assert.Equal(0, valueOffset);
Assert.True(kvp.EncodedName.Equals(kvp.DecodeName()));
Assert.True(kvp.EncodedValue.Equals(kvp.DecodeValue()));
}
}

Expand Down