[release/6.0] Check for duplicate endpoint names on startup (#36488)

* Check for duplicate endpoint names on startup

* Add display name to exception message

* Always validate duplicate endpoints and add more info to error

Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: Safia Abdalla <[email protected]>

Author: github-actions[bot]

src/Http/Routing/src/Matching/DataSourceDependentMatcher.cs

@@ -41,14 +41,33 @@ public override Task MatchAsync(HttpContext httpContext)
         private Matcher CreateMatcher(IReadOnlyList<Endpoint> endpoints)
         {
             var builder = _matcherBuilderFactory();
+            var seenEndpointNames = new Dictionary<string, string?>();
             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<ISuppressMatchingMetadata>()?.SuppressMatching != true)
+                if (endpoints[i] is RouteEndpoint endpoint)
                 {
-                    builder.AddEndpoint(endpoint);
+                    // Validate that endpoint names are unique.
+                    var endpointName = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>()?.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<ISuppressMatchingMetadata>()?.SuppressMatching != true)
+                    {
+                        builder.AddEndpoint(endpoint);
+                    }
                 }
             }
 

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<InvalidOperationException>(
+                () => 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<InvalidOperationException>(
+                () => 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<AggregateException>(
+                () => 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<MatcherBuilder> Create = () => new TestMatcherBuilder();