Indicate if the request has a body

[Tratcher]

#24175 In HTTP/2 and later the app can't tell from examining request headers if there's a request body. Content-Length is optional and Transfer-Encoding: chunked is prohibited. The only way for the app to know if there's a body is to try to read it. That's not great for scenarios like YARP that need to act differently if there's a request body, but want to avoid reading it as long as possible.

For back-compat Http.Sys/IIS adds a fake Transfer-Encoding: chunked header to HTTP/2 requests that have bodies and no Content-Length.

Edit: I've reworked this as a first class feature and property bool? HttpRequest.CanHaveBody. Returns null for not implemented, false if the server is sure there's no body, or true if there might be a body. I say might because with HTTP/2 or chunked the body could still end up being empty, you just don't know that up front.

Done:

• Kestrel
• HttpSys
• TestServer - Note TestServer has two modes:
  • HttpClient/HttpRequestMessage - I implemented CanHaveBody for this one, it maps directly to having an HttpContent.
  • Direct HttpContext - Not implemented. In this mode TestServer isn't involved in the request body processing, that's handled directly by the calling test (e.g. they directly set HttpRequest.Body with their own stream). The test would also set CanHaveBody if it cared.
  • I also implemented When streaming content to TestServer, Transfer-Encoding header is missing. #21677, automatically including the chunked header for HTTP/1.1 bodies.

TODO:

• IIS - Having issues running tests. Unable to run IIS Express tests locally #25107

The API:

         /// <summary>
        /// Indicates if the request may have a body.
        /// </summary>
        /// <remarks>
        /// This returns true when:
        /// - It's an HTTP/1.x request with a non-zero Content-Length or a 'Transfer-Encoding: chunked' header.
        /// - It's an HTTP/2 request that did not set the END_STREAM flag on the initial headers frame.
        /// The final request body length may still be zero for the chunked or HTTP/2 scenarios.
        ///
        /// This returns false when:
        /// - It's an HTTP/1.x request with no Content-Length or 'Transfer-Encoding: chunked' header, or the Content-Length is 0.
        /// - It's an HTTP/2 request that set END_STREAM on the initial headers frame.
        /// When false, the request body should never return data.
        ///
        /// This returns null if the underlying server has not implemented the feature.
        /// </remarks>
        bool IHttpRequestBodyDetectionFeature?.CanHaveBody { get; }

[davidfowl]

Why don't we add a feature?

[Tratcher]

Why don't we add a feature?

Like IHttpRequestBodyExistsFeature.HasBody? And HttpRequest.HasBody()?

[davidfowl]

Adding a new required feature is a breaking change. For this to be reliable we need to know if the server can even detect empty bodies before asking if the body is empty. If HttpRequest.HasBody returning false if the server hasn't implemented the feature then that's broken as well. We need to represent those states on the feature.

[Tratcher]

Make it a bool? that returns null if not implemented?

[Tratcher]

How's that? @davidfowl @halter73? If you like it I'll do IIS and TestServer also.

[ghost]

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

• The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
• The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
• Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

[Tratcher]

I'll follow-up for IIS in a later PR, I can't get those tests to run locally.

[halter73]

Maybe file an issue to verify our servers have consistent behavior when trying to read the request body of a Connection: Upgrade request.

[ghost]

Hello @Tratcher!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.