This fixes the case described in the comments in TemplateBinder.

This case is much more common for pages which is why we're only seeing it
now. We've had this issue for all of 1.0.0 in both conventional and
attribute routing.
This commit is contained in:
Ryan Nowak 2017-04-28 12:22:04 -07:00
parent acebfeff3e
commit 232b73a151
3 changed files with 140 additions and 0 deletions

View File

@ -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)
{

View File

@ -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);

View File

@ -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()
{