Merge branch 'release/2.1' into 'internal/release/2.1'

- resolve PatchConfig.props conflict

6 files changed, 46 insertions, 33 deletions

diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props
index 2824ff20a7..48f0a6d658 100644
--- a/eng/PatchConfig.props
+++ b/eng/PatchConfig.props
@@ -85,6 +85,7 @@ Later on, this will be checked using this condition:
   </PropertyGroup>
   <PropertyGroup Condition=" '$(VersionPrefix)' == '2.1.25' ">
     <PackagesInPatch>
+      Microsoft.AspNetCore.Server.Kestrel.Core;
     </PackagesInPatch>
   </PropertyGroup>
 </Project>
diff --git a/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs b/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs
index 033c8d9e1b..a29936b57c 100644
--- a/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs
+++ b/src/Servers/Kestrel/Core/src/BadHttpRequestException.cs
@@ -111,9 +111,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
                 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;
diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx
index 2b0326e1d7..adfdbe110c 100644
--- a/src/Servers/Kestrel/Core/src/CoreStrings.resx
+++ b/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>
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs
index 7aeea58f34..a323a6b611 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1MessageBody.cs
@@ -271,13 +271,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 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);
             }
 
diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs b/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs
index ee27b5cb96..e1c96f203f 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http/RequestRejectionReason.cs
@@ -32,7 +32,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         MissingHostHeader,
         MultipleHostHeaders,
         InvalidHostHeader,
-        UpgradeRequestCannotHavePayload,
         RequestBodyExceedsContentLength
     }
 }
diff --git a/src/Servers/Kestrel/test/FunctionalTests/UpgradeTests.cs b/src/Servers/Kestrel/test/FunctionalTests/UpgradeTests.cs
index 2b4027d37c..97ed5aa265 100644
--- a/src/Servers/Kestrel/test/FunctionalTests/UpgradeTests.cs
+++ b/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,9 +148,15 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         }
 
         [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",
@@ -156,22 +164,27 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     "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,9 +206,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
         }
 
         [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",
@@ -203,14 +225,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                     "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");
             }
         }