From 4f3e0966d99d110bd4dae997fd66e10ab4c6943f Mon Sep 17 00:00:00 2001 From: Artak Mkrtchyan Date: Tue, 23 Jan 2018 19:16:24 -0800 Subject: [PATCH 1/2] Enabling antiforgery tests, which were disabled because of the #7040 issue --- build/dependencies.props | 132 +++++++++--------- .../AntiforgeryTests.cs | 4 +- 2 files changed, 68 insertions(+), 68 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 97a62c0e3b..532d7e8d62 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -5,80 +5,80 @@ 0.10.11 2.1.0-preview1-15679 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 5.2.4-preview1 2.6.1 2.6.1 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 2.1.0-preview2-25711-01 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 - 2.1.0-preview1-28153 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 + 2.1.0-preview1-28157 2.0.0 - 2.1.0-preview1-26115-03 - 2.1.0-preview1-28153 + 2.1.0-preview1-26122-01 + 2.1.0-preview1-28157 15.3.0 4.7.49 1.0.1 - 4.5.0-preview1-26112-01 - 4.5.0-preview1-26112-01 - 4.5.0-preview1-26112-01 + 4.5.0-preview1-26119-06 + 4.5.0-preview1-26119-06 + 4.5.0-preview1-26119-06 0.8.0 2.3.1 2.3.1 diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryTests.cs index b195a9ee04..43967aca1f 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/AntiforgeryTests.cs @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("OK", await response.Content.ReadAsStringAsync()); } - [Fact(Skip = "https://github.com/aspnet/Mvc/issues/7040")] + [Fact] public async Task SetCookieAndHeaderBeforeFlushAsync_GeneratesCookieTokenAndHeader() { // Arrange & Act @@ -94,7 +94,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("no-cache", pragmaValue.Name); } - [Fact(Skip = "https://github.com/aspnet/Mvc/issues/7040")] + [Fact] public async Task SetCookieAndHeaderBeforeFlushAsync_PostToForm() { // Arrange From a74ef9dfd9487b81a46c6191aa750b109f9f5cad Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 23 Jan 2018 08:56:13 -0800 Subject: [PATCH 2/2] Error message for [ApiController] without attribute route needs to be better Fixes #7277 --- build/dependencies.props | 1 + .../ApplicationModels/ActionModel.cs | 18 +++++++++++++++++- .../ApplicationModels/ControllerModel.cs | 13 ++++++++++++- .../Controllers/ControllerActionDescriptor.cs | 3 ++- .../ApiBehaviorApplicationModelProvider.cs | 5 ++++- .../DefaultApplicationModelProvider.cs | 9 ++++----- .../Microsoft.AspNetCore.Mvc.Core.csproj | 1 + .../Properties/Resources.Designer.cs | 8 ++++---- .../Resources.resx | 4 ++-- .../ApplicationModel/ActionModelTest.cs | 7 ++++++- .../ApplicationModel/ControllerModelTest.cs | 5 +++++ .../ApiBehaviorApplicationModelProviderTest.cs | 6 +++--- 12 files changed, 61 insertions(+), 19 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index 532d7e8d62..46213d88b4 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -69,6 +69,7 @@ 2.1.0-preview1-28157 2.1.0-preview1-28157 2.1.0-preview1-28157 + 2.1.0-preview1-28157 2.1.0-preview1-28157 2.0.0 2.1.0-preview1-26122-01 diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs index eaea73d612..6114bc227f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionModel.cs @@ -9,10 +9,11 @@ using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ApplicationModels { - [DebuggerDisplay("{Controller.ControllerType.Name}.{ActionMethod.Name}")] + [DebuggerDisplay("{DisplayName}")] public class ActionModel : ICommonModel, IFilterModel, IApiExplorerModel { public ActionModel( @@ -123,5 +124,20 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels string ICommonModel.Name => ActionName; public IList Selectors { get; } + + public string DisplayName + { + get + { + if (Controller == null) + { + return ActionMethod.Name; + } + + var controllerType = TypeNameHelper.GetTypeDisplayName(Controller.ControllerType); + var controllerAssembly = Controller?.ControllerType.Assembly.GetName().Name; + return $"{controllerType}.{ActionMethod.Name} ({controllerAssembly})"; + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerModel.cs index 146fade19c..cd26b0e26d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerModel.cs @@ -8,10 +8,11 @@ using System.Linq; using System.Reflection; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ApplicationModels { - [DebuggerDisplay("Name={ControllerName}, Type={ControllerType.Name}")] + [DebuggerDisplay("{DisplayName}")] public class ControllerModel : ICommonModel, IFilterModel, IApiExplorerModel { public ControllerModel( @@ -118,5 +119,15 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels public IDictionary Properties { get; } public IList Selectors { get; } + + public string DisplayName + { + get + { + var controllerType = TypeNameHelper.GetTypeDisplayName(ControllerType); + var controllerAssembly = ControllerType.Assembly.GetName().Name; + return $"{controllerType} ({controllerAssembly})"; + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Controllers/ControllerActionDescriptor.cs b/src/Microsoft.AspNetCore.Mvc.Core/Controllers/ControllerActionDescriptor.cs index 7292139d1a..3cee82963a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Controllers/ControllerActionDescriptor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Controllers/ControllerActionDescriptor.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.Globalization; using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.Controllers { @@ -29,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers base.DisplayName = string.Format( CultureInfo.InvariantCulture, "{0}.{1} ({2})", - ControllerTypeInfo.FullName, + TypeNameHelper.GetTypeDisplayName(ControllerTypeInfo), MethodInfo.Name, ControllerTypeInfo.Assembly.GetName().Name); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index 2d6fcf9dff..d645da1546 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -114,7 +114,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal if (!controllerHasSelectorModel && !actionModel.Selectors.Any(s => s.AttributeRouteModel != null)) { // Require attribute routing with controllers annotated with ApiControllerAttribute - throw new InvalidOperationException(Resources.FormatApiController_AttributeRouteRequired(nameof(ApiControllerAttribute))); + var message = Resources.FormatApiController_AttributeRouteRequired( + actionModel.DisplayName, + nameof(ApiControllerAttribute)); + throw new InvalidOperationException(message); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs index b610bbd8f2..b9a3e830c7 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs @@ -298,7 +298,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal actionModel.RouteValues.Add(routeValueProvider.RouteKey, routeValueProvider.RouteValue); } - //TODO: modify comment // Now we need to determine the action selection info (cross-section of routes and constraints) // // For attribute routes on a action, we want to support 'overriding' routes on a @@ -313,9 +312,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal while (true) { routeAttributes = currentMethodInfo - .GetCustomAttributes(inherit: false) - .OfType() - .ToArray(); + .GetCustomAttributes(inherit: false) + .OfType() + .ToArray(); if (routeAttributes.Length > 0) { @@ -679,4 +678,4 @@ namespace Microsoft.AspNetCore.Mvc.Internal typeof(IEnumerable).IsAssignableFrom(parameterType); } } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj b/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj index 5861db1584..c858003fff 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj +++ b/src/Microsoft.AspNetCore.Mvc.Core/Microsoft.AspNetCore.Mvc.Core.csproj @@ -37,6 +37,7 @@ Microsoft.AspNetCore.Mvc.RouteAttribute + diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs index 2c4d96c4aa..93109738dd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs @@ -1341,7 +1341,7 @@ namespace Microsoft.AspNetCore.Mvc.Core => GetString("ValidationProblemDescription_Title"); /// - /// Action methods on controllers annotated with {0} must have an attribute route. + /// Action '{0}' does not have an attribute route. Action methods on controllers annotated with {1} must be attribute routed. /// internal static string ApiController_AttributeRouteRequired { @@ -1349,10 +1349,10 @@ namespace Microsoft.AspNetCore.Mvc.Core } /// - /// Action methods on controllers annotated with {0} must have an attribute route. + /// Action '{0}' does not have an attribute route. Action methods on controllers annotated with {1} must be attribute routed. /// - internal static string FormatApiController_AttributeRouteRequired(object p0) - => string.Format(CultureInfo.CurrentCulture, GetString("ApiController_AttributeRouteRequired"), p0); + internal static string FormatApiController_AttributeRouteRequired(object p0, object p1) + => string.Format(CultureInfo.CurrentCulture, GetString("ApiController_AttributeRouteRequired"), p0, p1); /// /// No file provider has been configured to process the supplied file. diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx index a607e28eff..d43e07dae3 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNetCore.Mvc.Core/Resources.resx @@ -416,9 +416,9 @@ One or more validation errors occurred. - Action methods on controllers annotated with {0} must have an attribute route. + Action '{0}' does not have an attribute route. Action methods on controllers annotated with {1} must be attribute routed. No file provider has been configured to process the supplied file. - \ No newline at end of file + diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs index 6ce268f676..8a43002db1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ActionModelTest.cs @@ -125,6 +125,11 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // Ensure non-default value Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); } + else if (property.Name.Equals(nameof(ActionModel.DisplayName))) + { + // DisplayName is re-calculated, hence reference equality wouldn't work. + Assert.Equal(value1, value2); + } else { Assert.Same(value1, value2); @@ -153,4 +158,4 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels public string RouteValue { get; set; } } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs index 08139d900c..dc19724cd6 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ControllerModelTest.cs @@ -132,6 +132,11 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // Ensure non-default value Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType)); } + else if (property.Name.Equals(nameof(ControllerModel.DisplayName))) + { + // DisplayName is re-calculated, hence reference equality wouldn't work. + Assert.Equal(value1, value2); + } else { Assert.Same(value1, value2); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index aeb6179718..88664b689a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -148,14 +148,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal public void OnProvidersExecuting_ThrowsIfControllerWithAttribute_HasActionsWithoutAttributeRouting() { // Arrange + var actionName = $"{typeof(ActionsWithoutAttributeRouting).FullName}.{nameof(ActionsWithoutAttributeRouting.Index)} ({typeof(ActionsWithoutAttributeRouting).Assembly.GetName().Name})"; + var expected = $"Action '{actionName}' does not have an attribute route. Action methods on controllers annotated with ApiControllerAttribute must be attribute routed."; var context = GetContext(typeof(ActionsWithoutAttributeRouting)); var provider = GetProvider(); // Act & Assert var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); - Assert.Equal( - "Action methods on controllers annotated with ApiControllerAttribute must have an attribute route.", - ex.Message); + Assert.Equal(expected, ex.Message); } [Fact]