Uri.TryUnescapeDataString throws ArgumentOutOfRangeException

[GerardSmit]

Description

When a specific string is provided to Uri.TryUnescapeDataString, it throws an exception

Reproduction Steps

Execute the following code:

using System;

Uri.TryUnescapeDataString("Prefix %20".AsSpan(), [], out _);

Sharplab

Expected behavior

No exception -> returns false

Actual behavior

Throws ArgumentOutOfRangeException

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

When you change the string that the escape is only at the beginning, it does NOT throw:

using System;

Uri.TryUnescapeDataString("%20suffix".AsSpan(), [], out _);

Sharplab

When you do both, it also throws:

using System;

Uri.TryUnescapeDataString("Prefix%20suffix".AsSpan(), [], out _);

Sharplab

[dotnet-policy-service]

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

[MihuBot]

I'm a bot. Here is a possible related and/or duplicate issue (I may be wrong):

• Uri.TryUnescapeDataString may throw with small destination buffers #124654

[GerardSmit]

Since .NET 11 preview 4 it's fixed (PR #124655, as found by the bot)

Will this fix be backported to .NET 9 and 10, or do I need to work around this in .NET 9 and 10?

[MihaZupan]

We should consider it. This is a new API in .NET 9 that shipped with this bug.
Sadly I only caught it in .NET 11 when we added fuzzing, and this is the first public report of the issue.

One reason the issue is less likely to hit is that unescaping will always produce an output of a smaller length (or equal if there's nothing to change). As such, callers that just rent a buffer that's the same size as the input will always succeed.

#128610

[MihaZupan]

I looked through the usages of this new API on GitHub (there aren't many, but that makes sense since it's new and specific to perf), and none would be impacted by this particular bug (the destination is large enough).

We'll wait with servicing previous releases until we see more real world impact, I'll keep this issue open so it's easier to find.