diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 1f518201f3380d0256ffd323dd5ce6081a45b45a..1a274cbc0d75aa2177654adc4df80b42b6b266e9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // Keep-alive is default for HTTP/1.1 and HTTP/2; parsing and errors will change its value // volatile, see: https://msdn.microsoft.com/en-us/library/x13ttww7.aspx protected volatile bool _keepAlive = true; - private bool _canHaveBody; + private bool _canWriteResponseBody; private bool _autoChunk; protected Exception _applicationException; private BadHttpRequestException _requestRejectedException; @@ -828,7 +828,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http VerifyAndUpdateWrite(data.Length); } - if (_canHaveBody) + if (_canWriteResponseBody) { if (_autoChunk) { @@ -857,7 +857,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http // WriteAsyncAwaited is only called for the first write to the body. // Ensure headers are flushed if Write(Chunked)Async isn't called. - if (_canHaveBody) + if (_canWriteResponseBody) { if (_autoChunk) { @@ -1140,48 +1140,47 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http } // Set whether response can have body - _canHaveBody = StatusCanHaveBody(StatusCode) && Method != HttpMethod.Head; + _canWriteResponseBody = CanWriteResponseBody(); - // Don't set the Content-Length or Transfer-Encoding headers - // automatically for HEAD requests or 204, 205, 304 responses. - if (_canHaveBody) + if (!_canWriteResponseBody && hasTransferEncoding) { - if (!hasTransferEncoding && !responseHeaders.ContentLength.HasValue) + RejectNonBodyTransferEncodingResponse(appCompleted); + } + else if (!hasTransferEncoding && !responseHeaders.ContentLength.HasValue) + { + if (StatusCode == StatusCodes.Status101SwitchingProtocols) + { + _keepAlive = false; + } + else if (appCompleted || !_canWriteResponseBody) { - if (appCompleted && StatusCode != StatusCodes.Status101SwitchingProtocols) + // Don't set the Content-Length header automatically for HEAD requests, 204 responses, or 304 responses. + if (CanAutoSetContentLengthZeroResponseHeader()) { - // Since the app has completed and we are only now generating - // the headers we can safely set the Content-Length to 0. + // Since the app has completed writing or cannot write to the response, we can safely set the Content-Length to 0. responseHeaders.ContentLength = 0; } - else - { - // Note for future reference: never change this to set _autoChunk to true on HTTP/1.0 - // connections, even if we were to infer the client supports it because an HTTP/1.0 request - // was received that used chunked encoding. Sending a chunked response to an HTTP/1.0 - // client would break compliance with RFC 7230 (section 3.3.1): - // - // A server MUST NOT send a response containing Transfer-Encoding unless the corresponding - // request indicates HTTP/1.1 (or later). - // - // This also covers HTTP/2, which forbids chunked encoding in RFC 7540 (section 8.1: - // - // The chunked transfer encoding defined in Section 4.1 of [RFC7230] MUST NOT be used in HTTP/2. - if (_httpVersion == Http.HttpVersion.Http11 && StatusCode != StatusCodes.Status101SwitchingProtocols) - { - _autoChunk = true; - responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked); - } - else - { - _keepAlive = false; - } - } } - } - else if (hasTransferEncoding) - { - RejectNonBodyTransferEncodingResponse(appCompleted); + // Note for future reference: never change this to set _autoChunk to true on HTTP/1.0 + // connections, even if we were to infer the client supports it because an HTTP/1.0 request + // was received that used chunked encoding. Sending a chunked response to an HTTP/1.0 + // client would break compliance with RFC 7230 (section 3.3.1): + // + // A server MUST NOT send a response containing Transfer-Encoding unless the corresponding + // request indicates HTTP/1.1 (or later). + // + // This also covers HTTP/2, which forbids chunked encoding in RFC 7540 (section 8.1: + // + // The chunked transfer encoding defined in Section 4.1 of [RFC7230] MUST NOT be used in HTTP/2. + else if (_httpVersion == Http.HttpVersion.Http11) + { + _autoChunk = true; + responseHeaders.SetRawTransferEncoding("chunked", _bytesTransferEncodingChunked); + } + else + { + _keepAlive = false; + } } responseHeaders.SetReadOnly(); @@ -1212,12 +1211,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders); } - public bool StatusCanHaveBody(int statusCode) + private bool CanWriteResponseBody() { // List of status codes taken from Microsoft.Net.Http.Server.Response - return statusCode != StatusCodes.Status204NoContent && - statusCode != StatusCodes.Status205ResetContent && - statusCode != StatusCodes.Status304NotModified; + return Method != HttpMethod.Head && + StatusCode != StatusCodes.Status204NoContent && + StatusCode != StatusCodes.Status205ResetContent && + StatusCode != StatusCodes.Status304NotModified; + } + + private bool CanAutoSetContentLengthZeroResponseHeader() + { + return Method != HttpMethod.Head && + StatusCode != StatusCodes.Status204NoContent && + StatusCode != StatusCodes.Status304NotModified; } private static void ThrowResponseAlreadyStartedException(string value) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 24e1287b73f47d0741e3daaf300d0fd434f4bb73..7ced52b531cc39ce785ad060a4a772e42658dbac 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -477,7 +477,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests [Theory] [InlineData(StatusCodes.Status204NoContent)] - [InlineData(StatusCodes.Status205ResetContent)] [InlineData(StatusCodes.Status304NotModified)] public async Task TransferEncodingChunkedNotSetOnNonBodyResponse(int statusCode) { @@ -504,6 +503,124 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests } } + [Fact] + public async Task ContentLengthZeroSetOn205Response() + { + using (var server = new TestServer(httpContext => + { + httpContext.Response.StatusCode = 205; + return Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + $"HTTP/1.1 205 Reset Content", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Theory] + [InlineData(StatusCodes.Status204NoContent)] + [InlineData(StatusCodes.Status304NotModified)] + public async Task AttemptingToWriteFailsForNonBodyResponse(int statusCode) + { + var responseWriteTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously); + + using (var server = new TestServer(async httpContext => + { + httpContext.Response.StatusCode = statusCode; + + try + { + await httpContext.Response.WriteAsync("hello, world"); + } + catch (Exception ex) + { + responseWriteTcs.TrySetException(ex); + throw; + } + + responseWriteTcs.TrySetResult("This should not be reached."); + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + + + var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => responseWriteTcs.Task).DefaultTimeout(); + Assert.Equal(CoreStrings.FormatWritingToResponseBodyNotSupported(statusCode), ex.Message); + + await connection.Receive( + $"HTTP/1.1 {Encoding.ASCII.GetString(ReasonPhrases.ToStatusBytes(statusCode))}", + $"Date: {server.Context.DateHeaderValue}", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task AttemptingToWriteFailsFor205Response() + { + var responseWriteTcs = new TaskCompletionSource<object>(TaskCreationOptions.RunContinuationsAsynchronously); + + using (var server = new TestServer(async httpContext => + { + httpContext.Response.StatusCode = 205; + + try + { + await httpContext.Response.WriteAsync("hello, world"); + } + catch (Exception ex) + { + responseWriteTcs.TrySetException(ex); + throw; + } + + responseWriteTcs.TrySetResult("This should not be reached."); + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + + + var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => responseWriteTcs.Task).DefaultTimeout(); + Assert.Equal(CoreStrings.FormatWritingToResponseBodyNotSupported(205), ex.Message); + + await connection.Receive( + $"HTTP/1.1 205 Reset Content", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + [Fact] public async Task TransferEncodingNotSetOnHeadResponse() { @@ -1850,10 +1967,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests "Host:", "Content-Length: 3", "", - "205POST / HTTP/1.1", - "Host:", - "Content-Length: 3", - "", "304POST / HTTP/1.1", "Host:", "Content-Length: 3", @@ -1863,9 +1976,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests "HTTP/1.1 204 No Content", $"Date: {testContext.DateHeaderValue}", "", - "HTTP/1.1 205 Reset Content", - $"Date: {testContext.DateHeaderValue}", - "", "HTTP/1.1 304 Not Modified", $"Date: {testContext.DateHeaderValue}", "",