diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs b/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs index 2b4aa25f12..cb37af76c1 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/BadHttpRequestException.cs +++ b/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; diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs index c2843a2d00..5fb8fa2c3a 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs +++ b/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(); - if (unparsedContentLength.Length > 0) + var unparsedContentLength = headers.HeaderContentLength; + if (unparsedContentLength.Count > 0) { - long contentLength; - if (!long.TryParse(unparsedContentLength, out contentLength) || contentLength < 0) + try + { + 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); } } diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs index 68693304e0..0c05a6b8ea 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/RequestRejectionReason.cs +++ b/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, } } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestLineSizeTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestLineSizeTests.cs index 35b66ed5b5..1118f28d07 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestLineSizeTests.cs +++ b/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", 17)] - [InlineData("GET / HTTP/1.1\r\n", 137)] - [InlineData("POST /abc/de HTTP/1.1\r\n", 23)] - [InlineData("POST /abc/de HTTP/1.1\r\n", 24)] - [InlineData("POST /abc/de HTTP/1.1\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\n", 29)] - [InlineData("PUT /abc/de?f=ghi HTTP/1.1\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", 41)] - [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n", 1027)] - public async Task ServerAcceptsRequestLineWithinLimit(string requestLine, int limit) + [InlineData("GET / HTTP/1.1\r\n\r\n", 16)] + [InlineData("GET / HTTP/1.1\r\n\r\n", 17)] + [InlineData("GET / HTTP/1.1\r\n\r\n", 137)] + [InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 23)] + [InlineData("POST /abc/de HTTP/1.1\r\nContent-Length: 0\r\n\r\n", 24)] + [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\nContent-Length: 0\r\n\r\n", 28)] + [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\nContent-Length: 0\r\n\r\n", 589)] + [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\r\n", 41)] + [InlineData("DELETE /a%20b%20c/d%20e?f=ghi HTTP/1.1\r\n\r\n", 1027)] + 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}", diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs index a4e0f11169..7b8b6a0114 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/BadHttpRequestTests.cs +++ b/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( diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs index 4ada15b2c5..a5ed8d7c9f 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs +++ b/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(() => - 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(() => + 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(() => + 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 StreamData => new[] { new object[] { new ThrowOnWriteSynchronousStream() },