diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs index 4b0a25f09c..56c0d95e15 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs @@ -34,8 +34,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal { var actions = new List(); - var routeValueKeys = new HashSet(StringComparer.OrdinalIgnoreCase); - var methodInfoMap = new MethodToActionMap(); var routeTemplateErrors = new List(); @@ -64,7 +62,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal actionDescriptor.ControllerTypeInfo = controller.ControllerType; AddApiExplorerInfo(actionDescriptor, application, controller, action); - AddRouteValues(routeValueKeys, actionDescriptor, controller, action); + AddRouteValues(actionDescriptor, controller, action); AddProperties(actionDescriptor, action, controller, application); actionDescriptor.BoundProperties = controllerPropertyDescriptors; @@ -108,12 +106,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal // attribute routes with a given name have the same template. AddActionToNamedGroup(actionsByRouteName, attributeRouteInfo.Name, actionDescriptor); } - - // Add a route value with 'null' for each user-defined route value in the set to all the - // actions that don't have that value. For example, if a controller defines - // an area, all actions that don't belong to an area must have a route - // value that prevents them from matching an incoming request when area is specified. - AddGlobalRouteValues(actionDescriptor, routeValueKeys); } if (attributeRoutingConfigurationErrors.Any()) @@ -409,7 +401,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } public static void AddRouteValues( - ISet keys, ControllerActionDescriptor actionDescriptor, ControllerModel controller, ActionModel action) @@ -421,8 +412,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal // generating a link. foreach (var kvp in action.RouteValues) { - keys.Add(kvp.Key); - // Skip duplicates if (!actionDescriptor.RouteValues.ContainsKey(kvp.Key)) { @@ -432,8 +421,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal foreach (var kvp in controller.RouteValues) { - keys.Add(kvp.Key); - // Skip duplicates - this also means that a value on the action will take precedence if (!actionDescriptor.RouteValues.ContainsKey(kvp.Key)) { @@ -483,19 +470,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - private static void AddGlobalRouteValues( - ControllerActionDescriptor actionDescriptor, - ISet removalConstraints) - { - foreach (var key in removalConstraints) - { - if (!actionDescriptor.RouteValues.ContainsKey(key)) - { - actionDescriptor.RouteValues.Add(key, string.Empty); - } - } - } - private static void AddActionToNamedGroup( IDictionary> actionsByRouteName, string routeName, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorProvider.cs index f6706effe1..476e2a14e6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorProvider.cs @@ -66,6 +66,36 @@ namespace Microsoft.AspNetCore.Mvc.Internal /// public void OnProvidersExecuted(ActionDescriptorProviderContext context) { + // After all of the providers have run, we need to provide a 'null' for each all of route values that + // participate in action selection. + // + // This is important for scenarios like Razor Pages, that use the 'page' route value. An action that + // uses 'page' shouldn't match when 'action' is set, and an action that uses 'action' shouldn't match when + // 'page is specified. + // + // Or for another example, consider areas. A controller that's not in an area needs a 'null' value for + // area so it can't match when the route produces an 'area' value. + var keys = new HashSet(StringComparer.OrdinalIgnoreCase); + for (var i = 0; i < context.Results.Count; i++) + { + var action = context.Results[i]; + foreach (var key in action.RouteValues.Keys) + { + keys.Add(key); + } + } + + for (var i = 0; i < context.Results.Count; i++) + { + var action = context.Results[i]; + foreach (var key in keys) + { + if (!action.RouteValues.ContainsKey(key)) + { + action.RouteValues.Add(key, null); + } + } + } } internal protected IEnumerable GetDescriptors() diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs index c99f472f98..04be8a1803 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Infrastructure/PageActionDescriptorProvider.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure _pagesOptions = pagesOptionsAccessor.Value; } - public int Order { get; set; } + public int Order { get; set; } = -900; // Run after the default MVC provider, but before others. public void OnProvidersExecuting(ActionDescriptorProviderContext context) { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs index 5a6302c484..15ebb8d8dc 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs @@ -277,11 +277,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal var descriptorWithoutValue = Assert.Single( descriptors, - ad => ad.RouteValues.Any(kvp => kvp.Key == "key" && string.IsNullOrEmpty(kvp.Value))); + ad => !ad.RouteValues.ContainsKey("key")); var descriptorWithValue = Assert.Single( descriptors, - ad => ad.RouteValues.Any(kvp => kvp.Key == "key" && kvp.Value == "value")); + ad => ad.RouteValues.ContainsKey("key")); // Assert Assert.Equal(2, descriptors.Length); @@ -303,7 +303,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal c.Key == "key" && c.Value == "value"); - Assert.Equal(3, descriptorWithoutValue.RouteValues.Count); + Assert.Equal(2, descriptorWithoutValue.RouteValues.Count); Assert.Single( descriptorWithoutValue.RouteValues, c => @@ -314,11 +314,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal c => c.Key == "action" && c.Value == "OnlyPost"); - Assert.Single( - descriptorWithoutValue.RouteValues, - c => - c.Key == "key" && - c.Value == string.Empty); } [Fact] @@ -932,7 +927,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var indexAction = Assert.Single(actionDescriptors, ad => ad.ActionName.Equals("Index")); - Assert.Equal(5, indexAction.RouteValues.Count); + Assert.Equal(3, indexAction.RouteValues.Count); var controllerDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("controller", StringComparison.OrdinalIgnoreCase)); Assert.Equal("ConventionalAndAttributeRoutedActionsWithArea", controllerDefault.Value); @@ -942,12 +937,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal var areaDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("area", StringComparison.OrdinalIgnoreCase)); Assert.Equal("Home", areaDefault.Value); - - var mvRouteValueDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("key", StringComparison.OrdinalIgnoreCase)); - Assert.Equal(string.Empty, mvRouteValueDefault.Value); - - var anotherRouteValue = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("second", StringComparison.OrdinalIgnoreCase)); - Assert.Equal(string.Empty, anotherRouteValue.Value); } [Fact] @@ -1355,6 +1344,42 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Single(action.ActionConstraints, a => a is ConstraintAttribute); } + [Fact] + public void OnProviderExecuted_AddsGlobalRouteValues() + { + // Arrange + var context = new ActionDescriptorProviderContext(); + context.Results.Add(new ActionDescriptor() + { + RouteValues = new Dictionary() + { + { "controller", "Home" }, + { "action", "Index" }, + } + }); + context.Results.Add(new ActionDescriptor() + { + RouteValues = new Dictionary() + { + { "page", "/Some/Page" } + } + }); + + var provider = GetProvider(); + + // Act + provider.OnProvidersExecuted(context); + + // Assert + Assert.True(context.Results[0].RouteValues.ContainsKey("page")); + Assert.Null(context.Results[0].RouteValues["page"]); + + Assert.True(context.Results[1].RouteValues.ContainsKey("controller")); + Assert.Null(context.Results[1].RouteValues["controller"]); + Assert.True(context.Results[1].RouteValues.ContainsKey("action")); + Assert.Null(context.Results[1].RouteValues["action"]); + } + private ControllerActionDescriptorProvider GetProvider( TypeInfo controllerTypeInfo, IEnumerable filters = null) diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs index 8ca4d70e11..0def6c5d57 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs @@ -791,6 +791,34 @@ Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary`1[AspNetCore._InjectedP Assert.Equal(expected, response.Headers.Location.ToString()); } + [Fact] + public async Task Controller_RedirectToPage() + { + // Arrange + var expected = "/RedirectToController?param=17"; + + // Act + var response = await Client.GetAsync("/RedirectToPage"); + + // Assert + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal(expected, response.Headers.Location.ToString()); + } + + [Fact] + public async Task Page_RedirectToController() + { + // Arrange + var expected = "/RedirectToPage?param=92"; + + // Act + var response = await Client.GetAsync("/RedirectToController"); + + // Assert + Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); + Assert.Equal(expected, response.Headers.Location.ToString()); + } + private async Task AddAntiforgeryHeaders(HttpRequestMessage request) { var getResponse = await Client.GetAsync(request.RequestUri); diff --git a/test/WebSites/RazorPagesWebSite/Controllers/RedirectController.cs b/test/WebSites/RazorPagesWebSite/Controllers/RedirectController.cs new file mode 100644 index 0000000000..073a63a85b --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Controllers/RedirectController.cs @@ -0,0 +1,16 @@ +// 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 RazorPagesWebSite +{ + public class RedirectController : Controller + { + [HttpGet("/RedirectToPage")] + public IActionResult RedirectToPage() + { + return RedirectToRoute(new { page = "/RedirectToController", param = 17 }); + } + } +} diff --git a/test/WebSites/RazorPagesWebSite/RedirectToController.cshtml b/test/WebSites/RazorPagesWebSite/RedirectToController.cshtml new file mode 100644 index 0000000000..cd4f5dcb15 --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/RedirectToController.cshtml @@ -0,0 +1,8 @@ +@page + +@functions { + public IActionResult OnGet() + { + return new RedirectToRouteResult(new { controller = "Redirect", action = "RedirectToPage", param = 92 }); + } +} \ No newline at end of file