diff --git a/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs b/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs index e8362bd347da..2906028a02a7 100644 --- a/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs +++ b/src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs @@ -41,14 +41,33 @@ public override Task MatchAsync(HttpContext httpContext) private Matcher CreateMatcher(IReadOnlyList endpoints) { var builder = _matcherBuilderFactory(); + var seenEndpointNames = new Dictionary(); for (var i = 0; i < endpoints.Count; i++) { // By design we only look at RouteEndpoint here. It's possible to // register other endpoint types, which are non-routable, and it's // ok that we won't route to them. - if (endpoints[i] is RouteEndpoint endpoint && endpoint.Metadata.GetMetadata()?.SuppressMatching != true) + if (endpoints[i] is RouteEndpoint endpoint) { - builder.AddEndpoint(endpoint); + // Validate that endpoint names are unique. + var endpointName = endpoint.Metadata.GetMetadata()?.EndpointName; + if (endpointName is not null) + { + if (seenEndpointNames.TryGetValue(endpointName, out var existingEndpoint)) + { + throw new InvalidOperationException($"Duplicate endpoint name '{endpointName}' found on '{endpoint.DisplayName}' and '{existingEndpoint}'. Endpoint names must be globally unique."); + } + + seenEndpointNames.Add(endpointName, endpoint.DisplayName ?? endpoint.RoutePattern.RawText); + } + + // We check for duplicate endpoint names on all endpoints regardless + // of whether they suppress matching because endpoint names can be + // used in OpenAPI specifications as well. + if (endpoint.Metadata.GetMetadata()?.SuppressMatching != true) + { + builder.AddEndpoint(endpoint); + } } } diff --git a/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs b/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs index 42374fbe1c1a..88563ba750d0 100644 --- a/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/DataSourceDependentMatcherTest.cs @@ -141,6 +141,102 @@ public void Matcher_UnsuppressedEndpoint_IsUsed() Assert.Same(endpoint, Assert.Single(inner.Endpoints)); } + [Fact] + public void Matcher_ThrowsOnDuplicateEndpoints() + { + // Arrange + var expectedError = "Duplicate endpoint name 'Foo' found on '/bar' and '/foo'. Endpoint names must be globally unique."; + var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); + dataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/foo"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Foo")), + "/foo" + )); + dataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/bar"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Foo")), + "/bar" + )); + + // Assert + var exception = Assert.Throws( + () => new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create)); + Assert.Equal(expectedError, exception.Message); + } + + [Fact] + public void Matcher_ThrowsOnDuplicateEndpointsFromMultipleSources() + { + // Arrange + var expectedError = "Duplicate endpoint name 'Foo' found on '/foo2' and '/foo'. Endpoint names must be globally unique."; + var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); + dataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/foo"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Foo")), + "/foo" + )); + dataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/bar"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Bar")), + "/bar" + )); + var anotherDataSource = new DynamicEndpointDataSource(); + anotherDataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/foo2"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Foo")), + "/foo2" + )); + + var compositeDataSource = new CompositeEndpointDataSource(new[] { dataSource, anotherDataSource }); + + // Assert + var exception = Assert.Throws( + () => new DataSourceDependentMatcher(compositeDataSource, lifetime, TestMatcherBuilder.Create)); + Assert.Equal(expectedError, exception.Message); + } + + [Fact] + public void Matcher_ThrowsOnDuplicateEndpointAddedLater() + { + // Arrange + var expectedError = "Duplicate endpoint name 'Foo' found on '/bar' and '/foo'. Endpoint names must be globally unique."; + var dataSource = new DynamicEndpointDataSource(); + var lifetime = new DataSourceDependentMatcher.Lifetime(); + dataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/foo"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Foo")), + "/foo" + )); + + // Act (should be all good since no duplicate has been added yet) + var matcher = new DataSourceDependentMatcher(dataSource, lifetime, TestMatcherBuilder.Create); + + // Assert that rerunning initializer throws AggregateException + var exception = Assert.Throws( + () => dataSource.AddEndpoint(new RouteEndpoint( + TestConstants.EmptyRequestDelegate, + RoutePatternFactory.Parse("/bar"), + 0, + new EndpointMetadataCollection(new EndpointNameMetadata("Foo")), + "/bar" + ))); + Assert.Equal(expectedError, exception.InnerException.Message); + } + private class TestMatcherBuilder : MatcherBuilder { public static Func Create = () => new TestMatcherBuilder();