Skip to content

Commit 1f0641f

Browse files
author
N. Taylor Mullen
committed
Reset endpoint and route values during exception handling.
- We initially did this change as part of EndpointRouting but the impact of that change resulted in a variety of performance regressions. To mitigate the impact of resetting state for a request we now only reset the state when an exception has occurred in a way that does not require any additional state machines to be allocated. - Added a test to validate that http context state gets reset on exception handling. #12897
1 parent 4f60223 commit 1f0641f

File tree

2 files changed

+101
-1
lines changed

2 files changed

+101
-1
lines changed

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddleware.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading.Tasks;
88
using Microsoft.AspNetCore.Builder;
99
using Microsoft.AspNetCore.Http;
10+
using Microsoft.AspNetCore.Http.Features;
1011
using Microsoft.Extensions.Logging;
1112
using Microsoft.Extensions.Options;
1213
using Microsoft.Net.Http.Headers;
@@ -103,7 +104,8 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
103104
}
104105
try
105106
{
106-
context.Response.Clear();
107+
ClearHttpContext(context);
108+
107109
var exceptionHandlerFeature = new ExceptionHandlerFeature()
108110
{
109111
Error = edi.SourceException,
@@ -137,6 +139,17 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
137139
edi.Throw(); // Re-throw the original if we couldn't handle it
138140
}
139141

142+
private static void ClearHttpContext(HttpContext context)
143+
{
144+
context.Response.Clear();
145+
146+
// An endpoint may have already been set. Since we're going to re-invoke the middleware pipeline we need to reset
147+
// the endpoint and route values to ensure things are re-calculated.
148+
context.SetEndpoint(endpoint: null);
149+
var routeValuesFeature = context.Features.Get<IRouteValuesFeature>();
150+
routeValuesFeature?.RouteValues?.Clear();
151+
}
152+
140153
private static Task ClearCacheHeaders(object state)
141154
{
142155
var headers = ((HttpResponse)state).Headers;
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Diagnostics;
6+
using System.Threading.Tasks;
7+
using Microsoft.AspNetCore.Builder;
8+
using Microsoft.AspNetCore.Http;
9+
using Microsoft.Extensions.Logging.Abstractions;
10+
using Microsoft.Extensions.Options;
11+
using Moq;
12+
using Xunit;
13+
14+
namespace Microsoft.AspNetCore.Diagnostics
15+
{
16+
public class ExceptionHandlerMiddlewareTest
17+
{
18+
[Fact]
19+
public async Task Invoke_ExceptionThrownResultsInClearedRouteValuesAndEndpoint()
20+
{
21+
// Arrange
22+
var httpContext = CreateHttpContext();
23+
httpContext.SetEndpoint(new Endpoint((_) => Task.CompletedTask, new EndpointMetadataCollection(), "Test"));
24+
httpContext.Request.RouteValues["John"] = "Doe";
25+
26+
var optionsAccessor = CreateOptionsAccessor(
27+
exceptionHandler: context =>
28+
{
29+
Assert.Empty(context.Request.RouteValues);
30+
Assert.Null(context.GetEndpoint());
31+
return Task.CompletedTask;
32+
});
33+
var middleware = CreateMiddleware(_ => throw new InvalidOperationException(), optionsAccessor);
34+
35+
// Act & Assert
36+
await middleware.Invoke(httpContext);
37+
}
38+
39+
private HttpContext CreateHttpContext()
40+
{
41+
var httpContext = new DefaultHttpContext
42+
{
43+
RequestServices = new TestServiceProvider()
44+
};
45+
46+
return httpContext;
47+
}
48+
49+
private IOptions<ExceptionHandlerOptions> CreateOptionsAccessor(
50+
RequestDelegate exceptionHandler = null,
51+
string exceptionHandlingPath = null)
52+
{
53+
exceptionHandler ??= c => Task.CompletedTask;
54+
var options = new ExceptionHandlerOptions()
55+
{
56+
ExceptionHandler = exceptionHandler,
57+
ExceptionHandlingPath = exceptionHandlingPath,
58+
};
59+
var optionsAccessor = Mock.Of<IOptions<ExceptionHandlerOptions>>(o => o.Value == options);
60+
return optionsAccessor;
61+
}
62+
63+
private ExceptionHandlerMiddleware CreateMiddleware(
64+
RequestDelegate next,
65+
IOptions<ExceptionHandlerOptions> options)
66+
{
67+
next ??= c => Task.CompletedTask;
68+
var listener = new DiagnosticListener("Microsoft.AspNetCore");
69+
70+
var middleware = new ExceptionHandlerMiddleware(
71+
next,
72+
NullLoggerFactory.Instance,
73+
options,
74+
listener);
75+
76+
return middleware;
77+
}
78+
79+
private class TestServiceProvider : IServiceProvider
80+
{
81+
public object GetService(Type serviceType)
82+
{
83+
throw new NotImplementedException();
84+
}
85+
}
86+
}
87+
}

0 commit comments

Comments
 (0)