[2.1] Allow/ignore upgrades with bodies (#28908)

* Allow/ignore upgrades with bodies #17081

* Update patchconfig.props

Author: Tratcher

eng/PatchConfig.props

@@ -83,4 +83,9 @@ Later on, this will be checked using this condition:
       Microsoft.AspNetCore.DataProtection.AzureStorage;
     </PackagesInPatch>
   </PropertyGroup>
+  <PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.25' ">
+    <PackagesInPatch>
+      Microsoft.AspNetCore.Server.Kestrel.Core;
+    </PackagesInPatch>
+  </PropertyGroup>
 </Project>

src/Servers/Kestrel/Core/src/BadHttpRequestException.cs

@@ -111,9 +111,6 @@ internal static BadHttpRequestException GetException(RequestRejectionReason reas
                 case RequestRejectionReason.InvalidHostHeader:
                     ex = new BadHttpRequestException(CoreStrings.BadRequest_InvalidHostHeader, StatusCodes.Status400BadRequest, reason);
                     break;
-                case RequestRejectionReason.UpgradeRequestCannotHavePayload:
-                    ex = new BadHttpRequestException(CoreStrings.BadRequest_UpgradeRequestCannotHavePayload, StatusCodes.Status400BadRequest, reason);
-                    break;
                 default:
                     ex = new BadHttpRequestException(CoreStrings.BadRequest, StatusCodes.Status400BadRequest, reason);
                     break;

src/Servers/Kestrel/Core/src/CoreStrings.resx

@@ -198,9 +198,6 @@
   <data name="BadRequest_UnrecognizedHTTPVersion" xml:space="preserve">
     <value>Unrecognized HTTP version: '{detail}'</value>
   </data>
-  <data name="BadRequest_UpgradeRequestCannotHavePayload" xml:space="preserve">
-    <value>Requests with 'Connection: Upgrade' cannot have content in the request body.</value>
-  </data>
   <data name="FallbackToIPv4Any" xml:space="preserve">
     <value>Failed to bind to http://[::]:{port} (IPv6Any). Attempting to bind to http://0.0.0.0:{port} instead.</value>
   </data>

src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs

@@ -271,13 +271,13 @@ public static MessageBody For(
                 keepAlive = (connectionOptions & ConnectionOptions.KeepAlive) == ConnectionOptions.KeepAlive;
             }
 
-            if (upgrade)
+            // Ignore upgrades if the request has a body. Technically it's possible to support, but we'd have to add a lot
+            // more logic to allow reading/draining the normal body before the connection could be fully upgraded.
+            // See https://tools.ietf.org/html/rfc7230#section-6.7, https://tools.ietf.org/html/rfc7540#section-3.2
+            if (upgrade
+                && headers.ContentLength.GetValueOrDefault() == 0
+                && headers.HeaderTransferEncoding.Count == 0)
             {
-                if (headers.HeaderTransferEncoding.Count > 0 || (headers.ContentLength.HasValue && headers.ContentLength.Value != 0))
-                {
-                    BadHttpRequestException.Throw(RequestRejectionReason.UpgradeRequestCannotHavePayload);
-                }
-
                 return new ForUpgrade(context);
             }
 

src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs

@@ -32,7 +32,6 @@ public enum RequestRejectionReason
         MissingHostHeader,
         MultipleHostHeaders,
         InvalidHostHeader,
-        UpgradeRequestCannotHavePayload,
         RequestBodyExceedsContentLength
     }
 }

src/Servers/Kestrel/test/FunctionalTests/UpgradeTests.cs

@@ -1,15 +1,17 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// 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 System;
 using System.IO;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Server.Kestrel.Core;
 using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure;
 using Microsoft.AspNetCore.Server.Kestrel.Tests;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.Logging.Testing;
+using Microsoft.Net.Http.Headers;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
@@ -146,32 +148,43 @@ await connection.Receive("HTTP/1.1 101 Switching Protocols",
         }
 
         [Fact]
-        public async Task RejectsRequestWithContentLengthAndUpgrade()
+        public async Task AcceptsRequestWithContentLengthAndUpgrade()
         {
-            using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory)))
+            using (var server = new TestServer(async context =>
+            {
+                var feature = context.Features.Get<IHttpUpgradeFeature>();
+                Assert.False(feature.IsUpgradableRequest);
+                Assert.Equal(1, context.Request.ContentLength);
+                Assert.Equal(1, await context.Request.Body.ReadAsync(new byte[10], 0, 10));
+            }, new TestServiceContext(LoggerFactory)))
             using (var connection = server.CreateConnection())
             {
                 await connection.Send("POST / HTTP/1.1",
                     "Host:",
                     "Content-Length: 1",
                     "Connection: Upgrade",
                     "",
-                    "");
+                    "A");
 
-                await connection.ReceiveForcedEnd(
-                    "HTTP/1.1 400 Bad Request",
-                    "Connection: close",
-                    $"Date: {server.Context.DateHeaderValue}",
-                    "Content-Length: 0",
-                    "",
-                    "");
+                await connection.Receive("HTTP/1.1 200 OK");
             }
         }
 
         [Fact]
         public async Task AcceptsRequestWithNoContentLengthAndUpgrade()
         {
-            using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory)))
+            using (var server = new TestServer(async context =>
+            {
+                var feature = context.Features.Get<IHttpUpgradeFeature>();
+                Assert.True(feature.IsUpgradableRequest);
+
+                if (HttpMethods.IsPost(context.Request.Method))
+                {
+                    Assert.Equal(0, context.Request.ContentLength);
+                }
+
+                Assert.Equal(0, await context.Request.Body.ReadAsync(new byte[10], 0, 10));
+            }, new TestServiceContext(LoggerFactory)))
             {
                 using (var connection = server.CreateConnection())
                 {
@@ -193,24 +206,30 @@ await connection.Send("POST / HTTP/1.1",
         }
 
         [Fact]
-        public async Task RejectsRequestWithChunkedEncodingAndUpgrade()
+        public async Task AcceptsRequestWithChunkedEncodingAndUpgrade()
         {
-            using (var server = new TestServer(context => Task.CompletedTask, new TestServiceContext(LoggerFactory)))
+            using (var server = new TestServer(async context =>
+            {
+                var feature = context.Features.Get<IHttpUpgradeFeature>();
+                Assert.Null(context.Request.ContentLength);
+                Assert.False(feature.IsUpgradableRequest);
+                Assert.True(HttpMethods.IsPost(context.Request.Method));
+                Assert.False(feature.IsUpgradableRequest);
+                Assert.Equal("chunked", context.Request.Headers[HeaderNames.TransferEncoding]);
+                Assert.Equal(11, await context.Request.Body.ReadAsync(new byte[12], 0, 12));
+            }, new TestServiceContext(LoggerFactory)))
             using (var connection = server.CreateConnection())
             {
                 await connection.Send("POST / HTTP/1.1",
                     "Host:",
                     "Transfer-Encoding: chunked",
                     "Connection: Upgrade",
                     "",
-                    "");
-                await connection.ReceiveForcedEnd(
-                    "HTTP/1.1 400 Bad Request",
-                    "Connection: close",
-                    $"Date: {server.Context.DateHeaderValue}",
-                    "Content-Length: 0",
+                    "B", "Hello World",
+                    "0",
                     "",
                     "");
+                await connection.Receive("HTTP/1.1 200 OK");
             }
         }