Skip to content

Replace Uri.UnescapeDataString with UrlDecoder in FormReader - #68183

Open
4bitsteams wants to merge 5 commits into
dotnet:mainfrom
4bitsteams:feature/form-reader-url-decoding
Open

Replace Uri.UnescapeDataString with UrlDecoder in FormReader#68183
4bitsteams wants to merge 5 commits into
dotnet:mainfrom
4bitsteams:feature/form-reader-url-decoding

Conversation

@4bitsteams

Copy link
Copy Markdown

Summary

Replace deprecated Uri.UnescapeDataString with UrlDecoder for URL form data decoding.

  • Uses more accurate UrlDecoder for form data parsing
  • Properly handles UTF-8 percent-encoded sequences
  • Consistent decoding behavior across platforms
  • Better compliance with URL encoding standards

Co-Authored-By: Claude Haiku 4.5 noreply@anthropic.com

Elfocrash and others added 5 commits May 12, 2022 12:21
Co-authored-by: Weihan Li <weihanli@outlook.com>
Complete the OpenAPI dotnet-openapi tool by registering the AddProjectCommand that was previously implemented but not wired up. This enables users to add OpenAPI references from project files.
Use the more accurate UrlDecoder for URL form data decoding instead of
Uri.UnescapeDataString which has known limitations with UTF-8 sequences
and cross-platform behavior differences.

This improves accuracy of form data parsing by:
- Properly handling UTF-8 percent-encoded sequences
- Consistent decoding behavior across platforms
- Better compliance with URL encoding standards

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Aug 3, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Thanks for your PR, @4bitsteams. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@gfoidl gfoidl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One suggestion, but you need to undo the public API changes that are unrelated to this PR.

Comment on lines 268 to 272

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The intermediate allocations could be avoided (if it matters):

Method Value Mean Ratio Allocated Alloc Ratio
BuildWord a+b%2(...)7D%7E [88] 274.82 ns 1.00 400 B 1.00
Suggestion a+b%2(...)7D%7E [88] 202.83 ns 0.74 88 B 0.22
BuildWord foo=bar+baz 83.50 ns 1.00 136 B 1.00
Suggestion foo=bar+baz 58.27 ns 0.70 48 B 0.35

For how see the code in details, method Suggestion1.

benchmark code
using System.Runtime.InteropServices;
using System.Text;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using Microsoft.AspNetCore.Internal;

BenchmarkRunner.Run<Benchmarks>();

[MemoryDiagnoser]
public class Benchmarks
{
    private readonly StringBuilder _builder = new();

    [Params(
        "foo=bar+baz",
        "a+b%22%25%2D%2E%3C%3E%5C%5E%5F%60%7B%7C%7D%7E=%22%25%2D%2E%3C%3E%5C%5E%5F%60%7B%7C%7D%7E"
    )]
    public string Value { get; set; } = "foo=bar+baz";

    private void IterationSetup()
    {
        _builder.Clear();
        _builder.Append(this.Value);
    }

    [Benchmark(Baseline = true)]
    public string BuildWord()
    {
        this.IterationSetup();

        string result = _builder.ToString();
        _builder.Clear();

        byte[] bytes = Encoding.UTF8.GetBytes(result);
        int decodedLength = UrlDecoder.DecodeInPlace(bytes, isFormEncoding: true);
        return Encoding.UTF8.GetString(bytes, 0, decodedLength);
    }

    [Benchmark]
    public string Suggestion()
    {
        this.IterationSetup();

        const int StackAllocBytesThreshold = 256;

        int length = _builder.Length;

        if (length > StackAllocBytesThreshold / sizeof(char))
        {
            return Fallback(_builder);
        }

        Span<char> charBuffer = stackalloc char[StackAllocBytesThreshold / sizeof(char)];
        _builder.CopyTo(0, charBuffer, length);
        _builder.Clear();

        // GetBytes is "narrowing", so 1 char (= 2 bytes) become 1 byte, thus we can re-use the
        // same buffer, w/o overriding contents.
        Span<byte> byteBuffer = MemoryMarshal.Cast<char, byte>(charBuffer);
        int written = Encoding.UTF8.GetBytes(charBuffer.Slice(0, length), byteBuffer);
        int decodedLength = UrlDecoder.DecodeInPlace(byteBuffer.Slice(0, written), isFormEncoding: true);
        return Encoding.UTF8.GetString(byteBuffer.Slice(0, decodedLength));

        static string Fallback(StringBuilder builder)
        {
            string result = builder.ToString();
            builder.Clear();

            byte[] bytes = Encoding.UTF8.GetBytes(result);
            int decodedLength = UrlDecoder.DecodeInPlace(bytes, isFormEncoding: true);
            return Encoding.UTF8.GetString(bytes, 0, decodedLength);
        }
    }
}

Alternatively the UrlDecoder.DecodeInPlace char-overload could be changed to have the isFormEncoding argument as well, to avoid going char -> byte -> char.

Footnotes

  1. just w/o the IterationSetup that I added only for the benchmark, to keep the product code copy & pastable -- the other way is to drop the _builder.Clear from the benchmarks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is unrelated to this PR and would need an API-approval.
Please undo this change in this PR.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants