Identifying if a request has a body #24175 (#24984)

Author: Tratcher

src/Hosting/TestHost/src/ClientHandler.cs

@@ -66,32 +66,35 @@ protected override async Task<HttpResponseMessage> SendAsync(
 
             var contextBuilder = new HttpContextBuilder(_application, AllowSynchronousIO, PreserveExecutionContext);
 
-            var requestContent = request.Content ?? new StreamContent(Stream.Null);
+            var requestContent = request.Content;
 
-            // Read content from the request HttpContent into a pipe in a background task. This will allow the request
-            // delegate to start before the request HttpContent is complete. A background task allows duplex streaming scenarios.
-            contextBuilder.SendRequestStream(async writer =>
+            if (requestContent != null)
             {
-                if (requestContent is StreamContent)
+                // Read content from the request HttpContent into a pipe in a background task. This will allow the request
+                // delegate to start before the request HttpContent is complete. A background task allows duplex streaming scenarios.
+                contextBuilder.SendRequestStream(async writer =>
                 {
+                    if (requestContent is StreamContent)
+                    {
                     // This is odd but required for backwards compat. If StreamContent is passed in then seek to beginning.
                     // This is safe because StreamContent.ReadAsStreamAsync doesn't block. It will return the inner stream.
                     var body = await requestContent.ReadAsStreamAsync();
-                    if (body.CanSeek)
-                    {
+                        if (body.CanSeek)
+                        {
                         // This body may have been consumed before, rewind it.
                         body.Seek(0, SeekOrigin.Begin);
-                    }
+                        }
 
-                    await body.CopyToAsync(writer);
-                }
-                else
-                {
-                    await requestContent.CopyToAsync(writer.AsStream());
-                }
+                        await body.CopyToAsync(writer);
+                    }
+                    else
+                    {
+                        await requestContent.CopyToAsync(writer.AsStream());
+                    }
 
-                await writer.CompleteAsync();
-            });
+                    await writer.CompleteAsync();
+                });
+            }
 
             contextBuilder.Configure((context, reader) =>
             {
@@ -110,6 +113,39 @@ protected override async Task<HttpResponseMessage> SendAsync(
 
                 req.Scheme = request.RequestUri.Scheme;
 
+                var canHaveBody = false;
+                if (requestContent != null)
+                {
+                    canHaveBody = true;
+                    // Chunked takes precedence over Content-Length, don't create a request with both Content-Length and chunked.
+                    if (request.Headers.TransferEncodingChunked != true)
+                    {
+                        // Reading the ContentLength will add it to the Headers‼
+                        // https://github.com/dotnet/runtime/blob/874399ab15e47c2b4b7c6533cc37d27d47cb5242/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs#L68-L87
+                        var contentLength = requestContent.Headers.ContentLength;
+                        if (!contentLength.HasValue && request.Version == HttpVersion.Version11)
+                        {
+                            // HTTP/1.1 requests with a body require either Content-Length or Transfer-Encoding: chunked.
+                            request.Headers.TransferEncodingChunked = true;
+                        }
+                        else if (contentLength == 0)
+                        {
+                            canHaveBody = false;
+                        }
+                    }
+
+                    foreach (var header in requestContent.Headers)
+                    {
+                        req.Headers.Append(header.Key, header.Value.ToArray());
+                    }
+
+                    if (canHaveBody)
+                    {
+                        req.Body = new AsyncStreamWrapper(reader.AsStream(), () => contextBuilder.AllowSynchronousIO);
+                    }
+                }
+                context.Features.Set<IHttpRequestBodyDetectionFeature>(new RequestBodyDetectionFeature(canHaveBody));
+
                 foreach (var header in request.Headers)
                 {
                     // User-Agent is a space delineated single line header but HttpRequestHeaders parses it as multiple elements.
@@ -141,17 +177,6 @@ protected override async Task<HttpResponseMessage> SendAsync(
                     req.PathBase = _pathBase;
                 }
                 req.QueryString = QueryString.FromUriComponent(request.RequestUri);
-
-                // Reading the ContentLength will add it to the Headers‼
-                // https://github.com/dotnet/runtime/blob/874399ab15e47c2b4b7c6533cc37d27d47cb5242/src/libraries/System.Net.Http/src/System/Net/Http/Headers/HttpContentHeaders.cs#L68-L87
-                _ = requestContent.Headers.ContentLength;
-
-                foreach (var header in requestContent.Headers)
-                {
-                    req.Headers.Append(header.Key, header.Value.ToArray());
-                }
-
-                req.Body = new AsyncStreamWrapper(reader.AsStream(), () => contextBuilder.AllowSynchronousIO);
             });
 
             var response = new HttpResponseMessage();

src/Hosting/TestHost/src/RequestBodyDetectionFeature.cs

@@ -0,0 +1,17 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Http.Features;
+
+namespace Microsoft.AspNetCore.TestHost
+{
+    internal class RequestBodyDetectionFeature : IHttpRequestBodyDetectionFeature
+    {
+        public RequestBodyDetectionFeature(bool canHaveBody)
+        {
+            CanHaveBody = canHaveBody;
+        }
+
+        public bool CanHaveBody { get; }
+    }
+}

src/Hosting/TestHost/test/ClientHandlerTests.cs

@@ -111,7 +111,9 @@ public Task ContentLengthWithBodyWorks()
             var contentBytes = Encoding.UTF8.GetBytes("This is a content!");
             var handler = new ClientHandler(new PathString(""), new DummyApplication(context =>
             {
+                Assert.True(context.Request.CanHaveBody());
                 Assert.Equal(contentBytes.LongLength, context.Request.ContentLength);
+                Assert.False(context.Request.Headers.ContainsKey(HeaderNames.TransferEncoding));
 
                 return Task.CompletedTask;
             }));
@@ -122,11 +124,13 @@ public Task ContentLengthWithBodyWorks()
         }
 
         [Fact]
-        public Task ContentLengthWithNoBodyWorks()
+        public Task ContentLengthNotPresentWithNoBody()
         {
             var handler = new ClientHandler(new PathString(""), new DummyApplication(context =>
             {
-                Assert.Equal(0, context.Request.ContentLength);
+                Assert.False(context.Request.CanHaveBody());
+                Assert.Null(context.Request.ContentLength);
+                Assert.False(context.Request.Headers.ContainsKey(HeaderNames.TransferEncoding));
 
                 return Task.CompletedTask;
             }));
@@ -136,11 +140,13 @@ public Task ContentLengthWithNoBodyWorks()
         }
 
         [Fact]
-        public Task ContentLengthWithChunkedTransferEncodingWorks()
+        public Task ContentLengthWithImplicitChunkedTransferEncodingWorks()
         {
             var handler = new ClientHandler(new PathString(""), new DummyApplication(context =>
             {
+                Assert.True(context.Request.CanHaveBody());
                 Assert.Null(context.Request.ContentLength);
+                Assert.Equal("chunked", context.Request.Headers[HeaderNames.TransferEncoding]);
 
                 return Task.CompletedTask;
             }));
@@ -150,6 +156,26 @@ public Task ContentLengthWithChunkedTransferEncodingWorks()
             return httpClient.PostAsync("http://example.com", new UnlimitedContent());
         }
 
+        [Fact]
+        public Task ContentLengthWithExplicitChunkedTransferEncodingWorks()
+        {
+            var handler = new ClientHandler(new PathString(""), new DummyApplication(context =>
+            {
+                Assert.True(context.Request.CanHaveBody());
+                Assert.Null(context.Request.ContentLength);
+                Assert.Equal("chunked", context.Request.Headers[HeaderNames.TransferEncoding]);
+
+                return Task.CompletedTask;
+            }));
+
+            var httpClient = new HttpClient(handler);
+            httpClient.DefaultRequestHeaders.TransferEncodingChunked = true;
+            var contentBytes = Encoding.UTF8.GetBytes("This is a content!");
+            var content = new ByteArrayContent(contentBytes);
+
+            return httpClient.PostAsync("http://example.com", content);
+        }
+
         [Fact]
         public async Task ServerTrailersSetOnResponseAfterContentRead()
         {

src/Hosting/TestHost/test/HttpContextBuilderTests.cs

@@ -40,6 +40,7 @@ public async Task ExpectedValuesAreAvailable()
             Assert.Equal("/A/Path", context.Request.PathBase.Value);
             Assert.Equal("/and/file.txt", context.Request.Path.Value);
             Assert.Equal("?and=query", context.Request.QueryString.Value);
+            Assert.Null(context.Request.CanHaveBody());
             Assert.NotNull(context.Request.Body);
             Assert.NotNull(context.Request.Headers);
             Assert.NotNull(context.Response.Headers);

src/Hosting/TestHost/test/TestServerTests.cs

@@ -228,6 +228,7 @@ public async Task DispoingTheRequestBodyDoesNotDisposeClientStreams()
 
             var stream = new ThrowOnDisposeStream();
             stream.Write(Encoding.ASCII.GetBytes("Hello World"));
+            stream.Seek(0, SeekOrigin.Begin);
             var response = await server.CreateClient().PostAsync("/", new StreamContent(stream));
             Assert.True(response.IsSuccessStatusCode);
             Assert.Equal("Hello World", await response.Content.ReadAsStringAsync());

src/Hosting/TestHost/test/Utilities.cs

@@ -3,6 +3,8 @@
 
 using System;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Testing;
 
 namespace Microsoft.AspNetCore.TestHost
@@ -14,5 +16,10 @@ internal static class Utilities
         internal static Task<T> WithTimeout<T>(this Task<T> task) => task.TimeoutAfter(DefaultTimeout);
 
         internal static Task WithTimeout(this Task task) => task.TimeoutAfter(DefaultTimeout);
+
+        internal static bool? CanHaveBody(this HttpRequest request)
+        {
+            return request.HttpContext.Features.Get<IHttpRequestBodyDetectionFeature>()?.CanHaveBody;
+        }
     }
 }

