diff --git a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs b/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs index 809a7a6913..68770fd2bc 100644 --- a/src/Microsoft.AspNetCore.Routing/Template/TemplateBinder.cs +++ b/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=. + // + // 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) { diff --git a/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs index 1514ed4c34..e7ce265e48 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/RouteTest.cs +++ b/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); diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs index 00fd208ee9..eabd339948 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Tree/TreeRouterTest.cs +++ b/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() {