[Fixes #370] Raw route values should be restored after template binder failing binding values when generating a url

- Reverted changes made in commit: 1c9a54aeb8

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,
             };
         }
 

test/Microsoft.AspNetCore.Routing.Tests/RouteCollectionTest.cs

@@ -363,6 +363,61 @@ namespace Microsoft.AspNetCore.Routing
             Assert.Empty(pathData.DataTokens);
         }
 
+        public static IEnumerable<object[]> 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<IRouter>(MockBehavior.Strict);
             target
                 .Setup(e => e.GetVirtualPath(It.IsAny<VirtualPathContext>()))
                 .Returns<VirtualPathContext>(rc => null);
 
-            var resolverMock = new Mock<IInlineConstraintResolver>();
+            if (constraintResolver == null)
+            {
+                constraintResolver = new Mock<IInlineConstraintResolver>().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(

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]