Skip to content

Commit 9a6881b

Browse files
author
N. Taylor Mullen
committed
Fix endpoint routing statefulness.
- In the case that other middleware change the path of an `HttpContext` and cause middleware to re-invoke we used to short-circuit on second time through the middleware pipeline, now we allow routing to occur. - Added unit tests to validate the clearing of state. #11233
1 parent ec81582 commit 9a6881b

File tree

2 files changed

+42
-6
lines changed

2 files changed

+42
-6
lines changed

src/Http/Routing/src/EndpointRoutingMiddleware.cs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public Task Invoke(HttpContext httpContext)
5151
if (endpoint != null)
5252
{
5353
Log.MatchSkipped(_logger, endpoint);
54+
55+
// Someone else set the endpoint, we'll let them handle the unsetting.
5456
return _next(httpContext);
5557
}
5658

@@ -87,7 +89,7 @@ static async Task AwaitMatch(EndpointRoutingMiddleware middleware, HttpContext h
8789
}
8890

8991
[MethodImpl(MethodImplOptions.AggressiveInlining)]
90-
private Task SetRoutingAndContinue(HttpContext httpContext)
92+
private async Task SetRoutingAndContinue(HttpContext httpContext)
9193
{
9294
// If there was no mutation of the endpoint then log failure
9395
var endpoint = httpContext.GetEndpoint();
@@ -107,7 +109,16 @@ private Task SetRoutingAndContinue(HttpContext httpContext)
107109
Log.MatchSuccess(_logger, endpoint);
108110
}
109111

110-
return _next(httpContext);
112+
try
113+
{
114+
await _next(httpContext);
115+
}
116+
finally
117+
{
118+
// We unset the endpoint after calling through to the next middleware. This enables any future calls into
119+
// endpoint routing don't no-op from there already being an endpoint set.
120+
httpContext.SetEndpoint(endpoint: null);
121+
}
111122
}
112123

113124
// Initialization is async to avoid blocking threads while reflection and things

src/Http/Routing/test/UnitTests/EndpointRoutingMiddlewareTest.cs

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,30 @@ namespace Microsoft.AspNetCore.Routing
2121
public class EndpointRoutingMiddlewareTest
2222
{
2323
[Fact]
24-
public async Task Invoke_OnCall_SetsEndpointFeature()
24+
public async Task Invoke_OnException_ResetsEndpoint()
25+
{
26+
// Arrange
27+
var httpContext = CreateHttpContext();
28+
29+
var middleware = CreateMiddleware(next: context => throw new Exception());
30+
31+
// Act
32+
try
33+
{
34+
await middleware.Invoke(httpContext);
35+
}
36+
catch
37+
{
38+
// Do nothing, we expect the test to throw.
39+
}
40+
41+
// Assert
42+
var endpoint = httpContext.GetEndpoint();
43+
Assert.Null(endpoint);
44+
}
45+
46+
[Fact]
47+
public async Task Invoke_OnCall_SetsEndpointFeatureAndResetsEndpoint()
2548
{
2649
// Arrange
2750
var httpContext = CreateHttpContext();
@@ -34,14 +57,16 @@ public async Task Invoke_OnCall_SetsEndpointFeature()
3457
// Assert
3558
var endpointFeature = httpContext.Features.Get<IEndpointFeature>();
3659
Assert.NotNull(endpointFeature);
60+
Assert.Null(endpointFeature.Endpoint);
3761
}
3862

3963
[Fact]
40-
public async Task Invoke_SkipsRouting_IfEndpointSet()
64+
public async Task Invoke_SkipsRoutingAndMaintainsEndpoint_IfEndpointSet()
4165
{
4266
// Arrange
4367
var httpContext = CreateHttpContext();
44-
httpContext.SetEndpoint(new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(), "myapp"));
68+
var expectedEndpoint = new Endpoint(c => Task.CompletedTask, new EndpointMetadataCollection(), "myapp");
69+
httpContext.SetEndpoint(expectedEndpoint);
4570

4671
var middleware = CreateMiddleware();
4772

@@ -50,7 +75,7 @@ public async Task Invoke_SkipsRouting_IfEndpointSet()
5075

5176
// Assert
5277
var endpoint = httpContext.GetEndpoint();
53-
Assert.NotNull(endpoint);
78+
Assert.Same(expectedEndpoint, endpoint);
5479
Assert.Equal("myapp", endpoint.DisplayName);
5580
}
5681

0 commit comments

Comments
 (0)