Use PipeReader JsonSerializer overloads

[BrennanConroy]

Use PipeReader JsonSerializer overloads

Description

Updates MVC and HttpRequestJsonExtensions (minimal APIs and user code) to use the new PipeReader overloads on JsonSerializer.DeserializeAsync. Because the ReadOnlySequence property on Utf8JsonReader is now being exercised, we're being cautious of custom JsonConverter implementations that don't properly handle that scenario, by providing an AppContext switch to go back to the Stream based overloads. We'll remove the switch in 11.0.

Closes #60657

Customer Impact

Decreased buffer pool usage and byte copying. Also, finishes the work we started in 9.0 to integrate System.IO.Pipelines into Json as an alternative to using Stream.

Regression?

• Yes
• No

Risk

• High
• Medium
• Low

Change at the JSON layer has been well tested. Decent test coverage for the HTTP layer that makes use of JSON. We also have added an AppContext switch to go back to the previous code as we anticipate user code having bugs in custom JsonConverter implementations.

Verification

• Manual (required)
• Automated

Packaging changes reviewed?

• Yes
• No
• N/A

[Copilot]

Pull Request Overview

This PR updates ASP.NET Core's JSON deserialization to use the new PipeReader overloads on JsonSerializer.DeserializeAsync instead of the existing Stream overloads for better performance. The changes include an AppContext switch (Microsoft.AspNetCore.UseStreamBasedJsonParsing) to allow fallback to the old Stream based overloads for backwards compatibility with custom JsonConverter implementations that may not handle ReadOnlySequence properly.

• Adds AppContext switch for backward compatibility with custom JsonConverter implementations
• Updates JSON deserialization to use PipeReader overloads by default for UTF-8 encoding
• Refactors resource management to only dispose transcoding streams when necessary

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs	Updates MVC input formatter to use PipeReader JsonSerializer overloads with AppContext switch
src/Http/Http.Extensions/src/HttpRequestJsonExtensions.cs	Updates HTTP request JSON extensions to use PipeReader overloads with AppContext switch
src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.BindAsync.cs	Skips test that relies on Stream.Position reset behavior
src/Http/Http.Extensions/test/HttpRequestJsonExtensionsTests.cs	Updates test assertion for broader exception type

[Copilot]

The AppContext switch name contains a typo or inconsistency. The switch is named 'UseStreamBasedJsonParsing' but the behavior is actually controlling whether to use Stream vs PipeReader overloads. Consider renaming to 'Microsoft.AspNetCore.Json.UseStreamOverloads' for clarity.

Suggested change

	_useStreamJsonOverload = AppContext.TryGetSwitch("Microsoft.AspNetCore.UseStreamBasedJsonParsing", out var isEnabled) && isEnabled;
	_useStreamJsonOverload =
	(AppContext.TryGetSwitch("Microsoft.AspNetCore.Json.UseStreamOverloads", out var isNewEnabled) && isNewEnabled) ||
	(AppContext.TryGetSwitch("Microsoft.AspNetCore.UseStreamBasedJsonParsing", out var isOldEnabled) && isOldEnabled);

[Copilot]

The same AppContext switch name issue exists here. Both classes use the same switch name but it should be consistent and clearly indicate its purpose of choosing between Stream and PipeReader overloads.

Suggested change

	private static readonly bool _useStreamJsonOverload = AppContext.TryGetSwitch("Microsoft.AspNetCore.UseStreamBasedJsonParsing", out var isEnabled) && isEnabled;
	private static readonly bool _useStreamJsonOverload = AppContext.TryGetSwitch("Microsoft.AspNetCore.Json.UseStreamOverPipeReader", out var isEnabled) && isEnabled;

[captainsafia]

LGTM assuming fix for skipped test is forthcoming.

[captainsafia]

Nit: Is there anyway for us to dedupe this logic across the different extension methods? Maybe a helper that figures out what serialization invocation to use based on the context switch and return the associated task?

[BrennanConroy]

The difficulty here is that each of the HttpRequestJsonExtension methods use a different overload of DeserializeAsync.
We could in theory share the other checks, but I'm not sure it's much cleaner:

public static async ValueTask<object?> ReadFromJsonAsync(
    this HttpRequest request,
    Type type,
    JsonSerializerContext context,
    CancellationToken cancellationToken = default)
{
    Stream? inputStream = null;
    bool isTranscodedStream = false;
    ValueTask<object?> deserializeTask;

    try
    {
        (var pipeReader, inputStream, isTranscodedStream) = Shared(request);
        if (pipeReader is not null)
        {
            deserializeTask = JsonSerializer.DeserializeAsync(pipeReader, type, context, cancellationToken);
        }
        else
        {
            deserializeTask = JsonSerializer.DeserializeAsync(inputStream, type, context, cancellationToken);
        }

        return await deserializeTask;
    }
    finally
    {
        if (isTranscodedStream)
        {
            await inputStream!.DisposeAsync();
        }
    }
}

private static (PipeReader?, Stream?, bool) Shared(HttpRequest request)
{
    if (!request.HasJsonContentType(out var charset))
    {
        ThrowContentTypeError(request);
    }

    var encoding = GetEncodingFromCharset(charset);

    if (encoding == null || encoding.CodePage == Encoding.UTF8.CodePage)
    {
        if (_useStreamJsonOverload)
        {
            return (null, request.Body, false);
        }
        return (request.BodyReader, null, false);
    }

    var inputStream = Encoding.CreateTranscodingStream(request.Body, encoding, Encoding.UTF8, leaveOpen: true);
    return (null, inputStream, true);
}

[BrennanConroy]

/backport to main

[github-actions]

Started backporting to main: https://github.com/dotnet/aspnetcore/actions/runs/16512533424

[davidfowl]

woop!