src/Http/Http.Features/src/IHttpRequestBodyDetectionFeature.cs

@@ -0,0 +1,29 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+namespace Microsoft.AspNetCore.Http.Features
+{
+    /// <summary>
+    /// Used to indicate if the request can have a body.
+    /// </summary>
+    public interface IHttpRequestBodyDetectionFeature
+    {
+        /// <summary>
+        /// Indicates if the request can 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/1.x request with Connection: Upgrade (e.g. WebSockets). There is no HTTP request body for these requests and
+        ///   no data should be received until after the upgrade.
+        /// - It's an HTTP/2 request that set END_STREAM on the initial headers frame.
+        /// When false, the request body should never return data.
+        /// </remarks>
+        bool CanHaveBody { get; }
+    }
+}

src/Servers/HttpSys/src/FeatureContext.cs

@@ -23,6 +23,7 @@ namespace Microsoft.AspNetCore.Server.HttpSys
 {
     internal class FeatureContext :
         IHttpRequestFeature,
+        IHttpRequestBodyDetectionFeature,
         IHttpConnectionFeature,
         IHttpResponseFeature,
         IHttpResponseBodyFeature,
@@ -212,6 +213,8 @@ string IHttpRequestFeature.Scheme
             set { _scheme = value; }
         }
 
+        bool IHttpRequestBodyDetectionFeature.CanHaveBody => Request.HasEntityBody;
+
         IPAddress IHttpConnectionFeature.LocalIpAddress
         {
             get

src/Servers/HttpSys/src/RequestProcessing/Request.cs

@@ -135,6 +135,7 @@ public long? ContentLength
             {
                 if (_contentBoundaryType == BoundaryType.None)
                 {
+                    // Note Http.Sys adds the Transfer-Encoding: chunked header to HTTP/2 requests with bodies for back compat.
                     string transferEncoding = Headers[HttpKnownHeaderNames.TransferEncoding];
                     if (string.Equals("chunked", transferEncoding?.Trim(), StringComparison.OrdinalIgnoreCase))
                     {

src/Servers/HttpSys/src/StandardFeatureCollection.cs

@@ -17,6 +17,7 @@ internal sealed class StandardFeatureCollection : IFeatureCollection
         private static readonly Dictionary<Type, Func<FeatureContext, object>> _featureFuncLookup = new Dictionary<Type, Func<FeatureContext, object>>()
         {
             { typeof(IHttpRequestFeature), _identityFunc },
+            { typeof(IHttpRequestBodyDetectionFeature), _identityFunc },
             { typeof(IHttpConnectionFeature), _identityFunc },
             { typeof(IHttpResponseFeature), _identityFunc },
             { typeof(IHttpResponseBodyFeature), _identityFunc },