Ensure status code is 500 if app calls HttpContext.Abort() and then throws an exception

Author: mikeharder

src/Microsoft.AspNetCore.Server.Kestrel/Internal/Http/FrameOfT.cs

@@ -126,12 +126,13 @@ public override async Task RequestProcessingAsync()
                                     await messageBody.Consume();
                                 }
 
-                                // ProduceEnd() must be called before _application.DisposeContext(), to ensure
-                                // HttpContext.Response.StatusCode is correctly set when
-                                // IHttpContextFactory.Dispose(HttpContext) is called.
-                                await ProduceEnd();
                             }
 
+                            // ProduceEnd() must be called before _application.DisposeContext(), to ensure
+                            // HttpContext.Response.StatusCode is correctly set when
+                            // IHttpContextFactory.Dispose(HttpContext) is called.
+                            await ProduceEnd();
+
                             _application.DisposeContext(context, _applicationException);
                         }
 

test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/ResponseTests.cs

@@ -15,6 +15,7 @@
 using Microsoft.AspNetCore.Server.Kestrel.Internal.Infrastructure;
 using Microsoft.AspNetCore.Testing;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Internal;
 using Microsoft.Extensions.Primitives;
 using Moq;
 using Xunit;
@@ -86,7 +87,7 @@ public async Task IgnoreNullHeaderValues(string headerName, StringValues headerV
                     app.Run(async context =>
                     {
                         context.Response.Headers.Add(headerName, headerValue);
-                        
+
                         await context.Response.WriteAsync("");
                     });
                 });
@@ -176,35 +177,73 @@ 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()
+        public Task ResponseStatusCodeSetBeforeHttpContextDisposeAppException()
+        {
+            return ResponseStatusCodeSetBeforeHttpContextDispose(
+                context =>
+                {
+                    throw new Exception();
+                },
+                expectedClientStatusCode: HttpStatusCode.InternalServerError,
+                expectedServerStatusCode: HttpStatusCode.InternalServerError);
+        }
+
+        [Fact]
+        public Task ResponseStatusCodeSetBeforeHttpContextDisposeRequestAborted()
+        {
+            return ResponseStatusCodeSetBeforeHttpContextDispose(
+                context =>
+                {
+                    context.Abort();
+                    return TaskCache.CompletedTask;
+                },
+                expectedClientStatusCode: null,
+                expectedServerStatusCode: HttpStatusCode.OK);
+        }
+
+        [Fact]
+        public Task ResponseStatusCodeSetBeforeHttpContextDisposeRequestAbortedAppException()
+        {
+            return ResponseStatusCodeSetBeforeHttpContextDispose(
+                context =>
+                {
+                    context.Abort();
+                    throw new Exception();
+                },
+                expectedClientStatusCode: null,
+                expectedServerStatusCode: HttpStatusCode.InternalServerError);
+        }
+
+        private static async Task ResponseStatusCodeSetBeforeHttpContextDispose(RequestDelegate handler,
+            HttpStatusCode? expectedClientStatusCode, HttpStatusCode expectedServerStatusCode)
         {
             var mockHttpContextFactory = new Mock<IHttpContextFactory>();
             mockHttpContextFactory.Setup(f => f.Create(It.IsAny<IFeatureCollection>()))
                 .Returns<IFeatureCollection>(fc => new DefaultHttpContext(fc));
 
-            int disposedStatusCode = -1;
+            var disposedTcs = new TaskCompletionSource<int>();
             mockHttpContextFactory.Setup(f => f.Dispose(It.IsAny<HttpContext>()))
-                .Callback<HttpContext>(c => disposedStatusCode = c.Response.StatusCode);
+                .Callback<HttpContext>(c =>
+                {
+                    disposedTcs.TrySetResult(c.Response.StatusCode);
+                });
 
             var hostBuilder = new WebHostBuilder()
                 .UseKestrel()
                 .UseUrls("http://127.0.0.1:0")
                 .ConfigureServices(services => services.AddSingleton<IHttpContextFactory>(mockHttpContextFactory.Object))
                 .Configure(app =>
                 {
-                    app.Run(context =>
-                    {
-                        throw new Exception();
-                    });
+                    app.Run(handler);
                 });
 
             using (var host = hostBuilder.Build())
@@ -213,12 +252,24 @@ public async Task ResponseStatusCodeSetBeforeHttpContextDispose()
 
                 using (var client = new HttpClient())
                 {
-                    var response = await client.GetAsync($"http://localhost:{host.GetPort()}/");
-                    Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+                    try
+                    {
+                        var response = await client.GetAsync($"http://localhost:{host.GetPort()}/");
+                        Assert.Equal(expectedClientStatusCode, response.StatusCode);
+                    }
+                    catch
+                    {
+                        if (expectedClientStatusCode != null)
+                        {
+                            throw;
+                        }
+                    }
                 }
-            }
 
-            Assert.Equal(HttpStatusCode.InternalServerError, (HttpStatusCode)disposedStatusCode);
+                var disposedStatusCode = await disposedTcs.Task.TimeoutAfter(TimeSpan.FromSeconds(10));
+
+                Assert.Equal(expectedServerStatusCode, (HttpStatusCode)disposedStatusCode);
+            }
         }
 
         // https://github.com/aspnet/KestrelHttpServer/pull/1111/files#r80584475 explains the reason for this test.