huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix #5055 use of page parameter (#7441) 

This change introduces some new heuristics to make it easier to use
'page' or other resevered parameter names as parameters in URL
generation.

--

The main change here is to allow the link generation tree to *ignore* a
value passed in to URL generation when it conflicts with an endpoint's
required values.

The main concern of this feature area is "how do we tell whether you are
linking to an action or a page?". Routing attempts to do the right thing
will requiring very little from the user in terms of expressing intent.

In this case, we try to tell the difference between an attempt to generate
a link to an action due to the presence of the 'action' parameter and
absence of the 'page' parameter. This obviously doesn't work when you
want to use 'page' as a non-reserved parameter in an action. The same
case occurs for pages, but users are already used to the idea that
'action' is a reserved word in MVC.

We can loosen this restriction when the value that's supplied for 'page'
is known not to be any existing value of the 'page' route value.

This approach seems somewhat reasonable but has many of the problems inherent
to this area. When it fails (the value you want to use for 'page' causes
a conflict) - it's going to be esoteric and hard to understand.
This commit is contained in:
Ryan Nowak 2019-02-22 11:44:43 -08:00 committed by Artak
parent 99d7d34447
commit 3d448f7197
5 changed files with 460 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/Http/Routing/src/Internal/LinkGenerationDecisionTree.cs
View File

@ -21,12 +21,29 @@ namespace Microsoft.AspNetCore.Routing.Internal
private static readonly RouteValueDictionary EmptyAmbientValues = new RouteValueDictionary();
private readonly DecisionTreeNode<OutboundMatch> _root;
private readonly Dictionary<string, HashSet<object>> _knownValues;
public LinkGenerationDecisionTree(IReadOnlyList<OutboundMatch> entries)
{
_root = DecisionTreeBuilder<OutboundMatch>.GenerateTree(
entries,
new OutboundMatchClassifier());
_knownValues = new Dictionary<string, HashSet<object>>(StringComparer.OrdinalIgnoreCase);
for (var i = 0; i < entries.Count; i++)
{
var entry = entries[i];
foreach (var kvp in entry.Entry.RequiredLinkValues)
{
if (!_knownValues.TryGetValue(kvp.Key, out var values))
{
values = new HashSet<object>(RouteValueEqualityComparer.Default);
_knownValues.Add(kvp.Key, values);
}
values.Add(kvp.Value ?? string.Empty);
}
}
}
public IList<OutboundMatchResult> GetMatches(RouteValueDictionary values, RouteValueDictionary ambientValues)
@ -93,6 +110,19 @@ namespace Microsoft.AspNetCore.Routing.Internal
{
Walk(results, values, ambientValues, branch, isFallbackPath);
}
else
{
// If an explicitly specified value doesn't match any branch, then speculatively walk the
// "null" path if the value doesn't match any known value.
//
// This can happen when linking from a page <-> action. We want to be
// able to use "page" and "action" as normal route parameters.
var knownValues = _knownValues[key];
if (!knownValues.Contains(value ?? string.Empty) && criterion.Branches.TryGetValue(string.Empty, out branch))
{
Walk(results, values, ambientValues, branch, isFallbackPath: true);
}
}
}
else
{
@ -210,4 +240,4 @@ namespace Microsoft.AspNetCore.Routing.Internal
}
}
}
}
}

381
src/Http/Routing/test/UnitTests/Internal/LinkGenerationDecisionTreeTest.cs
View File

@ -340,6 +340,385 @@ namespace Microsoft.AspNetCore.Routing.Internal.Routing
Assert.Equal(entries, matches);
}
[Fact]
public void GetMatches_ControllersWithArea_AllValuesExplicit()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Store", action = "Buy", area = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { controller = "Store", action = "Buy", area = "Admin" });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { controller = "Store", action = "Buy", area = "Admin" });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); });
}
[Fact]
public void GetMatches_ControllersWithArea_SomeValuesAmbient()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Store", action = "Buy", area = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { controller = "Store", action = "Buy", area = "Admin" });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { controller = "Store", }, new { action = "Buy", area = "Admin", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); },
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_ControllersWithArea_AllValuesAmbient()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Store", action = "Buy", area = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { controller = "Store", action = "Buy", area = "Admin" });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { }, new { controller = "Store", action = "Buy", area = "Admin", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); },
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_PagesWithArea_AllValuesExplicit()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { page = "/Store/Buy", area = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = "Admin" });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { page = "/Store/Buy", area = "Admin" });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); });
}
[Fact]
public void GetMatches_PagesWithArea_SomeValuesAmbient()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { page = "/Store/Buy", area = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = "Admin" });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { page = "/Store/Buy", }, new { area = "Admin", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); },
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_PagesWithArea_AllValuesAmbient()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { page = "/Store/Buy", area = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = "Admin" });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { }, new { page = "/Store/Buy", area = "Admin", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); },
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_LinkToControllerFromPage()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = (string)null, page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = (string)null, controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { controller = "Home", action = "Index", }, new { page = "/Store/Buy", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_LinkToControllerFromPage_WithArea()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = "Admin", page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = "Admin", controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { controller = "Home", action = "Index", }, new { page = "/Store/Buy", area = "Admin", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_LinkToControllerFromPage_WithPageValue()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = (string)null, page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = (string)null, controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { controller = "Home", action = "Index", page = "16", }, new { page = "/Store/Buy", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry1, m); });
}
[Fact]
public void GetMatches_LinkToControllerFromPage_WithPageValueAmbiguous()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = (string)null, page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = (string)null, controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { controller = "Home", action = "Index", page = "/Store/Buy", }, new { page = "/Store/Buy", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Empty(matches);
}
[Fact]
public void GetMatches_LinkToPageFromController()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = (string)null, page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = (string)null, controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { page = "/Store/Buy", }, new { controller = "Home", action = "Index", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); });
}
[Fact]
public void GetMatches_LinkToPageFromController_WithArea()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = "Admin", page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = "Admin", controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { page = "/Store/Buy", }, new { controller = "Home", action = "Index", area = "Admin", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); });
}
[Fact]
public void GetMatches_LinkToPageFromController_WithActionValue()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = (string)null, page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = (string)null, controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { page = "/Store/Buy", action = "buy", }, new { controller = "Home", action = "Index", page = "16", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Collection(
matches,
m => { Assert.Same(entry2, m); });
}
[Fact]
public void GetMatches_LinkToPageFromController_WithActionValueAmbiguous()
{
// Arrange
var entries = new List<OutboundMatch>();
var entry1 = CreateMatch(new { controller = "Home", action = "Index", area = (string)null, page = (string)null, });
entry1.Entry.RouteTemplate = TemplateParser.Parse("a");
entries.Add(entry1);
var entry2 = CreateMatch(new { page = "/Store/Buy", area = (string)null, controller = (string)null, action = (string)null, });
entry2.Entry.RouteTemplate = TemplateParser.Parse("b");
entries.Add(entry2);
var tree = new LinkGenerationDecisionTree(entries);
var context = CreateContext(new { page = "/Store/Buy", action = "Index", }, new { controller = "Home", action = "Index", page = "16", });
// Act
var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList();
// Assert
Assert.Empty(matches);
}
[Fact]
public void ToDebuggerDisplayString_GivesAFlattenedTree()
{
@ -392,4 +771,4 @@ namespace Microsoft.AspNetCore.Routing.Internal.Routing
return context;
}
}
}
}

