ReadOnlySpan<char> overload of Uri.UnescapeDataString

[SteveSandersonMS]

Background and Motivation

For context, this emerged in ASP.NET Core at dotnet/aspnetcore#33840 (comment)

Currently, Uri.UnescapeDataString both accepts and returns string. If the source data you're working with is a ReadOnlySpan<char>, then the caller is forced to allocate a string. This is especially inefficient if the caller only needs back a ReadOnlySpan<char>, since in most cases this seems like it should be achievable without any allocations (because the original argument could be returned directly if no transformation was required - see below).

Proposed API

namespace System
{
     public class Uri
     {
+        ReadOnlySpan<char> UnescapeDataString(ReadOnlySpan<char> stringToUnescape) { ... }
     }
}

Of course, if the input data contains escaped characters, then the implementation will need to allocate in order to provide a return value. However the existing logic within string UnescapeDataString(string) recognizes the extremely common case where the input string does not contain escaped characters and knows to return the original string directly. Similarly, the new ReadOnlySpan<char> overload could return the original argument value in this case.

Usage Examples

var unescapedValue = Uri.UnescapeDataString(someCharSpanFromAUri);

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and Motivation

For context, this emerged in ASP.NET Core at dotnet/aspnetcore#33840 (comment)

Currently, Uri.UnescapeDataString both accepts and returns string. If the source data you're working with is a ReadOnlySpan<char>, then the caller is forced to allocate a string. This is especially inefficient if the caller only needs back a ReadOnlySpan<char>, since in most cases this seems like it should be achievable without any allocations (because the original argument could be returned directly if no transformation was required - see below).

Proposed API

namespace System
{
     public class Uri
     {
+        ReadOnlySpan<char> UnescapeDataString(ReadOnlySpan<char> stringToUnescape) { ... }
     }
}

Of course, if the input data contains escaped characters, then the implementation will need to allocate in order to provide a return value. However the existing logic within string UnescapeDataString(string) recognizes the extremely common case where the input string does not contain escaped characters and knows to return the original string directly. Similarly, the new ReadOnlySpan<char> overload could return the original argument value in this case.

Usage Examples

var unescapedValue = Uri.UnescapeDataString(someCharSpanFromAUri);

Author:	SteveSandersonMS
Assignees:	-
Labels:	api-suggestion, area-System.Runtime, untriaged
Milestone:	-

[GrabYourPitchforks]

We try to avoid APIs which allocate then return the allocated memory as a ROS<T> rather than as a concrete type, even if there's only a slight chance of it doing so. Most APIs that return values try to return them as concrete types since these are generally more usable.

One way to address this could be to return a string? instead of a ROS<char>, with a null return value meaning "the original data did not require transformation." It's a little bit weirder of an API shape since the input and output types are different, but it does provide fuller control for the caller. Alternatively, you could have the caller specify both an input and an output buffer, which is more common for transformation-style APIs.

[SteveSandersonMS]

Thanks @GrabYourPitchforks. I wasn't aware of the convention to avoid allocating and returning ROS<T> but I see how that does work against this suggestion.

Alternatively, you could have the caller specify both an input and an output buffer, which is more common for transformation-style APIs.

TBH I don't think the caller would want to supply a buffer given that it's hard to predict the correct size (without an understanding of the decoding rules), and because it leads the caller into a complex world of needing to pool buffers etc.

One way to address this could be to return a string? instead of a ROS, with a null return value meaning "the original data did not require transformation."

It's slightly unusual, but I agree that does match the scenario. Perhaps the API would be something like:

bool DecodeIfRequired(ReadOnlySpan<char> value, [MaybeNullWhen(false)] out string result)

We're not blocked on this for 6.0 (at least, we're not in any way counting on this happening for 6.0!) but is potentially an opportunity for the future.

[Wraith2]

bool DecodeIfRequired(ReadOnlySpan<char> value, [MaybeNullWhen(false)] out string result)

wouldn't that more naturally fit with the try pattern?

bool TryDecode(ReadOnlySpan<char> value, [MaybeNullWhen(false)] out string result)

or even/also

(bool Success, string DecodedValue) TryDecode(ReadOnlySpan<char> value)

so usage would become:

public void DoSomethingWithUri(ReadOnlySpan<char> urlChars)
{
    if (Uri.TryDecode(uriChars) is { Success: true, DecodedValue: var decoded })
    {
        uriChars = decoded;
    }
    // carry on and use uriChars
}

At a later date you could add an overload which has a Span<char> target parameter so you can attempt to preallocate/share buffers for repeated calls and avoid allocations, 'cause you know @davidfowl is going to be all over it if you allocate anywhere in asp.net

[Sergio0694]

I'll admit I'm not the biggest far of value-tuples in public APIs in general, but still I'm not sure I get how would:

if (Uri.TryDecode(uriChars) is { Success: true, DecodedValue: var decoded })
{
}

be in any way better than:

if (Uri.TryDecode(uriChars, out var decoded)
{
}

Isn't it just more verbose and awkward compared to the usual TryFoo pattern? 🤔
The value-tuple API would also additionally lack the nullable flow analysis provided by [MaybeNullWhen].

[Wraith2]

Isn't it just more verbose and awkward compared to the usual TryFoo pattern?

I think so, but I also prefer the purity of separate input/output that using a value tuple allows. If they're going to be added I think the out param one is most likely with the valuetuple one a possibility but I wanted to make sure it was there for consideration.

[tannergooding]

CC. @karelz, this is another API proposal that impacts Uri. It's likely better handled by the networking team where the domain expertise resides.