From 7aa5967cd40ea6659d7084c4f34e0bc901109b6f Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 13 Aug 2015 12:34:26 -0700 Subject: [PATCH] Fix #2859 - Correct UrlHelper for special tokens This change restores a link generation behavior from MVC5 and earlier where 'action' and 'controller' values are special cased-when using Url.Action(...). The change is that in-effect 'action' and 'controller' are always included in the route values given to the routing system. Passing a null value into the Url.Action(...) method means that the ambient value for that token should be used explicitly. This means that the 'action' and 'controller' tokens become sticky, even when something to the lexical left in the URL (like area) changes. --- src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs | 22 ++- .../UrlHelperTest.cs | 154 ++++++++++++++++++ 2 files changed, 174 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs b/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs index b379f9f10e..c9118f1b2a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs @@ -48,12 +48,30 @@ namespace Microsoft.AspNet.Mvc { var valuesDictionary = PropertyHelper.ObjectToDictionary(actionContext.Values); - if (actionContext.Action != null) + if (actionContext.Action == null) + { + object action; + if (!valuesDictionary.ContainsKey("action") && + AmbientValues.TryGetValue("action", out action)) + { + valuesDictionary["action"] = action; + } + } + else { valuesDictionary["action"] = actionContext.Action; } - if (actionContext.Controller != null) + if (actionContext.Controller == null) + { + object controller; + if (!valuesDictionary.ContainsKey("controller") && + AmbientValues.TryGetValue("controller", out controller)) + { + valuesDictionary["controller"] = controller; + } + } + else { valuesDictionary["controller"] = actionContext.Controller; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/UrlHelperTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/UrlHelperTest.cs index 1a3b718915..714ffd9975 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/UrlHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/UrlHelperTest.cs @@ -3,11 +3,13 @@ using System; using System.Collections.Generic; +using System.Threading.Tasks; using Microsoft.AspNet.Builder; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.Internal; using Microsoft.AspNet.Routing; +using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.Logging; using Microsoft.Framework.Logging.Testing; using Microsoft.Framework.OptionsModel; @@ -807,6 +809,120 @@ namespace Microsoft.AspNet.Mvc Assert.Equal("https://myhost/named/home/newaction/someid", url); } + // Regression test for aspnet/Mvc#2859 + [Fact] + public void Action_RouteValueInvalidation_DoesNotAffectActionAndController() + { + // Arrage + var services = GetServices(); + var routeBuilder = new RouteBuilder() + { + DefaultHandler = new PassThroughRouter(), + ServiceProvider = services, + }; + + routeBuilder.MapRoute( + "default", + "{first}/{controller}/{action}", + new { second = "default", controller = "default", action = "default" }); + + var actionContext = services.GetService().ActionContext; + actionContext.RouteData.Values.Add("first", "a"); + actionContext.RouteData.Values.Add("controller", "Store"); + actionContext.RouteData.Values.Add("action", "Buy"); + actionContext.RouteData.Routers.Add(routeBuilder.Build()); + + var urlHelper = CreateUrlHelper(services); + + // Act + // + // In this test the 'first' route value has changed, meaning that *normally* the + // 'controller' value could not be used. However 'controller' and 'action' are treated + // specially by UrlHelper. + var url = urlHelper.Action("Checkout", new { first = "b" }); + + // Assert + Assert.NotNull(url); + Assert.Equal("/b/Store/Checkout", url); + } + + // Regression test for aspnet/Mvc#2859 + [Fact] + public void Action_RouteValueInvalidation_AffectsOtherRouteValues() + { + // Arrage + var services = GetServices(); + var routeBuilder = new RouteBuilder() + { + DefaultHandler = new PassThroughRouter(), + ServiceProvider = services, + }; + + routeBuilder.MapRoute( + "default", + "{first}/{second}/{controller}/{action}", + new { second = "default", controller = "default", action = "default" }); + + var actionContext = services.GetService().ActionContext; + actionContext.RouteData.Values.Add("first", "a"); + actionContext.RouteData.Values.Add("second", "x"); + actionContext.RouteData.Values.Add("controller", "Store"); + actionContext.RouteData.Values.Add("action", "Buy"); + actionContext.RouteData.Routers.Add(routeBuilder.Build()); + + var urlHelper = CreateUrlHelper(services); + + // Act + // + // In this test the 'first' route value has changed, meaning that *normally* the + // 'controller' value could not be used. However 'controller' and 'action' are treated + // specially by UrlHelper. + // + // 'second' gets no special treatment, and picks up its default value instead. + var url = urlHelper.Action("Checkout", new { first = "b" }); + + // Assert + Assert.NotNull(url); + Assert.Equal("/b/default/Store/Checkout", url); + } + + // Regression test for aspnet/Mvc#2859 + [Fact] + public void Action_RouteValueInvalidation_DoesNotAffectActionAndController_ActionPassedInRouteValues() + { + // Arrage + var services = GetServices(); + var routeBuilder = new RouteBuilder() + { + DefaultHandler = new PassThroughRouter(), + ServiceProvider = services, + }; + + routeBuilder.MapRoute( + "default", + "{first}/{controller}/{action}", + new { second = "default", controller = "default", action = "default" }); + + var actionContext = services.GetService().ActionContext; + actionContext.RouteData.Values.Add("first", "a"); + actionContext.RouteData.Values.Add("controller", "Store"); + actionContext.RouteData.Values.Add("action", "Buy"); + actionContext.RouteData.Routers.Add(routeBuilder.Build()); + + var urlHelper = CreateUrlHelper(services); + + // Act + // + // In this test the 'first' route value has changed, meaning that *normally* the + // 'controller' value could not be used. However 'controller' and 'action' are treated + // specially by UrlHelper. + var url = urlHelper.Action(action: null, values: new { first = "b", action = "Checkout" }); + + // Assert + Assert.NotNull(url); + Assert.Equal("/b/Store/Checkout", url); + } + private static HttpContext CreateHttpContext( IServiceProvider services, string appRoot) @@ -844,6 +960,14 @@ namespace Microsoft.AspNet.Mvc return new UrlHelper(actionContext, actionSelector.Object); } + private static UrlHelper CreateUrlHelper(IServiceProvider services) + { + var actionSelector = new Mock(MockBehavior.Strict); + return new UrlHelper( + services.GetRequiredService(), + actionSelector.Object); + } + private static UrlHelper CreateUrlHelper(string host) { var services = GetServices(); @@ -916,6 +1040,21 @@ namespace Microsoft.AspNet.Mvc .Setup(s => s.GetService(typeof(ILoggerFactory))) .Returns(NullLoggerFactory.Instance); + services + .Setup(s => s.GetService(typeof(IActionContextAccessor))) + .Returns(new ActionContextAccessor() + { + ActionContext = new ActionContext() + { + HttpContext = new DefaultHttpContext() + { + ApplicationServices = services.Object, + RequestServices = services.Object, + }, + RouteData = new RouteData(), + }, + }); + return services.Object; } @@ -952,5 +1091,20 @@ namespace Microsoft.AspNet.Mvc routeBuilder.Routes.Add(mockHttpRoute.Object); return routeBuilder.Build(); } + + private class PassThroughRouter : IRouter + { + public VirtualPathData GetVirtualPath(VirtualPathContext context) + { + context.IsBound = true; + return null; + } + + public Task RouteAsync(RouteContext context) + { + context.IsHandled = true; + return Task.FromResult(false); + } + } } } \ No newline at end of file