Reject POST and PUT requests with no Content-Length or Transfer-Encoding (#1130).

src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs

@@ -102,6 +102,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel
                 case RequestRejectionReason.FinalTransferCodingNotChunked:
                     ex = new BadHttpRequestException($"Final transfer coding is not \"chunked\": \"{value}\"", 400);
                     break;
+                case RequestRejectionReason.LengthRequired:
+                    ex = new BadHttpRequestException($"{value} request contains no Content-Length or Transfer-Encoding header", 411);
+                    break;
+                case RequestRejectionReason.LengthRequiredHttp10:
+                    ex = new BadHttpRequestException($"{value} request contains no Content-Length header", 400);
+                    break;
                 default:
                     ex = new BadHttpRequestException("Bad request.", 400);
                     break;

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs

@@ -6,6 +6,7 @@ using System.IO;
 using System.Numerics;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
 using Microsoft.Extensions.Internal;
 using Microsoft.Extensions.Primitives;
@@ -266,17 +267,29 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
                 return new ForChunkedEncoding(keepAlive, headers, context);
             }
 
-            var unparsedContentLength = headers.HeaderContentLength.ToString();
+            var unparsedContentLength = headers.HeaderContentLength;
-            if (unparsedContentLength.Length > 0)
+            if (unparsedContentLength.Count > 0)
             {
-                long contentLength;
+                try
-                if (!long.TryParse(unparsedContentLength, out contentLength) || contentLength < 0)
+                {
+                    var contentLength = FrameHeaders.ParseContentLength(unparsedContentLength);
+                    return new ForContentLength(keepAlive, contentLength, context);
+                }
+                catch (InvalidOperationException)
                 {
                     context.RejectRequest(RequestRejectionReason.InvalidContentLength, unparsedContentLength);
                 }
-                else
+            }
+
+            // Avoid slowing down most common case
+            if (!object.ReferenceEquals(context.Method, HttpMethods.Get))
+            {
+                // If we got here, request contains no Content-Length or Transfer-Encoding header.
+                // Reject with 411 Length Required.
+                if (HttpMethods.IsPost(context.Method) || HttpMethods.IsPut(context.Method))
                 {
-                    return new ForContentLength(keepAlive, contentLength, context);
+                    var requestRejectionReason = httpVersion == HttpVersion.Http11 ? RequestRejectionReason.LengthRequired : RequestRejectionReason.LengthRequiredHttp10;
+                    context.RejectRequest(requestRejectionReason, context.Method);
                 }
             }
 

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs

@@ -27,6 +27,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http
         MissingCRInHeaderLine,
         TooManyHeaders,
         RequestTimeout,
-        FinalTransferCodingNotChunked
+        FinalTransferCodingNotChunked,
+        LengthRequired,
+        LengthRequiredHttp10,
     }
 }

test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestLineSizeTests.cs

