From 4bfd663c451281acc4e78a9a84e105bdb4343800 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Fri, 30 Dec 2016 09:47:30 -0800 Subject: [PATCH] [Fixes #370] Raw route values should be restored after template binder failing binding values when generating a url - Reverted changes made in commit: 1c9a54aeb8fc5bf0d888cd4545a699f39ed22bdb --- .../Template/TemplateBinder.cs | 9 +-- .../RouteCollectionTest.cs | 65 ++++++++++++++++++- .../Template/TemplateBinderTests.cs | 28 ++++++++ 3 files changed, 92 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs index 73fa07b348..809a7a6913 100644 --- a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs +++ b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs @@ -171,8 +171,7 @@ namespace Microsoft.AspNetCore.Routing.Template // Add any ambient values that don't match parameters - they need to be visible to constraints // but they will ignored by link generation. - // Perf: Lazily allocate RouteValueDictionary only when needed as in most cases combined is same as accepted values - RouteValueDictionary combinedValues = null; + var combinedValues = new RouteValueDictionary(context.AcceptedValues); if (ambientValues != null) { foreach (var kvp in ambientValues) @@ -182,10 +181,6 @@ namespace Microsoft.AspNetCore.Routing.Template var parameter = GetParameter(kvp.Key); if (parameter == null && !context.AcceptedValues.ContainsKey(kvp.Key)) { - if (combinedValues == null) - { - combinedValues = new RouteValueDictionary(context.AcceptedValues); - } combinedValues.Add(kvp.Key, kvp.Value); } } @@ -195,7 +190,7 @@ namespace Microsoft.AspNetCore.Routing.Template return new TemplateValuesResult() { AcceptedValues = context.AcceptedValues, - CombinedValues = combinedValues ?? context.AcceptedValues + CombinedValues = combinedValues, }; } diff --git a/test/Microsoft.AspNetCore.Routing.Tests/RouteCollectionTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/RouteCollectionTest.cs index 283da1ad3c..73b4519f87 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/RouteCollectionTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/RouteCollectionTest.cs @@ -363,6 +363,61 @@ namespace Microsoft.AspNetCore.Routing Assert.Empty(pathData.DataTokens); } + public static IEnumerable RestoresRouteDataForEachRouterData + { + get + { + // Here 'area' segment doesn't have a value but the later segments have values. This is an invalid + // route match and the url generation should look into the next available route in the collection. + yield return new object[] { + new Route[] + { + CreateTemplateRoute("{area?}/{controller=Home}/{action=Index}/{id?}", "1"), + CreateTemplateRoute("{controller=Home}/{action=Index}/{id?}", "2") + }, + new RouteValueDictionary(new { controller = "Test", action = "Index" }), + "/Test", + "2" }; + + // Here the segment 'a' is valid but 'b' is not as it would be empty. This would be an invalid route match, but + // the route value of 'a' should still be present to be evaluated for the next available route. + yield return new object[] { + new[] + { + CreateTemplateRoute("{a}/{b?}/{c}", "1"), + CreateTemplateRoute("{a=Home}/{b=Index}", "2") + }, + new RouteValueDictionary(new { a = "Test", c = "Foo" }), + "/Test?c=Foo", + "2" }; + } + } + + [Theory] + [MemberData(nameof(RestoresRouteDataForEachRouterData))] + public void GetVirtualPath_RestoresRouteData_ForEachRouter( + Route[] routes, + RouteValueDictionary routeValues, + string expectedUrl, + string expectedRouteToMatch) + { + // Arrange + var routeCollection = new RouteCollection(); + foreach (var route in routes) + { + routeCollection.Add(route); + } + var context = CreateVirtualPathContext(routeValues); + + // Act + var pathData = routeCollection.GetVirtualPath(context); + + // Assert + Assert.Equal(expectedUrl, pathData.VirtualPath); + Assert.Same(expectedRouteToMatch, ((INamedRouter)pathData.Router).Name); + Assert.Empty(pathData.DataTokens); + } + [Fact] public void GetVirtualPath_NoBestEffort_NoMatch() { @@ -496,14 +551,18 @@ namespace Microsoft.AspNetCore.Routing private static Route CreateTemplateRoute( string template, string routerName = null, - RouteValueDictionary dataTokens = null) + RouteValueDictionary dataTokens = null, + IInlineConstraintResolver constraintResolver = null) { var target = new Mock(MockBehavior.Strict); target .Setup(e => e.GetVirtualPath(It.IsAny())) .Returns(rc => null); - var resolverMock = new Mock(); + if (constraintResolver == null) + { + constraintResolver = new Mock().Object; + } return new Route( target.Object, @@ -512,7 +571,7 @@ namespace Microsoft.AspNetCore.Routing defaults: null, constraints: null, dataTokens: dataTokens, - inlineConstraintResolver: resolverMock.Object); + inlineConstraintResolver: constraintResolver); } private static VirtualPathContext CreateVirtualPathContext( diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs index fc4186561b..4d71164ce6 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Template/TemplateBinderTests.cs @@ -691,6 +691,34 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests "/UrlEncode[[v1]]"); } + [Fact] + public void TemplateBinder_KeepsExplicitlySuppliedRouteValues_OnFailedRouetMatch() + { + // Arrange + var template = "{area?}/{controller=Home}/{action=Index}/{id?}"; + var encoder = new UrlTestEncoder(); + var binder = new TemplateBinder( + new UrlTestEncoder(), + new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy(encoder)), + TemplateParser.Parse(template), + defaults: null); + var ambientValues = new RouteValueDictionary(); + var routeValues = new RouteValueDictionary(new { controller = "Test", action = "Index" }); + + // Act + var templateValuesResult = binder.GetValues(ambientValues, routeValues); + var boundTemplate = binder.BindValues(templateValuesResult.AcceptedValues); + + // Assert + Assert.Null(boundTemplate); + Assert.Equal(2, templateValuesResult.CombinedValues.Count); + object routeValue; + Assert.True(templateValuesResult.CombinedValues.TryGetValue("controller", out routeValue)); + Assert.Equal("Test", routeValue?.ToString()); + Assert.True(templateValuesResult.CombinedValues.TryGetValue("action", out routeValue)); + Assert.Equal("Index", routeValue?.ToString()); + } + #if ROUTE_COLLECTION [Fact]