Merge branch 'rel/2.0.0-preview1' into dev

src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs

@@ -64,6 +64,9 @@ namespace Microsoft.AspNetCore.Routing.Template
             // If the URI had ordered parameters a="1", b="2", c="3" and the new values
             // specified that b="9", then we need to invalidate everything after it. The new
             // values should then be a="1", b="9", c=<no value>.
+            //
+            // We also handle the case where a parameter is optional but has no value - we shouldn't
+            // accept additional parameters that appear *after* that parameter.
             for (var i = 0; i < _template.Parameters.Count; i++)
             {
                 var parameter = _template.Parameters[i];
@@ -87,6 +90,25 @@ namespace Microsoft.AspNetCore.Routing.Template
                     }
                 }
 
+                if (!hasNewParameterValue && 
+                    !hasCurrentParameterValue && 
+                    _defaults?.ContainsKey(parameter.Name) != true)
+                {
+                    // This is an unsatisfied parameter value and there are no defaults. We might still
+                    // be able to generate a URL but we should stop 'accepting' ambient values.
+                    //
+                    // This might be a case like:
+                    //  template: a/{b?}/{c?}
+                    //  ambient: { c = 17 }
+                    //  values: { }
+                    //
+                    // We can still generate a URL from this ("/a") but we shouldn't accept 'c' because
+                    // we can't use it.
+                    // 
+                    // In the example above we should fall into this block for 'b'.
+                    break;
+                }
+
                 // If the parameter is a match, add it to the list of values we will use for URI generation
                 if (hasNewParameterValue)
                 {

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

@@ -1372,6 +1372,103 @@ namespace Microsoft.AspNetCore.Routing
             Assert.Empty(pathData.DataTokens);
         }
 
+        [Fact]
+        public void GetVirtualPath_TwoOptionalParameters_OneValueFromAmbientValues()
+        {
+            // Arrange
+            var route = CreateRoute(
+                template: "a/{b=15}/{c?}/{d?}",
+                defaults: null,
+                handleRequest: true,
+                constraints: null);
+
+            var context = CreateVirtualPathContext(
+                values: new { },
+                ambientValues: new { c = "17" });
+
+            // Act
+            var pathData = route.GetVirtualPath(context);
+
+            // Assert
+            Assert.NotNull(pathData);
+            Assert.Equal("/a/15/17", pathData.VirtualPath);
+            Assert.Same(route, pathData.Router);
+            Assert.Empty(pathData.DataTokens);
+        }
+
+
+        [Fact]
+        public void GetVirtualPath_OptionalParameterAfterDefault_OneValueFromAmbientValues()
+        {
+            // Arrange
+            var route = CreateRoute(
+                template: "a/{b=15}/{c?}",
+                defaults: null,
+                handleRequest: true,
+                constraints: null);
+
+            var context = CreateVirtualPathContext(
+                values: new { },
+                ambientValues: new { c = "17" });
+
+            // Act
+            var pathData = route.GetVirtualPath(context);
+
+            // Assert
+            Assert.NotNull(pathData);
+            Assert.Equal("/a/15/17", pathData.VirtualPath);
+            Assert.Same(route, pathData.Router);
+            Assert.Empty(pathData.DataTokens);
+        }
+
+        [Fact]
+        public void GetVirtualPath_TwoOptionalParametersAfterDefault_OneValueFromAmbientValues()
+        {
+            // Arrange
+            var route = CreateRoute(
+                template: "a/{b=15}/{c?}/{d?}",
+                defaults: null,
+                handleRequest: true,
+                constraints: null);
+
+            var context = CreateVirtualPathContext(
+                values: new { },
+                ambientValues: new { c = "17" });
+
+            // Act
+            var pathData = route.GetVirtualPath(context);
+
+            // Assert
+            Assert.NotNull(pathData);
+            Assert.Equal("/a/15/17", pathData.VirtualPath);
+            Assert.Same(route, pathData.Router);
+            Assert.Empty(pathData.DataTokens);
+        }
+
+        [Fact]
+        public void GetVirtualPath_TwoOptionalParametersAfterDefault_LastValueFromAmbientValues()
+        {
+            // Arrange
+            var route = CreateRoute(
+                template: "a/{b=15}/{c?}/{d?}",
+                defaults: null,
+                handleRequest: true,
+                constraints: null);
+
+            var context = CreateVirtualPathContext(
+                values: new { },
+                ambientValues: new { d = "17" });
+
+            // Act
+            var pathData = route.GetVirtualPath(context);
+
+            // Assert
+            Assert.NotNull(pathData);
+            Assert.Equal("/a", pathData.VirtualPath);
+            Assert.Same(route, pathData.Router);
+            Assert.Empty(pathData.DataTokens);
+        }
+
         private static VirtualPathContext CreateVirtualPathContext(object values)
         {
             return CreateVirtualPathContext(new RouteValueDictionary(values), null);

test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs

@@ -1317,6 +1317,27 @@ namespace Microsoft.AspNetCore.Routing.Tree
             Assert.Empty(pathData.DataTokens);
         }
 
+        [Fact]
+        public void TreeRouter_GenerateLink_Match_HasTwoOptionalParametersWithoutValues()
+        {
+            // Arrange
+            var builder = CreateBuilder();
+            MapOutboundEntry(builder, "Customers/SeparatePageModels/{handler?}/{id?}", new { page = "/Customers/SeparatePageModels/Index" });
+            var route = builder.Build();
+
+            var context = CreateVirtualPathContext(new { page = "/Customers/SeparatePageModels/Index" }, new { page = "/Customers/SeparatePageModels/Edit", id = "17" });
+
+            // Act
+            var pathData = route.GetVirtualPath(context);
+
+            // Assert
+            Assert.NotNull(pathData);
+            Assert.Equal("/Customers/SeparatePageModels", pathData.VirtualPath);
+            Assert.Same(route, pathData.Router);
+            Assert.Empty(pathData.DataTokens);
+        }
+
+
         [Fact]
         public void TreeRouter_GenerateLink_Match_WithParameters()
         {