From 5e245da3264835545bf577af10caf0f82eee12c3 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Fri, 23 Mar 2018 16:09:53 -0700 Subject: [PATCH] Add compatibility switch controlling parameter metadata and top-level validation - #7413 part 1 of 2 - made all `ModelMetadataProvider` and `ObjectModelValidator`-specific code conditional - fortunately, `MvcOptions` easy to get; affected code is primarily `internal` or pub-`Internal` - remove unnecessary `ModelMetadataProvider` use in `ApiBehaviorApplicationModelProvider` - run integration and functional tests with `CompatibilityVersion.Version_2_1` - functional test change depends on @javiercn's recent #7541 fix - remove test code now redundantly turning compatibility switches on nits: - correct spelling errors in `CompatibilitySwitch` - take VS suggestions, mostly in test code - rename methods in `ControllerBinderDelegateProviderTest` to match current API - slightly refactor in `ApiBehaviorApplicationModelProvider` --- .../DefaultApiDescriptionProvider.cs | 17 +- .../Infrastructure/CompatibilitySwitch.cs | 30 +-- ...MvcOptionsConfigureCompatibilityOptions.cs | 3 +- .../ApiBehaviorApplicationModelProvider.cs | 48 ++--- .../Internal/ControllerActionInvokerCache.cs | 14 +- .../ControllerBinderDelegateProvider.cs | 27 ++- .../ModelBinding/ObjectModelValidator.cs | 2 +- .../ModelBinding/ParameterBinder.cs | 53 +++-- .../MvcOptions.cs | 62 ++++-- .../Internal/PageActionInvokerProvider.cs | 6 +- .../Internal/PageBinderFactory.cs | 6 +- .../DefaultApiDescriptionProviderTest.cs | 91 +++++--- ...ApiBehaviorApplicationModelProviderTest.cs | 5 +- .../ControllerActionInvokerCacheTest.cs | 9 +- .../ControllerBinderDelegateProviderTest.cs | 199 +++++++++++++----- .../Internal/MiddlewareFilterTest.cs | 20 +- .../ModelBinding/ParameterBinderTest.cs | 97 ++++++++- .../CombineAuthorizeTests.cs | 43 ---- .../GlobalAuthorizationFilterTest.cs | 50 ++++- .../Infrastructure/MvcTestFixture.cs | 14 +- .../InputFormatterTests.cs | 4 +- .../BodyValidationIntegrationTests.cs | 54 +++-- .../ComplexTypeModelBinderIntegrationTest.cs | 2 +- .../HeaderModelBinderIntegrationTest.cs | 10 +- .../ModelBindingTestHelper.cs | 6 +- .../Internal/PageActionInvokerProviderTest.cs | 6 + .../Internal/PageActionInvokerTest.cs | 7 + .../Internal/PageBinderFactoryTest.cs | 83 +++++++- test/WebSites/RazorPagesWebSite/Startup.cs | 3 +- .../RazorPagesWebSite/StartupWithBasePath.cs | 5 +- ...horizeAndAllowCombiningAuthorizeFilters.cs | 47 ----- 31 files changed, 672 insertions(+), 351 deletions(-) delete mode 100644 test/Microsoft.AspNetCore.Mvc.FunctionalTests/CombineAuthorizeTests.cs delete mode 100644 test/WebSites/SecurityWebSite/StartupWithGlobalAuthorizeAndAllowCombiningAuthorizeFilters.cs diff --git a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs index ede016465c..56f340b9d0 100644 --- a/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ApiExplorer/DefaultApiDescriptionProvider.cs @@ -24,10 +24,9 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer /// public class DefaultApiDescriptionProvider : IApiDescriptionProvider { - private readonly IList _inputFormatters; - private readonly IList _outputFormatters; - private readonly IModelMetadataProvider _modelMetadataProvider; + private readonly MvcOptions _mvcOptions; private readonly IInlineConstraintResolver _constraintResolver; + private readonly IModelMetadataProvider _modelMetadataProvider; /// /// Creates a new instance of . @@ -41,8 +40,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer IInlineConstraintResolver constraintResolver, IModelMetadataProvider modelMetadataProvider) { - _inputFormatters = optionsAccessor.Value.InputFormatters; - _outputFormatters = optionsAccessor.Value.OutputFormatters; + _mvcOptions = optionsAccessor.Value; _constraintResolver = constraintResolver; _modelMetadataProvider = modelMetadataProvider; } @@ -164,7 +162,8 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer var visitor = new PseudoModelBindingVisitor(context, actionParameter); ModelMetadata metadata = null; - if (actionParameter is ControllerParameterDescriptor controllerParameterDescriptor && + if (_mvcOptions.AllowValidatingTopLevelNodes && + actionParameter is ControllerParameterDescriptor controllerParameterDescriptor && _modelMetadataProvider is ModelMetadataProvider provider) { // The default model metadata provider derives from ModelMetadataProvider @@ -344,7 +343,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer var results = new List(); foreach (var contentType in contentTypes) { - foreach (var formatter in _inputFormatters) + foreach (var formatter in _mvcOptions.InputFormatters) { if (formatter is IApiRequestFormatMetadataProvider requestFormatMetadataProvider) { @@ -422,7 +421,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer contentTypes.Add((string)null); } - var responseTypeMetadataProviders = _outputFormatters.OfType(); + var responseTypeMetadataProviders = _mvcOptions.OutputFormatters.OfType(); foreach (var objectType in objectTypes) { @@ -779,4 +778,4 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer } } } -} \ No newline at end of file +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs index 59f650038a..0c070ecbe4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/CompatibilitySwitch.cs @@ -17,16 +17,16 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure // behaviors in a minor release. // // To make things better for future users, we also want to provide an easy way for applications to get - // access to the new behaviors. We make changes when they are improvments, and if we're changing something + // access to the new behaviors. We make changes when they are improvements, and if we're changing something // we've already shipped, it must add value for all of our users (eventually). To this end, new applications // created using the template are always opted in to the 'current' version. // - // This means that all changes in behavior should be opt-in. + // This means that all changes in behavior should be opt-in. // // ----- - // - // Moving on from general philosophy, here's how to implement a behavior change and corresponding - // compatibility switch. + // + // Moving on from general philosophy, here's how to implement a behavior change and corresponding + // compatibility switch. // // Add a new property on options that uses a CompatibilitySwitch as a backing field. Make sure the // new switch is exposed by implementing IEnumerable on the options class. Pass the @@ -34,33 +34,33 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure // // Choose a boolean value or a new enum type as the 'value' of the property. // - // If the new property has a boolean value, it should be named something like `SuppressFoo` + // If the new property has a boolean value, it should be named something like `SuppressFoo` // (if the new value deactivates some behavior) or like `AllowFoo` (if the new value enables some behavior). // Choose a name so that the old behavior equates to 'false'. // - // If it's an enum, make sure you initialize the compatibility switch using the - // CompatibilitySwitch(string, value) constructor to make it obvious the correct value is passed in. It's + // If it's an enum, make sure you initialize the compatibility switch using the + // CompatibilitySwitch(string, value) constructor to make it obvious the correct value is passed in. It's // a good idea to equate the original behavior with the default enum value as well. // - // Then create (or modify) a subclass of ConfigureCompatibilityOptions appropriate for your options type. + // Then create (or modify) a subclass of ConfigureCompatibilityOptions appropriate for your options type. // Override the DefaultValues property and provide appropriate values based on the value of the Version // property. If you just added this class, register it as an IPostConfigureOptions in DI. // /// /// Infrastructure supporting the implementation of . This is an - /// implementation of suitible for use with the + /// implementation of suitable for use with the /// pattern. This is framework infrastructure and should not be used by application code. /// - /// The type of value assoicated with the compatibility switch. + /// The type of value associated with the compatibility switch. public class CompatibilitySwitch : ICompatibilitySwitch where TValue : struct { private TValue _value; /// - /// Creates a new compatiblity switch with the provided name. + /// Creates a new compatibility switch with the provided name. /// /// - /// The compatiblity switch name. The name must match a property name on an options type. + /// The compatibility switch name. The name must match a property name on an options type. /// public CompatibilitySwitch(string name) : this(name, default) @@ -68,10 +68,10 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure } /// - /// Creates a new compatiblity switch with the provided name and initial value. + /// Creates a new compatibility switch with the provided name and initial value. /// /// - /// The compatiblity switch name. The name must match a property name on an options type. + /// The compatibility switch name. The name must match a property name on an options type. /// /// /// The initial value to assign to the switch. diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs index e6b95f6b13..71957b40cf 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { public MvcOptionsConfigureCompatibilityOptions( ILoggerFactory loggerFactory, - IOptions compatibilityOptions) + IOptions compatibilityOptions) : base(loggerFactory, compatibilityOptions) { } @@ -27,6 +27,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { values[nameof(MvcOptions.AllowCombiningAuthorizeFilters)] = true; values[nameof(MvcOptions.AllowBindingHeaderValuesToNonStringModelTypes)] = true; + values[nameof(MvcOptions.AllowValidatingTopLevelNodes)] = true; values[nameof(MvcOptions.InputFormatterExceptionPolicy)] = InputFormatterExceptionPolicy.MalformedInputExceptions; values[nameof(MvcOptions.SuppressBindingUndefinedValueToEnumType)] = true; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index 3e44c3b9a8..e91b46c995 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -210,22 +210,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } } - + // internal for testing internal void InferParameterModelPrefixes(ActionModel actionModel) { foreach (var parameter in actionModel.Parameters) { - if (parameter.BindingInfo != null && - parameter.BindingInfo.BinderModelName == null && - parameter.BindingInfo.BindingSource != null && - !parameter.BindingInfo.BindingSource.IsGreedy) + var bindingInfo = parameter.BindingInfo; + if (bindingInfo?.BindingSource != null && + bindingInfo.BinderModelName == null && + !bindingInfo.BindingSource.IsGreedy && + IsComplexTypeParameter(parameter)) { - var metadata = GetParameterMetadata(parameter); - if (metadata.IsComplexType) - { - parameter.BindingInfo.BinderModelName = string.Empty; - } + parameter.BindingInfo.BinderModelName = string.Empty; } } } @@ -233,25 +230,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Internal for unit testing. internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { - var parameterType = parameter.ParameterInfo.ParameterType; if (ParameterExistsInAllRoutes(parameter.Action, parameter.ParameterName)) { return BindingSource.Path; } - else - { - var parameterMetadata = GetParameterMetadata(parameter); - if (parameterMetadata != null) - { - var bindingSource = parameterMetadata.IsComplexType ? - BindingSource.Body : - BindingSource.Query; - return bindingSource; - } - } + var bindingSource = IsComplexTypeParameter(parameter) ? + BindingSource.Body : + BindingSource.Query; - return null; + return bindingSource; } private bool ParameterExistsInAllRoutes(ActionModel actionModel, string parameterName) @@ -277,16 +265,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal return parameterExistsInSomeRoute; } - private ModelMetadata GetParameterMetadata(ParameterModel parameter) + private bool IsComplexTypeParameter(ParameterModel parameter) { - if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider) - { - return modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo); - } - else - { - return _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); - } + // No need for information from attributes on the parameter. Just use its type. + return _modelMetadataProvider + .GetMetadataForType(parameter.ParameterInfo.ParameterType) + .IsComplexType; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs index 8f012d0d10..0c6bd2adc1 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionInvokerCache.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Internal; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.Internal { @@ -21,7 +22,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal private readonly IModelMetadataProvider _modelMetadataProvider; private readonly IFilterProvider[] _filterProviders; private readonly IControllerFactoryProvider _controllerFactoryProvider; - + private readonly MvcOptions _mvcOptions; private volatile InnerCache _currentCache; public ControllerActionInvokerCache( @@ -30,7 +31,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal IModelBinderFactory modelBinderFactory, IModelMetadataProvider modelMetadataProvider, IEnumerable filterProviders, - IControllerFactoryProvider factoryProvider) + IControllerFactoryProvider factoryProvider, + IOptions mvcOptions) { _collectionProvider = collectionProvider; _parameterBinder = parameterBinder; @@ -38,6 +40,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal _modelMetadataProvider = modelMetadataProvider; _filterProviders = filterProviders.OrderBy(item => item.Order).ToArray(); _controllerFactoryProvider = factoryProvider; + _mvcOptions = mvcOptions.Value; } private InnerCache CurrentCache @@ -82,13 +85,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal _parameterBinder, _modelBinderFactory, _modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _mvcOptions); var actionMethodExecutor = ActionMethodExecutor.GetExecutor(objectMethodExecutor); cacheEntry = new ControllerActionInvokerCacheEntry( - filterFactoryResult.CacheableFilters, - controllerFactory, + filterFactoryResult.CacheableFilters, + controllerFactory, controllerReleaser, propertyBinderFactory, objectMethodExecutor, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs index a00265b24e..e29525ea11 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs @@ -16,13 +16,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal ParameterBinder parameterBinder, IModelBinderFactory modelBinderFactory, IModelMetadataProvider modelMetadataProvider, - ControllerActionDescriptor actionDescriptor) + ControllerActionDescriptor actionDescriptor, + MvcOptions mvcOptions) { if (parameterBinder == null) { throw new ArgumentNullException(nameof(parameterBinder)); } + if (modelBinderFactory == null) + { + throw new ArgumentNullException(nameof(modelBinderFactory)); + } + if (modelMetadataProvider == null) { throw new ArgumentNullException(nameof(modelMetadataProvider)); @@ -33,7 +39,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(actionDescriptor)); } - var parameterBindingInfo = GetParameterBindingInfo(modelBinderFactory, modelMetadataProvider, actionDescriptor); + if (mvcOptions == null) + { + throw new ArgumentNullException(nameof(mvcOptions)); + } + + var parameterBindingInfo = GetParameterBindingInfo( + modelBinderFactory, + modelMetadataProvider, + actionDescriptor, + mvcOptions); var propertyBindingInfo = GetPropertyBindingInfo(modelBinderFactory, modelMetadataProvider, actionDescriptor); if (parameterBindingInfo == null && propertyBindingInfo == null) @@ -104,7 +119,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static BinderItem[] GetParameterBindingInfo( IModelBinderFactory modelBinderFactory, IModelMetadataProvider modelMetadataProvider, - ControllerActionDescriptor actionDescriptor) + ControllerActionDescriptor actionDescriptor, + MvcOptions mvcOptions) { var parameters = actionDescriptor.Parameters; if (parameters.Count == 0) @@ -118,8 +134,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal var parameter = parameters[i]; ModelMetadata metadata; - if (modelMetadataProvider is ModelMetadataProvider modelMetadataProviderBase - && parameter is ControllerParameterDescriptor controllerParameterDescriptor) + if (mvcOptions.AllowValidatingTopLevelNodes && + modelMetadataProvider is ModelMetadataProvider modelMetadataProviderBase && + parameter is ControllerParameterDescriptor controllerParameterDescriptor) { // The default model metadata provider derives from ModelMetadataProvider // and can therefore supply information about attributes applied to parameters. diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ObjectModelValidator.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ObjectModelValidator.cs index ec17f06eb9..0b89489a5f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ObjectModelValidator.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ObjectModelValidator.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Initializes a new instance of . /// - /// The . + /// The . /// The list of . public ObjectModelValidator( IModelMetadataProvider modelMetadataProvider, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs index f7a15a8780..d78b1d1e13 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.ModelBinding { @@ -18,23 +19,31 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { private readonly IModelMetadataProvider _modelMetadataProvider; private readonly IModelBinderFactory _modelBinderFactory; + private readonly MvcOptions _mvcOptions; private readonly IObjectModelValidator _objectModelValidator; /// /// This constructor is obsolete and will be removed in a future version. The recommended alternative - /// is the overload that also takes an . + /// is the overload that also takes a accessor and an . + /// /// Initializes a new instance of . /// /// The . /// The . /// The . [Obsolete("This constructor is obsolete and will be removed in a future version. The recommended alternative" - + " is the overload that also takes an " + nameof(ILoggerFactory) + ".")] + + " is the overload that also takes a " + nameof(MvcOptions) + " accessor and an " + + nameof(ILoggerFactory) + " .")] public ParameterBinder( IModelMetadataProvider modelMetadataProvider, IModelBinderFactory modelBinderFactory, IObjectModelValidator validator) - : this(modelMetadataProvider, modelBinderFactory, validator, NullLoggerFactory.Instance) + : this( + modelMetadataProvider, + modelBinderFactory, + validator, + Options.Create(new MvcOptions()), + NullLoggerFactory.Instance) { } @@ -44,11 +53,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// The . /// The . /// The . + /// The accessor. /// The . public ParameterBinder( IModelMetadataProvider modelMetadataProvider, IModelBinderFactory modelBinderFactory, IObjectModelValidator validator, + IOptions mvcOptions, ILoggerFactory loggerFactory) { if (modelMetadataProvider == null) @@ -66,6 +77,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding throw new ArgumentNullException(nameof(validator)); } + if (mvcOptions == null) + { + throw new ArgumentNullException(nameof(mvcOptions)); + } + if (loggerFactory == null) { throw new ArgumentNullException(nameof(loggerFactory)); @@ -74,6 +90,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding _modelMetadataProvider = modelMetadataProvider; _modelBinderFactory = modelBinderFactory; _objectModelValidator = validator; + _mvcOptions = mvcOptions.Value; Logger = loggerFactory.CreateLogger(GetType()); } @@ -224,8 +241,21 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var modelBindingResult = modelBindingContext.Result; - var baseObjectValidator = _objectModelValidator as ObjectModelValidator; - if (baseObjectValidator == null) + if (_mvcOptions.AllowValidatingTopLevelNodes && + _objectModelValidator is ObjectModelValidator baseObjectValidator) + { + Logger.AttemptingToValidateParameterOrProperty(parameter, modelBindingContext); + + EnforceBindRequiredAndValidate( + baseObjectValidator, + actionContext, + metadata, + modelBindingContext, + modelBindingResult); + + Logger.DoneAttemptingToValidateParameterOrProperty(parameter, modelBindingContext); + } + else { // For legacy implementations (which directly implemented IObjectModelValidator), fall back to the // back-compatibility logic. In this scenario, top-level validation attributes will be ignored like @@ -239,19 +269,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding modelBindingResult.Model); } } - else - { - Logger.AttemptingToValidateParameterOrProperty(parameter, modelBindingContext); - - EnforceBindRequiredAndValidate( - baseObjectValidator, - actionContext, - metadata, - modelBindingContext, - modelBindingResult); - - Logger.DoneAttemptingToValidateParameterOrProperty(parameter, modelBindingContext); - } return modelBindingResult; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index bad17f6849..72634a49fa 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -27,6 +27,7 @@ namespace Microsoft.AspNetCore.Mvc // See CompatibilitySwitch.cs for guide on how to implement these. private readonly CompatibilitySwitch _allowBindingHeaderValuesToNonStringModelTypes; private readonly CompatibilitySwitch _allowCombiningAuthorizeFilters; + private readonly CompatibilitySwitch _allowValidatingTopLevelNodes; private readonly CompatibilitySwitch _inputFormatterExceptionPolicy; private readonly CompatibilitySwitch _suppressBindingUndefinedValueToEnumType; private readonly ICompatibilitySwitch[] _switches; @@ -50,6 +51,7 @@ namespace Microsoft.AspNetCore.Mvc _allowCombiningAuthorizeFilters = new CompatibilitySwitch(nameof(AllowCombiningAuthorizeFilters)); _allowBindingHeaderValuesToNonStringModelTypes = new CompatibilitySwitch(nameof(AllowBindingHeaderValuesToNonStringModelTypes)); + _allowValidatingTopLevelNodes = new CompatibilitySwitch(nameof(AllowValidatingTopLevelNodes)); _inputFormatterExceptionPolicy = new CompatibilitySwitch(nameof(InputFormatterExceptionPolicy), InputFormatterExceptionPolicy.AllExceptions); _suppressBindingUndefinedValueToEnumType = new CompatibilitySwitch(nameof(SuppressBindingUndefinedValueToEnumType)); @@ -57,6 +59,7 @@ namespace Microsoft.AspNetCore.Mvc { _allowCombiningAuthorizeFilters, _allowBindingHeaderValuesToNonStringModelTypes, + _allowValidatingTopLevelNodes, _inputFormatterExceptionPolicy, _suppressBindingUndefinedValueToEnumType, }; @@ -89,8 +92,8 @@ namespace Microsoft.AspNetCore.Mvc /// are applied. /// /// - /// This property is associated with a compatibility switch and can provide a different behavior depending on - /// the configured compatibility version for the application. See for + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for /// guidance and examples of setting the application's compatibility version. /// /// @@ -114,15 +117,15 @@ namespace Microsoft.AspNetCore.Mvc /// /// Gets or sets a value that determines if should bind to types other than - /// or a collection of . If set to true, - /// would bind to simple types (like , , - /// , etc.) or a collection of simple types. The default value of the + /// or a collection of . If set to true, + /// would bind to simple types (like , , + /// , etc.) or a collection of simple types. The default value of the /// property is false. /// /// /// - /// This property is associated with a compatibility switch and can provide a different behavior depending on - /// the configured compatibility version for the application. See for + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for /// guidance and examples of setting the application's compatibility version. /// /// @@ -144,6 +147,41 @@ namespace Microsoft.AspNetCore.Mvc set => _allowBindingHeaderValuesToNonStringModelTypes.Value = value; } + /// + /// Gets or sets a value that determines if model bound action parameters, controller properties, page handler + /// parameters, or page model properties are validated (in addition to validating their elements or + /// properties). If set to , and + /// ValidationAttributes on these top-level nodes are checked. Otherwise, such attributes are ignored. + /// + /// + /// The default value is if the version is + /// or later; otherwise. + /// + /// + /// + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for + /// guidance and examples of setting the application's compatibility version. + /// + /// + /// Configuring the desired value of the compatibility switch by calling this property's setter will take + /// precedence over the value implied by the application's . + /// + /// + /// If the application's compatibility version is set to then + /// this setting will have the value unless explicitly configured. + /// + /// + /// If the application's compatibility version is set to or + /// higher then this setting will have the value unless explicitly configured. + /// + /// + public bool AllowValidatingTopLevelNodes + { + get => _allowValidatingTopLevelNodes.Value; + set => _allowValidatingTopLevelNodes.Value = value; + } + /// /// Gets a Dictionary of CacheProfile Names, which are pre-defined settings for /// response caching. @@ -173,8 +211,8 @@ namespace Microsoft.AspNetCore.Mvc /// /// /// - /// This property is associated with a compatibility switch and can provide a different behavior depending on - /// the configured compatibility version for the application. See for + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for /// guidance and examples of setting the application's compatibility version. /// /// @@ -204,13 +242,13 @@ namespace Microsoft.AspNetCore.Mvc public FormatterCollection InputFormatters { get; } /// - /// Gets or sets an value indicating whether the model binding system will bind undefined values to + /// Gets or sets a value indicating whether the model binding system will bind undefined values to /// enum types. The default value of the property is false. /// /// /// - /// This property is associated with a compatibility switch and can provide a different behavior depending on - /// the configured compatibility version for the application. See for + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for /// guidance and examples of setting the application's compatibility version. /// /// diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs index a474bf6ad8..9a296ce7c4 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageActionInvokerProvider.cs @@ -6,7 +6,6 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Infrastructure; @@ -37,6 +36,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal private readonly ParameterBinder _parameterBinder; private readonly IModelMetadataProvider _modelMetadataProvider; private readonly ITempDataDictionaryFactory _tempDataFactory; + private readonly MvcOptions _mvcOptions; private readonly HtmlHelperOptions _htmlHelperOptions; private readonly IPageHandlerMethodSelector _selector; private readonly RazorProjectFileSystem _razorFileSystem; @@ -73,6 +73,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal _parameterBinder = parameterBinder; _modelMetadataProvider = modelMetadataProvider; _tempDataFactory = tempDataFactory; + _mvcOptions = mvcOptions.Value; _htmlHelperOptions = htmlHelperOptions.Value; _selector = selector; _razorFileSystem = razorFileSystem; @@ -261,7 +262,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal _modelMetadataProvider, _modelBinderFactory, actionDescriptor, - actionDescriptor.HandlerMethods[i]); + actionDescriptor.HandlerMethods[i], + _mvcOptions); } return results; diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageBinderFactory.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageBinderFactory.cs index 0b5900dbe4..6fcc9e08c8 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageBinderFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/PageBinderFactory.cs @@ -90,7 +90,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal IModelMetadataProvider modelMetadataProvider, IModelBinderFactory modelBinderFactory, CompiledPageActionDescriptor actionDescriptor, - HandlerMethodDescriptor handler) + HandlerMethodDescriptor handler, + MvcOptions mvcOptions) { if (handler.Parameters == null || handler.Parameters.Count == 0) { @@ -103,7 +104,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { var parameter = handler.Parameters[i]; ModelMetadata metadata; - if (modelMetadataProvider is ModelMetadataProvider modelMetadataProviderBase) + if (mvcOptions.AllowValidatingTopLevelNodes && + modelMetadataProvider is ModelMetadataProvider modelMetadataProviderBase) { // The default model metadata provider derives from ModelMetadataProvider // and can therefore supply information about attributes applied to parameters. diff --git a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs index 8ef866008a..faecd9554c 100644 --- a/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ApiExplorer.Test/DefaultApiDescriptionProviderTest.cs @@ -326,8 +326,10 @@ namespace Microsoft.AspNetCore.Mvc.Description { // Arrange var action = CreateActionDescriptor(); - action.AttributeRouteInfo = new AttributeRouteInfo(); - action.AttributeRouteInfo.Template = template; + action.AttributeRouteInfo = new AttributeRouteInfo + { + Template = template + }; // Act var descriptions = GetApiDescriptions(action); @@ -342,8 +344,10 @@ namespace Microsoft.AspNetCore.Mvc.Description { // Arrange var action = CreateActionDescriptor(); - action.AttributeRouteInfo = new AttributeRouteInfo(); - action.AttributeRouteInfo.Template = "api/Products/{id1}-{id2:int}"; + action.AttributeRouteInfo = new AttributeRouteInfo + { + Template = "api/Products/{id1}-{id2:int}" + }; // Act var descriptions = GetApiDescriptions(action); @@ -364,8 +368,10 @@ namespace Microsoft.AspNetCore.Mvc.Description { // Arrange var action = CreateActionDescriptor(); - action.AttributeRouteInfo = new AttributeRouteInfo(); - action.AttributeRouteInfo.Template = "api/Products/{id1}-{id2}/{id3:int}/{id4:int?}/{*id5:int}"; + action.AttributeRouteInfo = new AttributeRouteInfo + { + Template = "api/Products/{id1}-{id2}/{id3:int}/{id4:int?}/{*id5:int}" + }; // Act var descriptions = GetApiDescriptions(action); @@ -685,8 +691,10 @@ namespace Microsoft.AspNetCore.Mvc.Description // Arrange var action = CreateActionDescriptor(methodName); var filter = new ProducesResponseTypeAttribute(typeof(void), statusCode: 204); - action.FilterDescriptors = new List(); - action.FilterDescriptors.Add(new FilterDescriptor(filter, FilterScope.Action)); + action.FilterDescriptors = new List + { + new FilterDescriptor(filter, FilterScope.Action) + }; // Act var descriptions = GetApiDescriptions(action); @@ -717,8 +725,10 @@ namespace Microsoft.AspNetCore.Mvc.Description Type = typeof(Order) }; - action.FilterDescriptors = new List(); - action.FilterDescriptors.Add(new FilterDescriptor(filter, FilterScope.Action)); + action.FilterDescriptors = new List + { + new FilterDescriptor(filter, FilterScope.Action) + }; // Act var descriptions = GetApiDescriptions(action); @@ -758,8 +768,10 @@ namespace Microsoft.AspNetCore.Mvc.Description // Arrange var action = CreateActionDescriptor(nameof(ReturnsProduct)); var expectedMediaTypes = new[] { "text/json", "text/xml" }; - action.FilterDescriptors = new List(); - action.FilterDescriptors.Add(new FilterDescriptor(new ContentTypeAttribute("text/*"), FilterScope.Action)); + action.FilterDescriptors = new List + { + new FilterDescriptor(new ContentTypeAttribute("text/*"), FilterScope.Action) + }; // Act var descriptions = GetApiDescriptions(action); @@ -780,8 +792,10 @@ namespace Microsoft.AspNetCore.Mvc.Description Type = typeof(Order) }; - action.FilterDescriptors = new List(); - action.FilterDescriptors.Add(new FilterDescriptor(filter, FilterScope.Action)); + action.FilterDescriptors = new List + { + new FilterDescriptor(filter, FilterScope.Action) + }; var formatters = CreateOutputFormatters(); @@ -843,8 +857,10 @@ namespace Microsoft.AspNetCore.Mvc.Description // Arrange var action = CreateActionDescriptor(nameof(AcceptsProduct_Body)); - action.FilterDescriptors = new List(); - action.FilterDescriptors.Add(new FilterDescriptor(new ContentTypeAttribute("text/*"), FilterScope.Action)); + action.FilterDescriptors = new List + { + new FilterDescriptor(new ContentTypeAttribute("text/*"), FilterScope.Action) + }; // Act var descriptions = GetApiDescriptions(action); @@ -863,8 +879,10 @@ namespace Microsoft.AspNetCore.Mvc.Description // Arrange var action = CreateActionDescriptor(nameof(AcceptsProduct_Body)); - action.FilterDescriptors = new List(); - action.FilterDescriptors.Add(new FilterDescriptor(new ContentTypeAttribute("text/*"), FilterScope.Action)); + action.FilterDescriptors = new List + { + new FilterDescriptor(new ContentTypeAttribute("text/*"), FilterScope.Action) + }; var formatters = CreateInputFormatters(); @@ -914,7 +932,7 @@ namespace Microsoft.AspNetCore.Mvc.Description } [Fact] - public void GetApiDescription_ParameterDescription_IsRequired() + public void GetApiDescription_ParameterDescription_IsRequiredSet() { // Arrange var action = CreateActionDescriptor(nameof(RequiredParameter)); @@ -932,6 +950,25 @@ namespace Microsoft.AspNetCore.Mvc.Description Assert.True(parameter.ModelMetadata.IsBindingRequired); } + [Fact] + public void GetApiDescription_ParameterDescription_IsRequiredNotSet_IfNotValiatingTopLevelNodes() + { + // Arrange + var action = CreateActionDescriptor(nameof(RequiredParameter)); + + // Act + var descriptions = GetApiDescriptions(action, allowValidatingTopLevelNodes: false); + + // Assert + var description = Assert.Single(descriptions); + var parameter = Assert.Single(description.ParameterDescriptions); + Assert.Equal("name", parameter.Name); + Assert.Same(BindingSource.ModelBinding, parameter.Source); + Assert.Equal(typeof(string), parameter.Type); + Assert.False(parameter.ModelMetadata.IsRequired); + Assert.False(parameter.ModelMetadata.IsBindingRequired); + } + [Fact] public void GetApiDescription_ParameterDescription_SourceFromRouteData() { @@ -1382,11 +1419,15 @@ namespace Microsoft.AspNetCore.Mvc.Description private IReadOnlyList GetApiDescriptions( ActionDescriptor action, List inputFormatters = null, - List outputFormatters = null) + List outputFormatters = null, + bool allowValidatingTopLevelNodes = true) { var context = new ApiDescriptionProviderContext(new ActionDescriptor[] { action }); - var options = new MvcOptions(); + var options = new MvcOptions + { + AllowValidatingTopLevelNodes = allowValidatingTopLevelNodes, + }; foreach (var formatter in inputFormatters ?? CreateInputFormatters()) { options.InputFormatters.Add(formatter); @@ -1397,9 +1438,7 @@ namespace Microsoft.AspNetCore.Mvc.Description options.OutputFormatters.Add(formatter); } - var optionsAccessor = new Mock>(); - optionsAccessor.SetupGet(o => o.Value) - .Returns(options); + var optionsAccessor = Options.Create(options); var constraintResolver = new Mock(); constraintResolver.Setup(c => c.ResolveConstraint("int")) @@ -1408,7 +1447,7 @@ namespace Microsoft.AspNetCore.Mvc.Description var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var provider = new DefaultApiDescriptionProvider( - optionsAccessor.Object, + optionsAccessor, constraintResolver.Object, modelMetadataProvider); @@ -1917,4 +1956,4 @@ namespace Microsoft.AspNetCore.Mvc.Description public BindingSource BindingSource => BindingSource.FormFile; } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index 8bdea7d70c..935e0094e9 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -501,11 +501,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal { InvalidModelStateResponseFactory = _ => null, }; - var optionsProvider = Options.Create(options); + var optionsAccessor = Options.Create(options); + modelMetadataProvider = modelMetadataProvider ?? new TestModelMetadataProvider(); var loggerFactory = NullLoggerFactory.Instance; - return new ApiBehaviorApplicationModelProvider(optionsProvider, modelMetadataProvider, loggerFactory); + return new ApiBehaviorApplicationModelProvider(optionsAccessor, modelMetadataProvider, loggerFactory); } private static ApplicationModelProviderContext GetContext(Type type) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs index 93fa243d2d..19b1b3b171 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -103,6 +104,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal new[] { controllerContext.ActionDescriptor }); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var modelBinderFactory = TestModelBinderFactory.CreateDefault(); + var mvcOptions = Options.Create(new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }); return new ControllerActionInvokerCache( descriptorProvider, @@ -110,11 +115,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + mvcOptions, NullLoggerFactory.Instance), modelBinderFactory, modelMetadataProvider, filterProviders, - Mock.Of()); + Mock.Of(), + mvcOptions); } private static ControllerContext CreateControllerContext(FilterDescriptor[] filterDescriptors) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs index c1a5165621..6b9b9041db 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs @@ -20,6 +20,7 @@ using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -27,8 +28,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public class ControllerBinderDelegateProviderTest { + private static readonly MvcOptions _options = new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }; + private static readonly IOptions _optionsAccessor = Options.Create(_options); + [Fact] - public async Task BindActionArgumentsAsync_DoesNotAddActionArgumentsOrCallBinderOrValidator_IfBindingIsNotAllowed_OnParameter() + public async Task CreateBinderDelegate_Delegate_DoesNotAddActionArgumentsOrCallBinderOrValidator_IfBindingIsNotAllowed_OnParameter() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -61,6 +68,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new[] { GetModelValidatorProvider(mockValidator.Object) }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -68,7 +76,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, TestModelMetadataProvider.CreateDefaultProvider(), - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -85,7 +94,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_DoesNotAddActionArgumentsOrCallBinderOrValidator_IfBindingIsNotAllowed_OnProperty() + public async Task CreateBinderDelegate_Delegate_DoesNotAddActionArgumentsOrCallBinderOrValidator_IfBindingIsNotAllowed_OnProperty() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -109,12 +118,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var controller = new TestController(); + var parameterBinder = new ParameterBinder( modelMetadataProvider, factory, new DefaultObjectValidator( modelMetadataProvider, new[] { GetModelValidatorProvider(mockValidator.Object) }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -122,7 +133,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, TestModelMetadataProvider.CreateDefaultProvider(), - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -139,7 +151,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsNull() + public async Task CreateBinderDelegate_Delegate_DoesNotAddActionArguments_IfBinderReturnsNull() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -155,6 +167,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal binder .Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.CompletedTask); + var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var parameterBinder = new ParameterBinder( @@ -163,6 +176,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); var controllerContext = GetControllerContext(actionDescriptor); @@ -174,7 +188,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -183,7 +198,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderDoesNotSetModel() + public async Task CreateBinderDelegate_Delegate_DoesNotAddActionArguments_IfBinderDoesNotSetModel() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -199,6 +214,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal binder .Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.CompletedTask); + var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var parameterBinder = new ParameterBinder( @@ -207,6 +223,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); var controllerContext = GetControllerContext(actionDescriptor); @@ -218,7 +235,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -227,7 +245,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_AddsActionArguments_IfBinderReturnsNotNull() + public async Task CreateBinderDelegate_Delegate_AddsActionArguments_IfBinderReturnsNotNull() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -251,6 +269,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal context.Result = ModelBindingResult.Success(value); }) .Returns(Task.CompletedTask); + var factory = GetModelBinderFactory(binder.Object); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var parameterBinder = new ParameterBinder( @@ -259,6 +278,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); var controllerContext = GetControllerContext(actionDescriptor); @@ -270,7 +290,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -280,7 +301,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_GetsMetadataFromParameter() + public async Task CreateBinderDelegate_Delegate_GetsMetadataFromParameter() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -307,12 +328,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal mockMetadataProvider .Setup(p => p.GetMetadataForParameter(ParameterInfos.NoAttributesParameterInfo)) .Returns(modelMetadata.Object); + var parameterBinder = new ParameterBinder( mockMetadataProvider.Object, factory, new DefaultObjectValidator( mockMetadataProvider.Object, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -320,7 +343,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, mockMetadataProvider.Object, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -332,7 +356,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_GetsMetadataFromType_IsMetadataProviderIsNotDefaultMetadataProvider() + public async Task CreateBinderDelegate_Delegate_GetsMetadataFromType_IsMetadataProviderIsNotDefaultMetadataProvider() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -357,12 +381,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal mockMetadataProvider .Setup(p => p.GetMetadataForType(typeof(Person))) .Returns(modelMetadata.Object); + var parameterBinder = new ParameterBinder( mockMetadataProvider.Object, factory, new DefaultObjectValidator( mockMetadataProvider.Object, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -370,7 +396,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, mockMetadataProvider.Object, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -382,7 +409,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_CallsValidator_IfModelBinderSucceeds() + public async Task CreateBinderDelegate_Delegate_CallsValidator_IfModelBinderSucceeds() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -410,6 +437,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new[] { GetModelValidatorProvider(mockValidator.Object) }), + _optionsAccessor, NullLoggerFactory.Instance); var controller = new TestController(); var arguments = new Dictionary(StringComparer.Ordinal); @@ -419,15 +447,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); // Assert - mockValidator - .Verify(o => o.Validate( - It.IsAny()), - Times.Once()); + mockValidator.Verify(o => o.Validate(It.IsAny()), Times.Once()); Assert.False(controllerContext.ModelState.IsValid); Assert.Equal( @@ -436,7 +462,58 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_DoesNotCallValidator_IfModelBinderFails() + public async Task CreateBinderDelegate_Delegate_DoesNotCallValidator_IfNotValidatingTopLevelNodes() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ControllerParameterDescriptor + { + Name = "foo", + ParameterType = typeof(object), + ParameterInfo = ParameterInfos.CustomValidationParameterInfo + }); + + var controllerContext = GetControllerContext(actionDescriptor); + var factory = GetModelBinderFactory("Hello"); + + var mockValidator = new Mock(); + mockValidator + .Setup(o => o.Validate(It.IsAny())) + .Returns(new[] { new ModelValidationResult("memberName", "some message") }); + + // Do not set AllowValidatingTopLevelNodes. + var mvcOptions = new MvcOptions(); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var parameterBinder = new ParameterBinder( + modelMetadataProvider, + factory, + new DefaultObjectValidator( + modelMetadataProvider, + new[] { GetModelValidatorProvider(mockValidator.Object) }), + Options.Create(mvcOptions), + NullLoggerFactory.Instance); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); + + // Act + var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( + parameterBinder, + factory, + modelMetadataProvider, + actionDescriptor, + mvcOptions); + + await binderDelegate(controllerContext, controller, arguments); + + // Assert + Assert.True(controllerContext.ModelState.IsValid); + mockValidator.Verify(o => o.Validate(It.IsAny()), Times.Never()); + } + + [Fact] + public async Task CreateBinderDelegate_Delegate_DoesNotCallValidator_IfModelBinderFails() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -467,6 +544,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -474,7 +552,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -486,7 +565,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_CallsValidator_ForControllerProperties_IfModelBinderSucceeds() + public async Task CreateBinderDelegate_Delegate_CallsValidator_ForControllerProperties_IfModelBinderSucceeds() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -508,13 +587,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal var factory = GetModelBinderFactory("Hello"); var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var parameterBinder = new ParameterBinder( modelMetadataProvider, factory, new DefaultObjectValidator( modelMetadataProvider, new[] { GetModelValidatorProvider(mockValidator.Object) }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -522,15 +601,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); // Assert - mockValidator - .Verify(o => o.Validate( - It.IsAny()), - Times.Once()); + mockValidator.Verify(o => o.Validate(It.IsAny()), Times.Once()); Assert.False(controllerContext.ModelState.IsValid); Assert.Equal( "some message", @@ -567,6 +644,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal modelMetadataProvider, factory, mockValidator.Object, + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -574,7 +652,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -583,7 +662,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_DoesNotCallValidator_ForControllerProperties_IfModelBinderFails() + public async Task CreateBinderDelegate_Delegate_DoesNotCallValidator_ForControllerProperties_IfModelBinderFails() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -613,6 +692,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new[] { GetModelValidatorProvider(mockValidator.Object) }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -620,7 +700,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -632,7 +713,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_SetsControllerProperties_ForReferenceTypes() + public async Task CreateBinderDelegate_Delegate_SetsControllerProperties_ForReferenceTypes() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -656,6 +737,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -663,7 +745,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -674,7 +757,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_AddsToCollectionControllerProperties() + public async Task CreateBinderDelegate_Delegate_AddsToCollectionControllerProperties() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -699,6 +782,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -706,7 +790,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -717,7 +802,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_DoesNotSetNullValues_ForNonNullableProperties() + public async Task CreateBinderDelegate_Delegate_DoesNotSetNullValues_ForNonNullableProperties() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -742,6 +827,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Some non default value. @@ -752,7 +838,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -761,7 +848,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_SetsNullValues_ForNullableProperties() + public async Task CreateBinderDelegate_Delegate_SetsNullValues_ForNullableProperties() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -786,6 +873,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Some non default value. @@ -796,7 +884,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -805,7 +894,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_SupportsRequestPredicate_ForPropertiesAndParameters_NotBound() + public async Task CreateBinderDelegate_Delegate_SupportsRequestPredicate_ForPropertiesAndParameters_NotBound() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -851,6 +940,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Some non default value. @@ -861,7 +951,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -871,7 +962,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_SupportsRequestPredicate_ForPropertiesAndParameters_Bound() + public async Task CreateBinderDelegate_Delegate_SupportsRequestPredicate_ForPropertiesAndParameters_Bound() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -917,6 +1008,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Some non default value. @@ -927,7 +1019,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -978,7 +1071,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal [Theory] [MemberData(nameof(SkippedPropertyData))] - public async Task BindActionArgumentsAsync_SkipsReadOnlyControllerProperties( + public async Task CreateBinderDelegate_Delegate_SkipsReadOnlyControllerProperties( string propertyName, Type propertyType, Func propertyAccessor, @@ -1007,6 +1100,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -1014,7 +1108,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -1025,7 +1120,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public async Task BindActionArgumentsAsync_SetsMultipleControllerProperties() + public async Task CreateBinderDelegate_Delegate_SetsMultipleControllerProperties() { // Arrange var boundPropertyTypes = new Dictionary @@ -1085,6 +1180,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new DefaultObjectValidator( modelMetadataProvider, new IModelValidatorProvider[] { }), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -1092,7 +1188,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder, factory, modelMetadataProvider, - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, controller, arguments); @@ -1182,6 +1279,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new EmptyModelMetadataProvider(), factory, new DefaultObjectValidator(modelMetadataProvider, new[] { modelValidatorProvider }), + _optionsAccessor, NullLoggerFactory.Instance); parameterBinder.Setup(p => p.BindModelAsync( It.IsAny(), @@ -1223,7 +1321,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterBinder.Object, factory, TestModelMetadataProvider.CreateDefaultProvider(), - actionDescriptor); + actionDescriptor, + _options); await binderDelegate(controllerContext, new TestController(), arguments); @@ -1340,10 +1439,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal var objectModelValidator = new DefaultObjectValidator( metadataProvider, new[] { modelValidatorProvider }); + return new ParameterBinder( metadataProvider, factory, objectModelValidator, + _optionsAccessor, NullLoggerFactory.Instance); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs index 9e8ac6011e..9c6a474f3e 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public async Task MiddlewareFilter_SetsMiddlewareFilterFeature_OnExecution() { // Arrange - RequestDelegate requestDelegate = (context) => Task.FromResult(true); + Task requestDelegate(HttpContext context) => Task.FromResult(true); var middlwareFilter = new MiddlewareFilter(requestDelegate); var httpContext = new DefaultHttpContext(); var resourceExecutingContext = GetResourceExecutingContext(httpContext); @@ -315,8 +315,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal { var services = CreateServices(); - var httpContext = new DefaultHttpContext(); - httpContext.RequestServices = services.BuildServiceProvider(); + var httpContext = new DefaultHttpContext + { + RequestServices = services.BuildServiceProvider() + }; return httpContext; } @@ -326,8 +328,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal var applicationServices = new ServiceCollection(); var applicationBuilder = new ApplicationBuilder(applicationServices.BuildServiceProvider()); var middlewareFilterBuilderService = new MiddlewareFilterBuilder( - new MiddlewareFilterConfigurationProvider()); - middlewareFilterBuilderService.ApplicationBuilder = applicationBuilder; + new MiddlewareFilterConfigurationProvider()) + { + ApplicationBuilder = applicationBuilder + }; + return middlewareFilterBuilderService.GetPipeline(middlewarePipelineProviderType); } @@ -444,11 +449,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal private class TestParameterBinder : ParameterBinder { private readonly IDictionary _actionParameters; + public TestParameterBinder(IDictionary actionParameters) : base( new EmptyModelMetadataProvider(), TestModelBinderFactory.CreateDefault(), Mock.Of(), + Options.Create(new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }), NullLoggerFactory.Instance) { _actionParameters = actionParameters; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs index 85a0e02723..4838cf3f62 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs @@ -15,6 +15,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -22,6 +23,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { public class ParameterBinderTest { + private static readonly IOptions _optionsAccessor = Options.Create(new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }); + public static TheoryData BindModelAsyncData { get @@ -98,7 +104,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var parameterBinder = new ParameterBinder( metadataProvider, factory.Object, - Mock.Of< IObjectModelValidator>(), + Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); var controllerContext = GetControllerContext(); @@ -150,6 +157,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding metadataProvider, factory.Object, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); var valueProvider = new SimpleValueProvider @@ -175,7 +183,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding mockModelMetadata.Setup(o => o.IsBindingRequired).Returns(true); mockModelMetadata.Setup(o => o.DisplayName).Returns("Ignored Display Name"); // Bind attribute errors are phrased in terms of the model name, not display name - var parameterBinder = CreateParameterBinder(mockModelMetadata.Object, validator: null); + var parameterBinder = CreateParameterBinder(mockModelMetadata.Object); var modelBindingResult = ModelBindingResult.Failed(); // Act @@ -195,6 +203,37 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding actionContext.ModelState.Single().Value.Errors.Single().ErrorMessage); } + [Fact] + public async Task BindModelAsync_DoesNotEnforceTopLevelBindRequired_IfNotValidatingTopLevelNodes() + { + // Arrange + var actionContext = GetControllerContext(); + + var mockModelMetadata = CreateMockModelMetadata(); + mockModelMetadata.Setup(o => o.IsBindingRequired).Returns(true); + + // Bind attribute errors are phrased in terms of the model name, not display name + mockModelMetadata.Setup(o => o.DisplayName).Returns("Ignored Display Name"); + + // Do not set AllowValidatingTopLevelNodes. + var optionsAccessor = Options.Create(new MvcOptions()); + var parameterBinder = CreateParameterBinder(mockModelMetadata.Object, optionsAccessor: optionsAccessor); + var modelBindingResult = ModelBindingResult.Failed(); + + // Act + var result = await parameterBinder.BindModelAsync( + actionContext, + CreateMockModelBinder(modelBindingResult), + CreateMockValueProvider(), + new ParameterDescriptor { Name = "myParam", ParameterType = typeof(Person) }, + mockModelMetadata.Object, + "ignoredvalue"); + + // Assert + Assert.True(actionContext.ModelState.IsValid); + Assert.Empty(actionContext.ModelState); + } + [Fact] public async Task BindModelAsync_EnforcesTopLevelRequired() { @@ -213,9 +252,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding new RequiredAttribute(), stringLocalizer: null); - var parameterBinder = CreateParameterBinder( - mockModelMetadata.Object, - validator); + var parameterBinder = CreateParameterBinder(mockModelMetadata.Object, validator); var modelBindingResult = ModelBindingResult.Success(null); // Act @@ -235,6 +272,43 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding actionContext.ModelState.Single().Value.Errors.Single().ErrorMessage); } + [Fact] + public async Task BindModelAsync_DoesNotEnforceTopLevelRequired_IfNotValidatingTopLevelNodes() + { + // Arrange + var actionContext = GetControllerContext(); + var mockModelMetadata = CreateMockModelMetadata(); + mockModelMetadata.Setup(o => o.IsRequired).Returns(true); + mockModelMetadata.Setup(o => o.DisplayName).Returns("My Display Name"); + mockModelMetadata.Setup(o => o.ValidatorMetadata).Returns(new[] + { + new RequiredAttribute() + }); + + var validator = new DataAnnotationsModelValidator( + new ValidationAttributeAdapterProvider(), + new RequiredAttribute(), + stringLocalizer: null); + + // Do not set AllowValidatingTopLevelNodes. + var optionsAccessor = Options.Create(new MvcOptions()); + var parameterBinder = CreateParameterBinder(mockModelMetadata.Object, validator, optionsAccessor); + var modelBindingResult = ModelBindingResult.Success(null); + + // Act + var result = await parameterBinder.BindModelAsync( + actionContext, + CreateMockModelBinder(modelBindingResult), + CreateMockValueProvider(), + new ParameterDescriptor { Name = "myParam", ParameterType = typeof(Person) }, + mockModelMetadata.Object, + "ignoredvalue"); + + // Assert + Assert.True(actionContext.ModelState.IsValid); + Assert.Empty(actionContext.ModelState); + } + [Fact] public async Task BindModelAsync_EnforcesTopLevelDataAnnotationsAttribute() { @@ -252,9 +326,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding validationAttribute, stringLocalizer: null); - var parameterBinder = CreateParameterBinder( - mockModelMetadata.Object, - validator); + var parameterBinder = CreateParameterBinder(mockModelMetadata.Object, validator); var modelBindingResult = ModelBindingResult.Success(123); // Act @@ -353,7 +425,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding private static ParameterBinder CreateParameterBinder( ModelMetadata modelMetadata, - IModelValidator validator) + IModelValidator validator = null, + IOptions optionsAccessor = null) { var mockModelMetadataProvider = new Mock(MockBehavior.Strict); mockModelMetadataProvider @@ -361,12 +434,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding .Returns(modelMetadata); var mockModelBinderFactory = new Mock(MockBehavior.Strict); + optionsAccessor = optionsAccessor ?? _optionsAccessor; return new ParameterBinder( mockModelMetadataProvider.Object, mockModelBinderFactory.Object, new DefaultObjectValidator( - mockModelMetadataProvider.Object, + mockModelMetadataProvider.Object, new[] { GetModelValidatorProvider(validator) }), + optionsAccessor, NullLoggerFactory.Instance); } @@ -390,7 +465,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding }); return validatorProvider.Object; } - + private static ParameterBinder CreateBackCompatParameterBinder( ModelMetadata modelMetadata, IObjectModelValidator validator) diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/CombineAuthorizeTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/CombineAuthorizeTests.cs deleted file mode 100644 index 839c10081e..0000000000 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/CombineAuthorizeTests.cs +++ /dev/null @@ -1,43 +0,0 @@ -// 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 System; -using System.Linq; -using System.Net; -using System.Net.Http; -using System.Threading.Tasks; -using SecurityWebSite; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.FunctionalTests -{ - public class CombineAuthorizeTests : IClassFixture> - { - public CombineAuthorizeTests(MvcTestFixture fixture) - { - Client = fixture.CreateDefaultClient(); - } - - public HttpClient Client { get; } - - [Fact] - public async Task CanAuthorizeWithOnlyCookie2() - { - // Arrange & Act - var response = await Client.PostAsync("http://localhost/Administration/SignInCookie2", null); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.True(response.Headers.Contains("Set-Cookie")); - - var cookie2 = response.Headers.GetValues("Set-Cookie").SingleOrDefault(); - - var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Administration/EitherCookie"); - request.Headers.Add("Cookie", cookie2); - - response = await Client.SendAsync(request); - var body = await response.Content.ReadAsStringAsync(); - Assert.Equal("Administration.EitherCookie:AuthorizeCount=1", body); - } - } -} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/GlobalAuthorizationFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/GlobalAuthorizationFilterTest.cs index aaab315e59..23dce2a053 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/GlobalAuthorizationFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/GlobalAuthorizationFilterTest.cs @@ -6,7 +6,9 @@ using System.Net; using System.Net.Http; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.TestHost; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; using Xunit; namespace Microsoft.AspNetCore.Mvc.FunctionalTests @@ -15,8 +17,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests { public GlobalAuthorizationFilterTest(MvcTestFixture fixture) { - var factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); - Client = factory.CreateDefaultClient(); + Factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(ConfigureWebHostBuilder); + Client = Factory.CreateDefaultClient(); } private static void ConfigureWebHostBuilder(IWebHostBuilder builder) => @@ -24,6 +26,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public HttpClient Client { get; } + public WebApplicationFactory Factory { get; } + [Fact] public async Task DeniesAnonymousUsers_ByDefault() { @@ -51,10 +55,15 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task AuthorizationPoliciesDoNotCombineByDefault() + public async Task AuthorizationPoliciesDoNotCombine_WithV2_0() { // Arrange & Act - var response = await Client.PostAsync("http://localhost/Administration/SignInCookie2", null); + var factory = Factory.WithWebHostBuilder( + builder => builder.ConfigureServices( + services => services.Configure( + options => options.CompatibilityVersion = CompatibilityVersion.Version_2_0))); + var client = factory.CreateDefaultClient(); + var response = await client.PostAsync("http://localhost/Administration/SignInCookie2", null); // Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); @@ -65,8 +74,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Administration/EitherCookie"); request.Headers.Add("Cookie", cookie2); - // Will fail because default cookie is not sent so [Authorize] fails - response = await Client.SendAsync(request); + // Will fail because default cookie is not sent so [Authorize] fails. + response = await client.SendAsync(request); Assert.Equal(HttpStatusCode.Redirect, response.StatusCode); Assert.NotNull(response.Headers.Location); Assert.Equal( @@ -74,5 +83,28 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests response.Headers.Location.ToString()); } - } -} \ No newline at end of file + [Fact] + public async Task AuthorizationPoliciesCombine_WithV2_1() + { + // Arrange & Act 1 + var response = await Client.PostAsync("http://localhost/Administration/SignInCookie2", null); + + // Assert 1 + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.True(response.Headers.Contains("Set-Cookie")); + + // Arrange 2 + var cookie2 = response.Headers.GetValues("Set-Cookie").SingleOrDefault(); + var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Administration/EitherCookie"); + request.Headers.Add("Cookie", cookie2); + + // Act 2: Will succeed because, with AllowCombiningAuthorizeFilters true, [Authorize] allows either cookie. + response = await Client.SendAsync(request); + + // Assert 2 + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Null(response.Headers.Location); + } + + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs index 579de362fd..f80e530537 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/Infrastructure/MvcTestFixture.cs @@ -1,20 +1,26 @@ // 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 System; using System.Globalization; -using System.IO; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; namespace Microsoft.AspNetCore.Mvc.FunctionalTests { public class MvcTestFixture : WebApplicationFactory where TStartup : class { - protected override void ConfigureWebHost(IWebHostBuilder builder) => - builder.UseRequestCulture("en-GB", "en-US"); + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder + .UseRequestCulture("en-GB", "en-US") + .ConfigureServices( + services => services.Configure( + options => options.CompatibilityVersion = CompatibilityVersion.Version_2_1)); + } protected override TestServer CreateServer(IWebHostBuilder builder) { diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputFormatterTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputFormatterTests.cs index e0d6950fb0..2907b248c8 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputFormatterTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputFormatterTests.cs @@ -109,9 +109,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests // Assert Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); - - // Update me in 3.0 xD - Assert.Equal("{\"\":[\"The input was not valid.\"]}", responseBody); + Assert.Equal("{\"\":[\"Unexpected end when reading JSON. Path '', line 1, position 1.\"]}", responseBody); } [Theory] diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index 82970be594..2257eb6eb1 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -384,7 +384,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var optionsAccessor = testContext.GetService>(); optionsAccessor.Value.AllowEmptyInputInBodyModelBinding = true; - + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(optionsAccessor.Value); // Act @@ -428,7 +428,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var optionsAccessor = testContext.GetService>(); optionsAccessor.Value.AllowEmptyInputInBodyModelBinding = true; - + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(optionsAccessor.Value); // Act @@ -453,7 +453,18 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_JsonFormatterAddsModelStateError() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var testContext = ModelBindingTestHelper.GetTestContext( + request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); + request.ContentType = "application/json"; + }); + + // Override the AllowInputFormatterExceptionMessages setting ModelBindingTestHelper chooses. + var options = testContext.GetService>().Value; + options.AllowInputFormatterExceptionMessages = false; + + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var parameter = new ParameterDescriptor { Name = "Parameter1", @@ -464,13 +475,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests ParameterType = typeof(Person4) }; - var testContext = ModelBindingTestHelper.GetTestContext( - request => - { - request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); - request.ContentType = "application/json"; - }); - var modelState = testContext.ModelState; // Act @@ -486,7 +490,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Null(entry.Value.AttemptedValue); Assert.Null(entry.Value.RawValue); var error = Assert.Single(entry.Value.Errors); - + // Update me in 3.0 when MvcJsonOptions.AllowInputFormatterExceptionMessages is removed Assert.IsType(error.Exception); Assert.Empty(error.ErrorMessage); @@ -549,7 +553,18 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public async Task FromBodyWithInvalidPropertyData_JsonFormatterAddsModelError() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var testContext = ModelBindingTestHelper.GetTestContext( + request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{ \"Number\": \"not a number\" }")); + request.ContentType = "application/json"; + }); + + // Override the AllowInputFormatterExceptionMessages setting ModelBindingTestHelper chooses. + var options = testContext.GetService>().Value; + options.AllowInputFormatterExceptionMessages = false; + + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var parameter = new ParameterDescriptor { Name = "Parameter1", @@ -560,13 +575,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests ParameterType = typeof(Person5) }; - var testContext = ModelBindingTestHelper.GetTestContext( - request => - { - request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{ \"Number\": \"not a number\" }")); - request.ContentType = "application/json"; - }); - var modelState = testContext.ModelState; // Act @@ -596,7 +604,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData(false, false)] [InlineData(true, true)] public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( - bool allowEmptyInputInBodyModelBindingSetting, + bool allowEmptyInputInBodyModelBindingSetting, bool expectedModelStateIsValid) { // Arrange @@ -615,10 +623,10 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests { request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; - }); + }, + options => options.AllowEmptyInputInBodyModelBinding = allowEmptyInputInBodyModelBindingSetting); var optionsAccessor = testContext.GetService>(); - optionsAccessor.Value.AllowEmptyInputInBodyModelBinding = allowEmptyInputInBodyModelBindingSetting; var modelState = testContext.ModelState; var parameterBinder = ModelBindingTestHelper.GetParameterBinder(optionsAccessor.Value); @@ -630,7 +638,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); var boundPerson = Assert.IsType(modelBindingResult.Model); Assert.NotNull(boundPerson); - + if (expectedModelStateIsValid) { Assert.True(modelState.IsValid); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs index 49f64eacfa..5a017bd1d0 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs @@ -2701,7 +2701,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests entry = Assert.Single(modelState, e => e.Key == "Info.Value.GpsCoordinates").Value; Assert.Equal("10,20", entry.AttemptedValue); - Assert.Equal(new[] { "10", "20" }, entry.RawValue); + Assert.Equal("10,20", entry.RawValue); } private static void SetJsonBodyContent(HttpRequest request, string content) diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs index 8088c0302f..fb89e0e204 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs @@ -441,14 +441,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Action updateRequest = null, Action updateOptions = null) { - if (updateOptions == null) - { - updateOptions = o => - { - o.AllowBindingHeaderValuesToNonStringModelTypes = true; - }; - } - return ModelBindingTestHelper.GetTestContext(updateRequest, updateOptions); } @@ -520,4 +512,4 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs index 030e723481..00d2d6046d 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -71,6 +71,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests new DefaultObjectValidator( metadataProvider, new[] { new CompositeModelValidatorProvider(GetModelValidatorProviders(options)) }), + options, NullLoggerFactory.Instance); } @@ -95,6 +96,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests new DefaultObjectValidator( metadataProvider, new[] { new CompositeModelValidatorProvider(GetModelValidatorProviders(options)) }), + options, NullLoggerFactory.Instance); } @@ -151,7 +153,9 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var serviceCollection = new ServiceCollection(); serviceCollection.AddAuthorization(); serviceCollection.AddSingleton(new ApplicationPartManager()); - serviceCollection.AddMvc(); + serviceCollection + .AddMvc() + .SetCompatibilityVersion(CompatibilityVersion.Version_2_1); serviceCollection .AddSingleton() .AddTransient() diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs index e672a1e346..f41c4db6e5 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs @@ -487,10 +487,16 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var modelBinderFactory = TestModelBinderFactory.CreateDefault(); + var mvcOptions = new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }; + var parameterBinder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), Mock.Of(), + Options.Create(mvcOptions), NullLoggerFactory.Instance); return new PageActionInvokerProvider( diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs index 12c4029fdc..e1e7abf2ff 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs @@ -26,6 +26,7 @@ using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -1240,10 +1241,16 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal } var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var mvcOptions = new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }; + return new ParameterBinder( metadataProvider, factory, new DefaultObjectValidator(metadataProvider, new[] { validator }), + Options.Create(mvcOptions), NullLoggerFactory.Instance); } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs index 531f223d85..eed043db10 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageBinderFactoryTest.cs @@ -15,6 +15,7 @@ using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -22,6 +23,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { public class PageBinderFactoryTest { + private static readonly MvcOptions _options = new MvcOptions + { + AllowValidatingTopLevelNodes = true, + }; + private static readonly IOptions _optionsAccessor = Options.Create(_options); + [Fact] public void GetModelBinderFactory_ReturnsNullIfPageHasNoBoundProperties() { @@ -37,6 +44,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -62,6 +70,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -86,6 +95,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -111,6 +121,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -135,6 +146,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -160,6 +172,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance); // Act @@ -285,7 +298,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal // Assert // Verify that the page properties were not bound. - Assert.Equal(default(int), page.Id); + Assert.Equal(default, page.Id); Assert.Null(page.RouteDifferentValue); Assert.Equal(10, model.Id); @@ -541,6 +554,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal new DefaultObjectValidator( modelMetadataProvider, new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + _optionsAccessor, NullLoggerFactory.Instance); var factory = PageBinderFactory.CreatePropertyBinder(binder, modelMetadataProvider, modelBinderFactory, actionDescriptor); @@ -581,13 +595,13 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var modelBinderFactory = TestModelBinderFactory.CreateDefault(); - var factory = PageBinderFactory.CreateHandlerBinder( parameterBinder, modelMetadataProvider, modelBinderFactory, actionDescriptor, - actionDescriptor.HandlerMethods[0]); + actionDescriptor.HandlerMethods[0], + _options); var page = new PageWithProperty { @@ -625,13 +639,13 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var modelBinderFactory = TestModelBinderFactory.CreateDefault(); - var factory = PageBinderFactory.CreateHandlerBinder( parameterBinder, modelMetadataProvider, modelBinderFactory, actionDescriptor, - actionDescriptor.HandlerMethods[0]); + actionDescriptor.HandlerMethods[0], + _options); var page = new PageWithProperty { @@ -659,13 +673,13 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); var modelBinderFactory = TestModelBinderFactory.CreateDefault(); - var parameterBinder = new ParameterBinder( modelMetadataProvider, modelBinderFactory, new DefaultObjectValidator( modelMetadataProvider, new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + _optionsAccessor, NullLoggerFactory.Instance); var factory = PageBinderFactory.CreateHandlerBinder( @@ -673,7 +687,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal modelMetadataProvider, modelBinderFactory, actionDescriptor, - actionDescriptor.HandlerMethods[0]); + actionDescriptor.HandlerMethods[0], + _options); var page = new PageWithProperty { @@ -690,13 +705,62 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var modelState = page.PageContext.ModelState; Assert.False(modelState.IsValid); Assert.Collection( - page.PageContext.ModelState, + modelState, kvp => { Assert.Equal("name", kvp.Key); }); } + [Fact] + public async Task CreateHandlerBinder_DoesNotValidateTopLevelParameters_IfDisabled() + { + // Arrange + var type = typeof(PageModelWithExecutors); + var actionDescriptor = GetActionDescriptorWithHandlerMethod( + type, + nameof(PageModelWithExecutors.OnPostWithValidation)); + + // Act + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelBinderFactory = TestModelBinderFactory.CreateDefault(); + var mvcOptions = new MvcOptions(); + + var parameterBinder = new ParameterBinder( + modelMetadataProvider, + modelBinderFactory, + new DefaultObjectValidator( + modelMetadataProvider, + new[] { TestModelValidatorProvider.CreateDefaultProvider() }), + Options.Create(mvcOptions), + NullLoggerFactory.Instance); + + var factory = PageBinderFactory.CreateHandlerBinder( + parameterBinder, + modelMetadataProvider, + modelBinderFactory, + actionDescriptor, + actionDescriptor.HandlerMethods[0], + mvcOptions); + + var page = new PageWithProperty + { + PageContext = GetPageContext() + }; + + var model = new PageModelWithExecutors(); + var arguments = new Dictionary(); + + // Act + await factory(page.PageContext, arguments); + + // Assert + var modelState = page.PageContext.ModelState; + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + private static CompiledPageActionDescriptor GetActionDescriptorWithHandlerMethod(Type type, string method) { var handlerMethodInfo = type.GetMethod(method); @@ -756,6 +820,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal TestModelMetadataProvider.CreateDefaultProvider(), TestModelBinderFactory.CreateDefault(), Mock.Of(), + _optionsAccessor, NullLoggerFactory.Instance) { _args = args; @@ -914,8 +979,6 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public void OnPostWithValidation([Required] string name) { } - } - } } diff --git a/test/WebSites/RazorPagesWebSite/Startup.cs b/test/WebSites/RazorPagesWebSite/Startup.cs index 98de9be426..fe3cecc4e9 100644 --- a/test/WebSites/RazorPagesWebSite/Startup.cs +++ b/test/WebSites/RazorPagesWebSite/Startup.cs @@ -25,8 +25,7 @@ namespace RazorPagesWebSite options.Conventions.AddPageRoute("/Pages/NotTheRoot", string.Empty); options.Conventions.Add(new CustomModelTypeConvention()); }) - .WithRazorPagesAtContentRoot() - .SetCompatibilityVersion(Microsoft.AspNetCore.Mvc.CompatibilityVersion.Version_2_1); + .WithRazorPagesAtContentRoot(); } public void Configure(IApplicationBuilder app) diff --git a/test/WebSites/RazorPagesWebSite/StartupWithBasePath.cs b/test/WebSites/RazorPagesWebSite/StartupWithBasePath.cs index 669c1dba24..4ee45cae12 100644 --- a/test/WebSites/RazorPagesWebSite/StartupWithBasePath.cs +++ b/test/WebSites/RazorPagesWebSite/StartupWithBasePath.cs @@ -1,8 +1,8 @@ // 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.Builder; using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.AspNetCore.Builder; using Microsoft.Extensions.DependencyInjection; using RazorPagesWebSite.Conventions; @@ -24,8 +24,7 @@ namespace RazorPagesWebSite options.Conventions.AuthorizeAreaFolder("Accounts", "/RequiresAuth"); options.Conventions.AllowAnonymousToAreaPage("Accounts", "/RequiresAuth/AllowAnonymous"); options.Conventions.Add(new CustomModelTypeConvention()); - }) - .SetCompatibilityVersion(Microsoft.AspNetCore.Mvc.CompatibilityVersion.Version_2_1); + }); } public void Configure(IApplicationBuilder app) diff --git a/test/WebSites/SecurityWebSite/StartupWithGlobalAuthorizeAndAllowCombiningAuthorizeFilters.cs b/test/WebSites/SecurityWebSite/StartupWithGlobalAuthorizeAndAllowCombiningAuthorizeFilters.cs deleted file mode 100644 index 135c153367..0000000000 --- a/test/WebSites/SecurityWebSite/StartupWithGlobalAuthorizeAndAllowCombiningAuthorizeFilters.cs +++ /dev/null @@ -1,47 +0,0 @@ -// 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.Authentication.Cookies; -using Microsoft.AspNetCore.Authorization.Policy; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Mvc.Authorization; -using Microsoft.Extensions.DependencyInjection; - -namespace SecurityWebSite -{ - public class StartupWithGlobalAuthorizeAndAllowCombiningAuthorizeFilters - { - // This method gets called by the runtime. Use this method to add services to the container. - public void ConfigureServices(IServiceCollection services) - { - // Add framework services. - services.AddMvc(o => - { - o.AllowCombiningAuthorizeFilters = true; - o.Filters.Add(new AuthorizeFilter()); - }); - - services.AddAntiforgery(); - services.AddAuthentication(CookieAuthenticationDefaults.AuthenticationScheme).AddCookie(options => - { - options.LoginPath = "/Home/Login"; - options.LogoutPath = "/Home/Logout"; - }).AddCookie("Cookie2"); - - services.AddScoped(); - } - - // This method gets called by the runtime. Use this method to configure the HTTP request pipeline. - public void Configure(IApplicationBuilder app) - { - app.UseAuthentication(); - - app.UseMvc(routes => - { - routes.MapRoute( - name: "default", - template: "{controller=Home}/{action=Index}/{id?}"); - }); - } - } -}