diff --git a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs index eeba9695d..a837c7d0e 100644 --- a/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs +++ b/src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs @@ -115,21 +115,24 @@ public override async Task RequestProcessingAsync() await FireOnCompleted(); } - _application.DisposeContext(context, _applicationException); - } - - // If _requestAbort is set, the connection has already been closed. - if (Volatile.Read(ref _requestAborted) == 0) - { - ResumeStreams(); - - if (_keepAlive) + // If _requestAbort is set, the connection has already been closed. + if (Volatile.Read(ref _requestAborted) == 0) { - // Finish reading the request body in case the app did not. - await messageBody.Consume(); + ResumeStreams(); + + if (_keepAlive) + { + // Finish reading the request body in case the app did not. + await messageBody.Consume(); + } + + // ProduceEnd() must be called before _application.DisposeContext(), to ensure + // HttpContext.Response.StatusCode is correct set when + // IHttpContextFactory.Dispose(HttpContext) is called. + await ProduceEnd(); } - await ProduceEnd(); + _application.DisposeContext(context, _applicationException); } StopStreams(); diff --git a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs index bcd2e7ecf..399a63e61 100644 --- a/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs +++ b/test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs @@ -10,10 +10,14 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Internal.Http; using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure; using Microsoft.AspNetCore.Testing; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Internal; +using Microsoft.Extensions.ObjectPool; +using Microsoft.Extensions.Options; using Microsoft.Extensions.Primitives; using Moq; using Xunit; @@ -175,13 +179,50 @@ public async Task OnStartingThrowsWhenSetAfterResponseHasAlreadyStarted() using (var client = new HttpClient()) { var response = await client.GetAsync($"http://localhost:{host.GetPort()}/"); - + // Despite the error, the response had already started Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(ex); } } } + + [Fact] + public async Task ResponseStatusCodeSetBeforeHttpContextDispose() + { + var mockHttpContextFactory = new Mock(); + mockHttpContextFactory.Setup(f => f.Create(It.IsAny())) + .Returns(fc => new DefaultHttpContext(fc)); + + int disposedStatusCode = -1; + mockHttpContextFactory.Setup(f => f.Dispose(It.IsAny())) + .Callback(c => disposedStatusCode = c.Response.StatusCode); + + var hostBuilder = new WebHostBuilder() + .UseKestrel() + .UseUrls("http://127.0.0.1:0") + .ConfigureServices(services => services.AddSingleton(mockHttpContextFactory.Object)) + .Configure(app => + { + app.Run(context => + { + throw new Exception(); + }); + }); + + using (var host = hostBuilder.Build()) + { + host.Start(); + + using (var client = new HttpClient()) + { + var response = await client.GetAsync($"http://localhost:{host.GetPort()}/"); + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + } + } + + Assert.Equal(HttpStatusCode.InternalServerError, (HttpStatusCode)disposedStatusCode); + } // https://github.com/aspnet/KestrelHttpServer/pull/1111/files#r80584475 explains the reason for this test. [Fact]