From 73656f6503688b6eb47ed7a5e93b3db76cbb2280 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Tue, 27 Sep 2016 16:42:13 -0700 Subject: [PATCH] Assume zero length on non-keepalive requests without Content-Length or Transfer-Encoding (#1104). --- .../Internal/Http/MessageBody.cs | 12 +- .../MaxRequestBufferSizeTests.cs | 14 +-- .../ChunkedRequestTests.cs | 1 + .../ConnectionFilterTests.cs | 4 +- .../EngineTests.cs | 23 ++-- .../MessageBodyTests.cs | 104 +++++++++++++++++- 6 files changed, 129 insertions(+), 29 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs index 784f8d97de..bff4ba622f 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/MessageBody.cs @@ -234,6 +234,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http var connection = headers.HeaderConnection.ToString(); if (connection.Length > 0) { + if (connection.Equals("upgrade", StringComparison.OrdinalIgnoreCase)) + { + return new ForRemainingData(context); + } + keepAlive = connection.Equals("keep-alive", StringComparison.OrdinalIgnoreCase); } @@ -257,12 +262,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http } } - if (keepAlive) - { - return new ForContentLength(true, 0, context); - } - - return new ForRemainingData(context); + return new ForContentLength(keepAlive, 0, context); } private class ForRemainingData : MessageBody diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs index 463a11145c..d78f60a7cd 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/MaxRequestBufferSizeTests.cs @@ -53,15 +53,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests // Disables all code related to computing and limiting the size of the input buffer. Tuple.Create((long?)null, false) }; - var sendContentLengthHeaderValues = new[] { true, false }; var sslValues = new[] { true, false }; return from maxRequestBufferSize in maxRequestBufferSizeValues - from sendContentLengthHeader in sendContentLengthHeaderValues from ssl in sslValues select new object[] { maxRequestBufferSize.Item1, - sendContentLengthHeader, ssl, maxRequestBufferSize.Item2 }; @@ -70,7 +67,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests [Theory] [MemberData("LargeUploadData")] - public async Task LargeUpload(long? maxRequestBufferSize, bool sendContentLengthHeader, bool ssl, bool expectPause) + public async Task LargeUpload(long? maxRequestBufferSize, bool ssl, bool expectPause) { // Parameters var data = new byte[_dataLength]; @@ -91,7 +88,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var socket = CreateSocket(port)) using (var stream = await CreateStreamAsync(socket, ssl, host.GetHost())) { - await WritePostRequestHeaders(stream, sendContentLengthHeader ? (int?)data.Length : null); + await WritePostRequestHeaders(stream, data.Length); var bytesWritten = 0; @@ -236,15 +233,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests return socket; } - private static async Task WritePostRequestHeaders(Stream stream, int? contentLength) + private static async Task WritePostRequestHeaders(Stream stream, int contentLength) { using (var writer = new StreamWriter(stream, Encoding.ASCII, bufferSize: 1024, leaveOpen: true)) { await writer.WriteAsync("POST / HTTP/1.0\r\n"); - if (contentLength.HasValue) - { - await writer.WriteAsync($"Content-Length: {contentLength.Value}\r\n"); - } + await writer.WriteAsync($"Content-Length: {contentLength}\r\n"); await writer.WriteAsync("\r\n"); } } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedRequestTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedRequestTests.cs index 9214b5b1f7..221922a050 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedRequestTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/ChunkedRequestTests.cs @@ -102,6 +102,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "0", "", "POST / HTTP/1.0", + "Content-Length: 7", "", "Goodbye"); await connection.Receive( diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs index d2a3b7cc22..3d1423dc2b 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/ConnectionFilterTests.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var filter = new RewritingConnectionFilter(); var serviceContext = new TestServiceContext(filter); - var sendString = "POST / HTTP/1.0\r\n\r\nHello World?"; + var sendString = "POST / HTTP/1.0\r\nContent-Length: 12\r\n\r\nHello World?"; using (var server = new TestServer(App, serviceContext)) { @@ -66,6 +66,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { await connection.SendEnd( "POST / HTTP/1.0", + "Content-Length: 12", "", "Hello World?"); await connection.ReceiveEnd( @@ -91,6 +92,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { await connection.SendEnd( "POST / HTTP/1.0", + "Content-Length: 12", "", "Hello World?"); } diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs index 2fdb63a220..21ecb56a19 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs @@ -14,7 +14,6 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel; using Microsoft.AspNetCore.Server.Kestrel.Internal; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; -using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Internal; using Xunit; @@ -108,7 +107,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var started = engine.CreateServer(address); var socket = TestConnection.CreateConnectedLoopbackSocket(address.Port); - socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.0\r\n\r\nHello World")); + socket.Send(Encoding.ASCII.GetBytes("POST / HTTP/1.0\r\nContent-Length: 11\r\n\r\nHello World")); socket.Shutdown(SocketShutdown.Send); var buffer = new byte[8192]; while (true) @@ -131,6 +130,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { await connection.SendEnd( "POST / HTTP/1.0", + "Content-Length: 11", "", "Hello World"); await connection.ReceiveEnd( @@ -156,6 +156,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "", "GET / HTTP/1.1", "Connection: close", + "Content-Length: 7", "", "Goodbye"); await connection.ReceiveEnd( @@ -219,7 +220,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { var requestData = Enumerable.Repeat("GET / HTTP/1.1\r\n", loopCount) - .Concat(new[] { "GET / HTTP/1.1\r\nConnection: close\r\n\r\nGoodbye" }); + .Concat(new[] { "GET / HTTP/1.1\r\nContent-Length: 7\r\nConnection: close\r\n\r\nGoodbye" }); var response = string.Join("\r\n", new string[] { "HTTP/1.1 200 OK", @@ -288,6 +289,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "Connection: keep-alive", "", "POST / HTTP/1.0", + "Content-Length: 7", "", "Goodbye"); await connection.Receive( @@ -320,8 +322,8 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "Connection: keep-alive", "", "POST / HTTP/1.0", - "Content-Length: 7", "Connection: keep-alive", + "Content-Length: 7", "", "Goodbye"); await connection.Receive( @@ -354,6 +356,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "Connection: keep-alive", "", "Hello WorldPOST / HTTP/1.0", + "Content-Length: 7", "", "Goodbye"); await connection.Receive( @@ -461,11 +464,17 @@ namespace Microsoft.AspNetCore.Server.KestrelTests [MemberData(nameof(ConnectionFilterData))] public async Task ZeroContentLengthSetAutomaticallyForNonKeepAliveRequests(TestServiceContext testContext) { - using (var server = new TestServer(EmptyApp, testContext)) + using (var server = new TestServer(async httpContext => + { + Assert.Equal(0, await httpContext.Request.Body.ReadAsync(new byte[1], 0, 1).TimeoutAfter(TimeSpan.FromSeconds(10))); + }, testContext)) { using (var connection = server.CreateConnection()) { - await connection.SendEnd( + // Use Send instead of SendEnd to ensure the connection will remain open while + // the app runs and reads 0 bytes from the body nonetheless. This checks that + // https://github.com/aspnet/KestrelHttpServer/issues/1104 is not regressing. + await connection.Send( "GET / HTTP/1.1", "Connection: close", "", @@ -481,7 +490,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests using (var connection = server.CreateConnection()) { - await connection.SendEnd( + await connection.Send( "GET / HTTP/1.0", "", ""); diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs index 9dd54a75d0..1790cf8094 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/MessageBodyTests.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { using (var input = new TestInput()) { - var body = MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders(), input.FrameContext); + var body = MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders { HeaderContentLength = "5" }, input.FrameContext); var stream = new FrameRequestStream(); stream.StartAcceptingReads(body); @@ -48,7 +48,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests { using (var input = new TestInput()) { - var body = MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders(), input.FrameContext); + var body = MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders { HeaderContentLength = "5" }, input.FrameContext); var stream = new FrameRequestStream(); stream.StartAcceptingReads(body); @@ -65,7 +65,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } [Fact] - public async Task CanHandleLargeBlocks() + public void Http10NoContentLength() { using (var input = new TestInput()) { @@ -73,6 +73,102 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var stream = new FrameRequestStream(); stream.StartAcceptingReads(body); + input.Add("Hello", true); + + var buffer1 = new byte[1024]; + Assert.Equal(0, stream.Read(buffer1, 0, 1024)); + } + } + + [Fact] + public async Task Http10NoContentLengthAsync() + { + using (var input = new TestInput()) + { + var body = MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders(), input.FrameContext); + var stream = new FrameRequestStream(); + stream.StartAcceptingReads(body); + + input.Add("Hello", true); + + var buffer1 = new byte[1024]; + Assert.Equal(0, await stream.ReadAsync(buffer1, 0, 1024)); + } + } + + [Fact] + public void Http11NoContentLength() + { + using (var input = new TestInput()) + { + var body = MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders(), input.FrameContext); + var stream = new FrameRequestStream(); + stream.StartAcceptingReads(body); + + input.Add("Hello", true); + + var buffer1 = new byte[1024]; + Assert.Equal(0, stream.Read(buffer1, 0, 1024)); + } + } + + [Fact] + public async Task Http11NoContentLengthAsync() + { + using (var input = new TestInput()) + { + var body = MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders(), input.FrameContext); + var stream = new FrameRequestStream(); + stream.StartAcceptingReads(body); + + input.Add("Hello", true); + + var buffer1 = new byte[1024]; + Assert.Equal(0, await stream.ReadAsync(buffer1, 0, 1024)); + } + } + + [Fact] + public void Http11NoContentLengthConnectionClose() + { + using (var input = new TestInput()) + { + var body = MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders { HeaderConnection = "close" }, input.FrameContext); + var stream = new FrameRequestStream(); + stream.StartAcceptingReads(body); + + input.Add("Hello", true); + + var buffer1 = new byte[1024]; + Assert.Equal(0, stream.Read(buffer1, 0, 1024)); + } + } + + [Fact] + public async Task Http11NoContentLengthConnectionCloseAsync() + { + using (var input = new TestInput()) + { + var body = MessageBody.For(HttpVersion.Http11, new FrameRequestHeaders { HeaderConnection = "close" }, input.FrameContext); + var stream = new FrameRequestStream(); + stream.StartAcceptingReads(body); + + input.Add("Hello", true); + + var buffer1 = new byte[1024]; + Assert.Equal(0, await stream.ReadAsync(buffer1, 0, 1024)); + } + } + + [Fact] + public async Task CanHandleLargeBlocks() + { + using (var input = new TestInput()) + { + var body = MessageBody.For(HttpVersion.Http10, new FrameRequestHeaders { HeaderContentLength = "8197" }, input.FrameContext); + var stream = new FrameRequestStream(); + stream.StartAcceptingReads(body); + // Input needs to be greater than 4032 bytes to allocate a block not backed by a slab. var largeInput = new string('a', 8192); @@ -101,8 +197,6 @@ namespace Microsoft.AspNetCore.Server.KestrelTests public static IEnumerable RequestData => new[] { - // Remaining Data - new object[] { new FrameRequestHeaders { HeaderConnection = "close" }, new[] { "Hello ", "World!" } }, // Content-Length new object[] { new FrameRequestHeaders { HeaderContentLength = "12" }, new[] { "Hello ", "World!" } }, // Chunked