diff --git a/src/Microsoft.AspNetCore.Routing/Template/TemplateMatcher.cs b/src/Microsoft.AspNetCore.Routing/Template/TemplateMatcher.cs index 1d74bcb361..7efe3b3bc4 100644 --- a/src/Microsoft.AspNetCore.Routing/Template/TemplateMatcher.cs +++ b/src/Microsoft.AspNetCore.Routing/Template/TemplateMatcher.cs @@ -232,11 +232,11 @@ namespace Microsoft.AspNetCore.Routing.Template // It's ok for a catch-all to produce a null value if (_hasDefaultValue[i] || part.IsCatchAll) { - // Don't trounce an existing value with a null. + // Don't replace an existing value with a null. var defaultValue = _defaultValues[i]; if (defaultValue != null || !values.ContainsKey(part.Name)) { - values[part.Name] = _defaultValues[i]; + values[part.Name] = defaultValue; } } } diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateMatcherTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateMatcherTests.cs index a08d9c3766..afa2724ecd 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateMatcherTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateMatcherTests.cs @@ -697,7 +697,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests } [Fact] - public void TryMatch_RouteWithCatchAllClauseCapturesManySlashes() + public void TryMatch_RouteWithCatchAll_MatchesMultiplePathSegments() { // Arrange var matcher = CreateMatcher("{p1}/{*p2}"); @@ -715,7 +715,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests } [Fact] - public void TryMatch_RouteWithCatchAllClauseCapturesTrailingSlash() + public void TryMatch_RouteWithCatchAll_MatchesTrailingSlash() { // Arrange var matcher = CreateMatcher("{p1}/{*p2}"); @@ -733,7 +733,7 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests } [Fact] - public void TryMatch_RouteWithCatchAllClauseCapturesEmptyContent() + public void TryMatch_RouteWithCatchAll_MatchesEmptyContent() { // Arrange var matcher = CreateMatcher("{p1}/{*p2}"); @@ -751,12 +751,30 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests } [Fact] - public void TryMatch_RouteWithCatchAllClauseUsesDefaultValueForEmptyContent() + public void TryMatch_RouteWithCatchAll_MatchesEmptyContent_DoesNotReplaceExistingRouteValue() + { + // Arrange + var matcher = CreateMatcher("{p1}/{*p2}"); + + var values = new RouteValueDictionary(new { p2 = "hello" }); + + // Act + var match = matcher.TryMatch("/v1", values); + + // Assert + Assert.True(match); + Assert.Equal(2, values.Count); + Assert.Equal("v1", values["p1"]); + Assert.Equal("hello", values["p2"]); + } + + [Fact] + public void TryMatch_RouteWithCatchAll_UsesDefaultValueForEmptyContent() { // Arrange var matcher = CreateMatcher("{p1}/{*p2}", new { p2 = "catchall" }); - var values = new RouteValueDictionary(); + var values = new RouteValueDictionary(new { p2 = "overridden" }); // Act var match = matcher.TryMatch("/v1", values); @@ -769,12 +787,12 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests } [Fact] - public void TryMatch_RouteWithCatchAllClauseIgnoresDefaultValueForNonEmptyContent() + public void TryMatch_RouteWithCatchAll_IgnoresDefaultValueForNonEmptyContent() { // Arrange var matcher = CreateMatcher("{p1}/{*p2}", new { p2 = "catchall" }); - var values = new RouteValueDictionary(); + var values = new RouteValueDictionary(new { p2 = "overridden" }); // Act var match = matcher.TryMatch("/v1/hello/whatever", values); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs index dba3fe2a25..8fb123f38b 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs @@ -152,6 +152,97 @@ namespace Microsoft.AspNetCore.Routing.Tree Assert.Equal(expectedResult, context.RouteData.Values["path"]); } + [Theory] + [InlineData("a/{*path}", "/a")] + [InlineData("a/{*path}", "/a/")] + public async Task TreeRouter_RouteAsync_MatchesCatchAll_NullValue( + string template, + string requestPath) + { + // Arrange + var next = new Mock(); + next + .Setup(r => r.RouteAsync(It.IsAny())) + .Callback(c => c.Handler = NullHandler) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var firstRoute = CreateMatchingEntry(next.Object, template, order: 0); + var matchingRoutes = new[] { firstRoute }; + var linkGenerationEntries = Enumerable.Empty(); + var attributeRoute = CreateAttributeRoute(next.Object, matchingRoutes, linkGenerationEntries); + var context = CreateRouteContext(requestPath); + + // Act + await attributeRoute.RouteAsync(context); + + // Assert + Assert.NotNull(context.Handler); + Assert.Null(context.RouteData.Values["path"]); + } + + [Theory] + [InlineData("a/{*path}", "/a")] + [InlineData("a/{*path}", "/a/")] + public async Task TreeRouter_RouteAsync_MatchesCatchAll_NullValue_DoesNotReplaceExistingValue( + string template, + string requestPath) + { + // Arrange + var next = new Mock(); + next + .Setup(r => r.RouteAsync(It.IsAny())) + .Callback(c => c.Handler = NullHandler) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var firstRoute = CreateMatchingEntry(next.Object, template, order: 0); + var matchingRoutes = new[] { firstRoute }; + var linkGenerationEntries = Enumerable.Empty(); + var attributeRoute = CreateAttributeRoute(next.Object, matchingRoutes, linkGenerationEntries); + + var context = CreateRouteContext(requestPath); + context.RouteData.Values["path"] = "existing-value"; + + // Act + await attributeRoute.RouteAsync(context); + + // Assert + Assert.NotNull(context.Handler); + Assert.Equal("existing-value", context.RouteData.Values["path"]); + } + + [Theory] + [InlineData("a/{*path=default}", "/a")] + [InlineData("a/{*path=default}", "/a/")] + public async Task TreeRouter_RouteAsync_MatchesCatchAll_UsesDefaultValue( + string template, + string requestPath) + { + // Arrange + var next = new Mock(); + next + .Setup(r => r.RouteAsync(It.IsAny())) + .Callback(c => c.Handler = NullHandler) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var firstRoute = CreateMatchingEntry(next.Object, template, order: 0); + var matchingRoutes = new[] { firstRoute }; + var linkGenerationEntries = Enumerable.Empty(); + var attributeRoute = CreateAttributeRoute(next.Object, matchingRoutes, linkGenerationEntries); + + var context = CreateRouteContext(requestPath); + context.RouteData.Values["path"] = "existing-value"; + + // Act + await attributeRoute.RouteAsync(context); + + // Assert + Assert.NotNull(context.Handler); + Assert.Equal("default", context.RouteData.Values["path"]); + } + [Theory] [InlineData("template/5")] [InlineData("template/{parameter:int}")] @@ -1581,9 +1672,19 @@ namespace Microsoft.AspNetCore.Routing.Tree entry.Target = router; entry.RouteTemplate = TemplateParser.Parse(template); var parsedRouteTemplate = TemplateParser.Parse(template); - entry.TemplateMatcher = new TemplateMatcher( - parsedRouteTemplate, - new RouteValueDictionary(new { test_route_group = routeGroup })); + + var defaults = new RouteValueDictionary(); + foreach (var parameter in parsedRouteTemplate.Parameters) + { + if (parameter.DefaultValue != null) + { + defaults.Add(parameter.Name, parameter.DefaultValue); + } + } + + defaults["test_route_group"] = routeGroup; + + entry.TemplateMatcher = new TemplateMatcher(parsedRouteTemplate, defaults); entry.Precedence = RoutePrecedence.ComputeMatched(parsedRouteTemplate); entry.Order = order; entry.Constraints = GetRouteConstriants(CreateConstraintResolver(), template, parsedRouteTemplate);