@@ -11,19 +11,19 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
     public class MaxRequestLineSizeTests
     {
         [Theory]
-        [InlineData("GET / HTTP/1.1\r\n", 16)]
+        [InlineData("GET / HTTP/1.1\r\n\r\n", 16)]
-        [InlineData("GET / HTTP/1.1\r\n", 17)]
+        [InlineData("GET / HTTP/1.1\r\n\r\n", 17)]
-        [InlineData("GET / HTTP/1.1\r\n", 137)]
+        [InlineData("GET / HTTP/1.1\r\n\r\n", 137)]
-        [InlineData("POST /abc/de HTTP/1.1\r\n", 23)]
+        [InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 23)]
-        [InlineData("POST /abc/de HTTP/1.1\r\n", 24)]
+        [InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 24)]
-        [InlineData("POST /abc/de HTTP/1.1\r\n", 287)]
+        [InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 287)]
-        [InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\n", 28)]
+        [InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 28)]
-        [InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\n", 29)]
+        [InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 29)]
-        [InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\n", 589)]
+        [InlineData("PUT /abc/de?f=ghi HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 589)]
-        [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 40)]
+        [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 40)]
-        [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 41)]
+        [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 41)]
-        [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 1027)]
+        [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 1027)]
-        public async Task ServerAcceptsRequestLineWithinLimit(string requestLine, int limit)
+        public async Task ServerAcceptsRequestLineWithinLimit(string request, int limit)
         {
             var maxRequestLineSize = limit;
 
@@ -31,7 +31,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
             {
                 using (var connection = new TestConnection(server.Port))
                 {
-                    await connection.SendEnd($"{requestLine}\r\n");
+                    await connection.SendEnd(request);
                     await connection.ReceiveEnd(
                         "HTTP/1.1 200 OK",
                         $"Date: {server.Context.DateHeaderValue}",

test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs

@@ -174,6 +174,36 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             }
         }
 
+        [Theory]
+        [InlineData("POST")]
+        [InlineData("PUT")]
+        public async Task BadRequestIfMethodRequiresLengthButNoContentLengthOrTransferEncodingInRequest(string method)
+        {
+            using (var server = new TestServer(context => { return Task.FromResult(0); }))
+            {
+                using (var connection = server.CreateConnection())
+                {
+                    await connection.SendEnd($"{method} / HTTP/1.1\r\n\r\n");
+                    await ReceiveBadRequestResponse(connection, "411 Length Required", server.Context.DateHeaderValue);
+                }
+            }
+        }
+
+        [Theory]
+        [InlineData("POST")]
+        [InlineData("PUT")]
+        public async Task BadRequestIfMethodRequiresLengthButNoContentLengthInHttp10Request(string method)
+        {
+            using (var server = new TestServer(context => { return Task.FromResult(0); }))
+            {
+                using (var connection = server.CreateConnection())
+                {
+                    await connection.SendEnd($"{method} / HTTP/1.0\r\n\r\n");
+                    await ReceiveBadRequestResponse(connection, "400 Bad Request", server.Context.DateHeaderValue);
+                }
+            }
+        }
+
         private async Task ReceiveBadRequestResponse(TestConnection connection, string expectedResponseStatusCode, string expectedDateHeaderValue)
         {
             await connection.ReceiveForcedEnd(

test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs

@@ -196,13 +196,45 @@ namespace Microsoft.AspNetCore.Server.KestrelTests
             using (var input = new TestInput())
             {
                 var ex = Assert.Throws<BadHttpRequestException>(() =>
-                    MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders { HeaderTransferEncoding = "chunked, not-chunked" }, input.FrameContext));
+                    MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders { HeaderTransferEncoding = "chunked, not-chunked" }, input.FrameContext));
 
                 Assert.Equal(400, ex.StatusCode);
                 Assert.Equal("Final transfer coding is not \"chunked\": \"chunked, not-chunked\"", ex.Message);
             }
         }
 
+        [Theory]
+        [InlineData("POST")]
+        [InlineData("PUT")]
+        public void ForThrowsWhenMethodRequiresLengthButNoContentLengthOrTransferEncodingIsSet(string method)
+        {
+            using (var input = new TestInput())
+            {
+                input.FrameContext.Method = method;
+                var ex = Assert.Throws<BadHttpRequestException>(() =>
+                    MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders(), input.FrameContext));
+
+                Assert.Equal(411, ex.StatusCode);
+                Assert.Equal($"{method} request contains no Content-Length or Transfer-Encoding header", ex.Message);
+            }
+        }
+
+        [Theory]
+        [InlineData("POST")]
+        [InlineData("PUT")]
+        public void ForThrowsWhenMethodRequiresLengthButNoContentLengthSetHttp10(string method)
+        {
+            using (var input = new TestInput())
+            {
+                input.FrameContext.Method = method;
+                var ex = Assert.Throws<BadHttpRequestException>(() =>
+                    MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders(), input.FrameContext));
+
+                Assert.Equal(400, ex.StatusCode);
+                Assert.Equal($"{method} request contains no Content-Length header", ex.Message);
+            }
+        }
+
         public static IEnumerable<object[]> StreamData => new[]
         {
             new object[] { new ThrowOnWriteSynchronousStream() },