From be33d3b3e87191f8ea7f0bc3696e806a27051803 Mon Sep 17 00:00:00 2001 From: Sami Daniel Date: Wed, 16 Jul 2025 14:51:23 -0300 Subject: [PATCH 1/2] Change startvscode.sh file permissions to make it executable --- src/Http/startvscode.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 src/Http/startvscode.sh diff --git a/src/Http/startvscode.sh b/src/Http/startvscode.sh old mode 100644 new mode 100755 From cdb6e2bc384d7c43928d02c617c83a7c42a346ba Mon Sep 17 00:00:00 2001 From: Sami Daniel Date: Wed, 16 Jul 2025 14:51:51 -0300 Subject: [PATCH 2/2] Enhance endpoint selection by resolving ambiguities through constraint specificity analysis Reduce ambiguity between constraints by defining a level of specificity between the constraints. Previously, if you had two endpoints as follows: ``` GET test/{id:required} ``` and ``` GET test/{id:guid:required} ``` The DefaultEndpointSelector would not be able to determine which endpoint is the correct one among the two, as neither violates the Route Constraint policy and therefore are considered valid candidates. From a human perspective, the endpoint that has the constraint of the GUID is more specific than simply REQUIRED, but the engine did not make that distinction. Now, the DefaultEndpointSelector can eliminate ambiguities based on their constraint level of priority. Lets say the same routes described above. The engine will get two candidates available for it, but it can remove the first one cause its less specific than second one. But it is interesting to note that if the system cannot determine the priority, it will still report the ambiguity. Let's assume that the second request was equal to the first, it would trigger AmbiguousMatchException. The currently specifity order is defined as (from higher to lower one): 1 - Strong typed route constraint (e.g int, guid, long etc.) 2 - Ranged route contraint (e.g min, max, range) 3 - Length route constraint (e.g length, minlength, maxlength) 4 - String patterns (e.g regex, alphanumerical) 5 - File and non file 6 - Unknown route constraint 7 - Required route constraint As mentioned earlier, if after processing the ambiguities he still hasn't been able to determine a specificity, he will report the ambiguity, since there is nothing to be done in this case : ). --- .../src/Matching/DefaultEndpointSelector.cs | 127 +++++++++++++++++- .../test/UnitTests/Matching/DfaMatcherTest.cs | 61 +++++++++ 2 files changed, 181 insertions(+), 7 deletions(-) diff --git a/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs b/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs index becb14415dc8..3dcfcaeb2cba 100644 --- a/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs +++ b/src/Http/Routing/src/Matching/DefaultEndpointSelector.cs @@ -60,6 +60,8 @@ private static void ProcessFinalCandidates( Endpoint? endpoint = null; RouteValueDictionary? values = null; int? foundScore = null; + var candidatesWithSameScore = new List(); + for (var i = 0; i < candidateState.Length; i++) { ref var state = ref candidateState[i]; @@ -74,6 +76,7 @@ private static void ProcessFinalCandidates( endpoint = state.Endpoint; values = state.Values; foundScore = state.Score; + candidatesWithSameScore.Add(state); } else if (foundScore < state.Score) { @@ -85,15 +88,27 @@ private static void ProcessFinalCandidates( } else if (foundScore == state.Score) { - // This is the second match we've found of the same score, so there - // must be an ambiguity. - // - // Don't worry about the 'null == state.Score' case, it returns false. + // Same score - collect for constraint specificity analysis. We cant + // just dismiss these candidate and report an ambiguity, we need to + // analyze them for constraint specificity. + candidatesWithSameScore.Add(state); + } + } + // If we have multiple candidates with the same score, try to resolve using + // constraint specificity rules + if (candidatesWithSameScore.Count > 1) + { + var mostSpecific = SelectMostSpecificEndpoint(candidatesWithSameScore); + if (mostSpecific.HasValue) + { + endpoint = mostSpecific.Value.Endpoint; + values = mostSpecific.Value.Values; + } + else + { + // Still ambiguous after constraint analysis ReportAmbiguity(candidateState); - - // Unreachable, ReportAmbiguity always throws. - throw new NotSupportedException(); } } @@ -104,6 +119,104 @@ private static void ProcessFinalCandidates( } } + private static CandidateState? SelectMostSpecificEndpoint(List candidates) + { + CandidateState? mostSpecific = null; + var highestSpecificity = -1; + var hasAmbiguity = false; + + foreach (var candidate in candidates) + { + if (candidate.Endpoint is not RouteEndpoint routeEndpoint) + { + continue; + } + + var specificity = CalculateConstraintSpecificity(routeEndpoint); + + if (specificity > highestSpecificity) + { + highestSpecificity = specificity; + mostSpecific = candidate; + hasAmbiguity = false; + } + else if (specificity == highestSpecificity) + { + // Okay, note the ambiguity and continue trying + // to determine a higher level of specificity. + hasAmbiguity = true; + } + } + + return hasAmbiguity ? null : mostSpecific; + } + + private static int CalculateConstraintSpecificity(RouteEndpoint endpoint) + { + var specificity = 0; + var routePattern = endpoint.RoutePattern; + + foreach (var parameter in routePattern.Parameters) + { + // We may have parameter without constraints, e.g. "id" in "/products/{id}" + if (parameter.ParameterPolicies?.Count > 0) + { + foreach (var policy in parameter.ParameterPolicies) + { + if (policy.Content != null) + { + specificity += GetConstraintSpecificityWeight(policy.Content); + } + } + } + } + + return specificity; + } + + private static int GetConstraintSpecificityWeight(string constraintName) + { + return constraintName.ToLowerInvariant() switch + { + // Strong typed constraints that are very restrictive and has + // the highest specificity + "guid" => 100, + "datetime" => 90, + "decimal" => 85, + "double" => 80, + "float" => 75, + "long" => 70, + "int" => 65, + "bool" => 60, + + // Range constraint are more restrictive than other types + var range when range.StartsWith("range(", StringComparison.OrdinalIgnoreCase) => 55, + var min when min.StartsWith("min(", StringComparison.OrdinalIgnoreCase) => 50, + var max when max.StartsWith("max(", StringComparison.OrdinalIgnoreCase) => 50, + + // This one is a bit odd, but we will consider it less specific than range, + // since it defines only the length of the value and will consider it as raw + // string not a number. + var length when length.StartsWith("length(", StringComparison.OrdinalIgnoreCase) => 45, + var minlength when minlength.StartsWith("minlength(", StringComparison.OrdinalIgnoreCase) => 40, + var maxlength when maxlength.StartsWith("maxlength(", StringComparison.OrdinalIgnoreCase) => 40, + + // String patterns, which are less specific than length + "alpha" => 35, + var regex when regex.StartsWith("regex(", StringComparison.OrdinalIgnoreCase) => 30, + + // File constraints + "file" => 25, + "nonfile" => 25, + + // Least specific just requires non empty + "required" => 10, + + // Unknown constraint, assign medium specificity to it + _ => 20 + }; + } + private static void ReportAmbiguity(Span candidateState) { // If we get here it's the result of an ambiguity - we're OK with this diff --git a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs index 4e577e4a6072..84f5ef531289 100644 --- a/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs +++ b/src/Http/Routing/test/UnitTests/Matching/DfaMatcherTest.cs @@ -58,6 +58,67 @@ private DataSourceDependentMatcher CreateDfaMatcher( return Assert.IsType(factory.CreateMatcher(dataSource)); } + [Fact] + public async Task MatchAsync_SimilarEndpoints_CanDetermine_MostSpecificOne() + { + // Arrange + var dataSource = new DefaultEndpointDataSource(new List + { + CreateEndpoint("/test/{name:required}", 0), + CreateEndpoint("/test/{id:guid:required}", 0), + }); + + var matcher = CreateDfaMatcher(dataSource); + + var httpContext = CreateContext(); + httpContext.Request.Path = "/test/" + Guid.NewGuid().ToString(); + + // Act + await matcher.MatchAsync(httpContext); + + // Assert + Assert.Same(dataSource.Endpoints[1], httpContext.GetEndpoint()); + } + + [Fact] + public async Task MatchAsync_MultipleEndpointsWithSameRequiredValues_EndpointMatched() + { + // Arrange + var endpoint1 = CreateEndpoint( + "{controller}/{action}/{id?}", + 0, + requiredValues: new { controller = "Home", action = "Index", area = (string)null, page = (string)null }); + var endpoint2 = CreateEndpoint( + "{controller}/{action}/{id?}", + 0, + requiredValues: new { controller = "Login", action = "Index", area = (string)null, page = (string)null }); + + var dataSource = new DefaultEndpointDataSource(new List + { + endpoint1, + endpoint2 + }); + + var matcher = CreateDfaMatcher(dataSource); + + var httpContext = CreateContext(); + httpContext.Request.Path = "/Home/Index/123"; + + // Act 1 + await matcher.MatchAsync(httpContext); + + // Assert 1 + Assert.Same(endpoint1, httpContext.GetEndpoint()); + + httpContext.Request.Path = "/Login/Index/123"; + + // Act 2 + await matcher.MatchAsync(httpContext); + + // Assert 2 + Assert.Same(endpoint2, httpContext.GetEndpoint()); + } + [Fact] public async Task MatchAsync_ValidRouteConstraint_EndpointMatched() {