From bd995d4cb1d5a64b607f5de316f08acc9ff0f7d9 Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Mon, 9 Jul 2018 05:40:21 -0700 Subject: [PATCH] [Fixes #7959] Conventional routing with custom templates not working when you have area attributes --- .../Internal/ActionSelector.cs | 11 +- .../Internal/ActionSelectorTest.cs | 170 ++++++++++++++++++ 2 files changed, 179 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs index f9c9fa750b..b3d14d1be8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs @@ -88,7 +88,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal values[i] = value as string ?? Convert.ToString(value); } } - + if (cache.OrdinalEntries.TryGetValue(values, out var matchingRouteValues) || cache.OrdinalIgnoreCaseEntries.TryGetValue(values, out matchingRouteValues)) { @@ -441,7 +441,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal var hash = new HashCodeCombiner(); for (var i = 0; i < obj.Length; i++) { - hash.Add(obj[i], _valueComparer); + var o = obj[i]; + + // Route values define null and "" to be equivalent. + if (string.IsNullOrEmpty(o)) + { + o = null; + } + hash.Add(o, _valueComparer); } return hash.CombinedHash; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionSelectorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionSelectorTest.cs index 5a5dbe7787..6db71801b5 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionSelectorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ActionSelectorTest.cs @@ -277,6 +277,176 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure Assert.Equal(expected, candidates); } + [Fact] + public void SelectCandidates_Match_CaseSensitiveMatch_MatchesOnEmptyString() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "area", null }, + { "controller", "Home" }, + { "action", "Index" } + }, + } + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + // Example: In conventional route, one could set non-inline defaults + // new { area = "", controller = "Home", action = "Index" } + routeContext.RouteData.Values.Add("area", ""); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + var action = Assert.Single(candidates); + Assert.Same(actions[0], action); + } + + [Fact] + public void SelectCandidates_Match_CaseInsensitiveMatch_MatchesOnEmptyString() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "area", null }, + { "controller", "Home" }, + { "action", "Index" } + }, + } + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + // Example: In conventional route, one could set non-inline defaults + // new { area = "", controller = "Home", action = "Index" } + routeContext.RouteData.Values.Add("area", ""); + routeContext.RouteData.Values.Add("controller", "HoMe"); + routeContext.RouteData.Values.Add("action", "InDeX"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + var action = Assert.Single(candidates); + Assert.Same(actions[0], action); + } + + [Fact] + public void SelectCandidates_Match_MatchesOnNull() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "area", null }, + { "controller", "Home" }, + { "action", "Index" } + }, + } + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + // Example: In conventional route, one could set non-inline defaults + // new { area = (string)null, controller = "Foo", action = "Index" } + routeContext.RouteData.Values.Add("area", null); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + var action = Assert.Single(candidates); + Assert.Same(actions[0], action); + } + + [Fact] + public void SelectCandidates_Match_ActionDescriptorWithEmptyRouteValues_MatchesOnEmptyString() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "foo", "" }, + { "controller", "Home" }, + { "action", "Index" } + }, + } + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + // Example: In conventional route, one could set non-inline defaults + // new { area = (string)null, controller = "Home", action = "Index" } + routeContext.RouteData.Values.Add("foo", ""); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + var action = Assert.Single(candidates); + Assert.Same(actions[0], action); + } + + [Fact] + public void SelectCandidates_Match_ActionDescriptorWithEmptyRouteValues_MatchesOnNull() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "foo", "" }, + { "controller", "Home" }, + { "action", "Index" } + }, + } + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + // Example: In conventional route, one could set non-inline defaults + // new { area = (string)null, controller = "Home", action = "Index" } + routeContext.RouteData.Values.Add("foo", null); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + var action = Assert.Single(candidates); + Assert.Same(actions[0], action); + } + [Fact] public void SelectBestCandidate_AmbiguousActions_LogIsCorrect() {