From 89a63d190edd69f6ae63123c6aeb2ad27cb92ef7 Mon Sep 17 00:00:00 2001 From: Cesar Blum Silveira Date: Thu, 12 Jan 2017 12:38:15 -0800 Subject: [PATCH] Don't close connection when Content-Length set but no bytes written. --- .../Internal/Http/Frame.cs | 8 ++++++- .../ResponseTests.cs | 20 +++++++++------- .../EngineTests.cs | 24 ++++++++----------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs index d829c68c18..66868595f2 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/Frame.cs @@ -695,7 +695,13 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Internal.Http responseHeaders.HeaderContentLengthValue.HasValue && _responseBytesWritten < responseHeaders.HeaderContentLengthValue.Value) { - _keepAlive = false; + // We need to close the connection if any bytes were written since the client + // cannot be certain of how many bytes it will receive. + if (_responseBytesWritten > 0) + { + _keepAlive = false; + } + ReportApplicationError(new InvalidOperationException( $"Response Content-Length mismatch: too few bytes written ({_responseBytesWritten} of {responseHeaders.HeaderContentLengthValue.Value}).")); } diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs index c84b3a4663..edf5d39703 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs @@ -668,7 +668,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } [Fact] - public async Task WhenAppSetsContentLengthButDoesNotWriteBody500ResponseSentAndConnectionCloses() + public async Task WhenAppSetsContentLengthButDoesNotWriteBody500ResponseSentAndConnectionDoesNotClose() { var testLogger = new TestApplicationErrorLogger(); var serviceContext = new TestServiceContext { Log = new TestKestrelTrace(testLogger) }; @@ -682,12 +682,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests using (var connection = server.CreateConnection()) { await connection.Send( + "GET / HTTP/1.1", + "", "GET / HTTP/1.1", "", ""); - await connection.ReceiveForcedEnd( - $"HTTP/1.1 500 Internal Server Error", - "Connection: close", + await connection.Receive( + "HTTP/1.1 500 Internal Server Error", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + "HTTP/1.1 500 Internal Server Error", $"Date: {server.Context.DateHeaderValue}", "Content-Length: 0", "", @@ -695,10 +700,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests } } - var errorMessage = Assert.Single(testLogger.Messages, message => message.LogLevel == LogLevel.Error); - Assert.Equal( - $"Response Content-Length mismatch: too few bytes written (0 of 5).", - errorMessage.Exception.Message); + var error = testLogger.Messages.Where(message => message.LogLevel == LogLevel.Error); + Assert.Equal(2, error.Count()); + Assert.All(error, message => message.Equals("Response Content-Length mismatch: too few bytes written (0 of 5).")); } [Theory] diff --git a/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs b/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs index f4965d908f..f138f063ba 100644 --- a/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs +++ b/test/Microsoft.AspNetCore.Server.KestrelTests/EngineTests.cs @@ -853,12 +853,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests [MemberData(nameof(ConnectionAdapterData))] public async Task ThrowingInOnStartingResultsInFailedWritesAnd500Response(ListenOptions listenOptions) { + var callback1Called = false; + var callback2CallCount = 0; + var testContext = new TestServiceContext(); - - var onStartingCallCount1 = 0; - var onStartingCallCount2 = 0; - var failedWriteCount = 0; - var testLogger = new TestApplicationErrorLogger(); testContext.Log = new KestrelTrace(testLogger); @@ -869,19 +867,17 @@ namespace Microsoft.AspNetCore.Server.KestrelTests var response = httpContext.Response; response.OnStarting(_ => { - onStartingCallCount1++; + callback1Called = true; throw onStartingException; }, null); response.OnStarting(_ => { - onStartingCallCount2++; + callback2CallCount++; throw onStartingException; }, null); var writeException = await Assert.ThrowsAsync(async () => await response.Body.FlushAsync()); Assert.Same(onStartingException, writeException.InnerException); - - failedWriteCount++; }, testContext, listenOptions)) { using (var connection = server.CreateConnection()) @@ -892,7 +888,7 @@ namespace Microsoft.AspNetCore.Server.KestrelTests "GET / HTTP/1.1", "", ""); - await connection.Receive( + await connection.ReceiveEnd( "HTTP/1.1 500 Internal Server Error", $"Date: {testContext.DateHeaderValue}", "Content-Length: 0", @@ -905,10 +901,10 @@ namespace Microsoft.AspNetCore.Server.KestrelTests } } - // The first registered OnStarting callback should not be called, - // since they are called LIFO and the other one failed. - Assert.Equal(0, onStartingCallCount1); - Assert.Equal(2, onStartingCallCount2); + // The first registered OnStarting callback should have been called, + // since they are called LIFO order and the other one failed. + Assert.False(callback1Called); + Assert.Equal(2, callback2CallCount); Assert.Equal(2, testLogger.ApplicationErrorsLogged); }