Reading form with invalid content throws NullReferenceException

[martincostello]

Describe the bug

While working on some error handling cases for a PR, I needed to make HttpRequest.ReadFormAsync() throw an InvalidDataException. In the process of creating a test case that did that, I found that posting an invalid form to the web application would cause a NullReferenceException to be thrown on the server when the form was read.

I can't replicate this with 5.0.10, 6.0.100-rc.1.21463.6, or with release/6.0, but I can in main as of 406515e for v7.

To Reproduce

Add the test application endpoint and corresponding test from 91a43d6 to the repo, and then run the test.

app.MapPost("/form", async (HttpContext context) =>
{
    try
    {
        var request = context.Request;
        var form = await request.ReadFormAsync();
        context.Response.StatusCode = 204;
    }
    catch (InvalidDataException)
    {
        context.Response.StatusCode = 400;
    }
});

[Fact]
public async Task Application_Handles_Invalid_Form()
{
    // Arrange
    using var content = new StringContent("&&&", null, "application/x-www-form-urlencoded");

    // Act
    using var response = await Client.PostAsync("/form", content);

    // Assert
    Assert.NotEqual(HttpStatusCode.InternalServerError, response.StatusCode);
}

Exceptions (if any)

 Microsoft.AspNetCore.Mvc.FunctionalTests.SimpleWithWebApplicationBuilderTests.Application_Handles_Invalid_Form
   Source: SimpleWithWebApplicationBuilderTests.cs line 235
   Duration: 271 ms

  Message: 
System.NullReferenceException : Object reference not set to an instance of an object.

  Stack Trace: 
KeyValueAccumulator.Append(String key, String value) line 36
FormPipeReader.AppendAndVerify(KeyValueAccumulator& accumulator, String decodedKey, String decodedValue) line 369
FormPipeReader.ParseFormValuesFast(ReadOnlySpan`1 span, KeyValueAccumulator& accumulator, Boolean isFinalBlock, Int32& consumed) line 233
FormPipeReader.ParseFormValues(ReadOnlySequence`1& buffer, KeyValueAccumulator& accumulator, Boolean isFinalBlock) line 142
FormPipeReader.ReadFormAsync(CancellationToken cancellationToken) line 108
FormFeature.InnerReadFormAsync(CancellationToken cancellationToken) line 182
<<<Main>$>b__0_10>d.MoveNext() line 48
--- End of stack trace from previous location ---
EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger) line 79
CultureReplacerMiddleware.Invoke(HttpContext context) line 45
<<SendAsync>g__RunRequestAsync|0>d.MoveNext() line 114
--- End of stack trace from previous location ---
ClientHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) line 191
HttpClient.<SendAsync>g__Core|83_0(HttpRequestMessage request, HttpCompletionOption completionOption, CancellationTokenSource cts, Boolean disposeCts, CancellationTokenSource pendingRequestsCts, CancellationToken originalCancellationToken)
SimpleWithWebApplicationBuilderTests.Application_Handles_Invalid_Form() line 241
--- End of stack trace from previous location ---

Further technical details

main @ 406515e

[adityamandaleeka]

Triage: this update looks suspicious: 8bb1281#diff-4a901400441403949ef2fe3565a0947b65049427fcc40936dddbc8d85a8a290b

@dougbu

[dougbu]

@adityamandaleeka yes, I agree. I'm on build ops at the moment and haven't prioritized undoing the damage I caused in #35547. I'm taking this bug and will get to it later this week.

[dougbu]

/fyi @Tratcher @sebastienros