15
src/Mvc/Mvc.sln
View File

@ -293,6 +293,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Mvc.Ra
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.Test", "Mvc.Razor.RuntimeCompilation\test\Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.Test.csproj", "{2FFB927A-C039-4A1F-83A5-CBBB664A0E81}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Components.Server", "..\Components\Server\src\Microsoft.AspNetCore.Components.Server.csproj", "{916BF32D-6896-4D02-BBD1-A72878FDBDFF}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
@ -1641,6 +1643,18 @@ Global
{2FFB927A-C039-4A1F-83A5-CBBB664A0E81}.Release|Mixed Platforms.Build.0 = Release|Any CPU
{2FFB927A-C039-4A1F-83A5-CBBB664A0E81}.Release|x86.ActiveCfg = Release|Any CPU
{2FFB927A-C039-4A1F-83A5-CBBB664A0E81}.Release|x86.Build.0 = Release|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Debug|Any CPU.Build.0 = Debug|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Debug|x86.ActiveCfg = Debug|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Debug|x86.Build.0 = Debug|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Release|Any CPU.ActiveCfg = Release|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Release|Any CPU.Build.0 = Release|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Release|Mixed Platforms.Build.0 = Release|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Release|x86.ActiveCfg = Release|Any CPU
{916BF32D-6896-4D02-BBD1-A72878FDBDFF}.Release|x86.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
@ -1758,6 +1772,7 @@ Global
{F49A6E01-BFC5-4CEB-9A2C-19DDD3539510} = {25C08DED-1C7D-4C6D-B1CC-F340C1B21DE7}
{0CE75D4A-4EFD-434A-99CD-7776AE2BFD39} = {1261FF02-C7F8-4395-AA8A-29F69FC9870B}
{2FFB927A-C039-4A1F-83A5-CBBB664A0E81} = {1261FF02-C7F8-4395-AA8A-29F69FC9870B}
{916BF32D-6896-4D02-BBD1-A72878FDBDFF} = {5FE3048A-E96B-44F8-A7C4-FC590D7E04B4}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {63D344F6-F86D-40E6-85B9-0AABBE338C4A}

11
src/Mvc/test/Mvc.FunctionalTests/RoutingTestsBase.cs
View File

@ -1533,6 +1533,17 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal("Hello from middleware after routing", content);
}
[Fact]
public async Task CanUseLinkGeneration_To_ConventionalActionWithPageParameter()
{
// Act
var response = await Client.GetAsync("/PageParameter/LinkToPageParameter");
// Assert
await response.AssertStatusCodeAsync(HttpStatusCode.OK);
var content = await response.Content.ReadAsStringAsync();
Assert.Equal("/PageParameter/PageParameter?page=17", content);
}
protected static LinkBuilder LinkFrom(string url)
{

23
src/Mvc/test/WebSites/RoutingWebSite/Controllers/PageParameterController.cs Normal file
View File

@ -0,0 +1,23 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using Microsoft.AspNetCore.Mvc;
namespace RoutingWebSite.Controllers
{
public class PageParameterController : Controller
{
// We've had issues with using 'page' as a parameter in tandem with conventional
// routing + razor pages.
public ActionResult PageParameter(string page)
{
return Content($"page={page}");
}
[HttpGet("/PageParameter/LinkToPageParameter")]
public ActionResult LinkToPageParameter()
{
return Content(Url.Action(nameof(PageParameter), new { page = "17", }));
}
}
}