diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs index eb2136e413..6228923e70 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs @@ -17,6 +17,19 @@ public abstract class ModelBinderProviderContext /// An . public abstract IModelBinder CreateBinder(ModelMetadata metadata); + /// + /// Creates an for the given + /// and . + /// + /// The for the model. + /// The that should be used + /// for creating the binder. + /// An . + public virtual IModelBinder CreateBinder(ModelMetadata metadata, BindingInfo bindingInfo) + { + throw new NotSupportedException(); + } + /// /// Gets the . /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs index c99042f0d3..e6b95f6b13 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcOptionsConfigureCompatibilityOptions.cs @@ -26,6 +26,7 @@ protected override IReadOnlyDictionary DefaultValues if (Version >= CompatibilityVersion.Version_2_1) { values[nameof(MvcOptions.AllowCombiningAuthorizeFilters)] = true; + values[nameof(MvcOptions.AllowBindingHeaderValuesToNonStringModelTypes)] = true; values[nameof(MvcOptions.InputFormatterExceptionPolicy)] = InputFormatterExceptionPolicy.MalformedInputExceptions; values[nameof(MvcOptions.SuppressBindingUndefinedValueToEnumType)] = true; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs index d65da95d40..54162d0b2c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs @@ -107,6 +107,7 @@ internal static class MvcCoreLoggerExtensions private static readonly Action _cannotBindToComplexType; private static readonly Action _cannotBindToFilesCollectionDueToUnsupportedContentType; private static readonly Action _cannotCreateHeaderModelBinder; + private static readonly Action _cannotCreateHeaderModelBinderCompatVersion_2_0; private static readonly Action _noFilesFoundInRequest; private static readonly Action _noNonIndexBasedFormatFoundForCollection; private static readonly Action _attemptingToBindCollectionUsingIndices; @@ -490,7 +491,7 @@ static MvcCoreLoggerExtensions() _cannotCreateHeaderModelBinder = LoggerMessage.Define( LogLevel.Debug, 20, - "Could not create a binder for type '{ModelType}' as this binder only supports 'System.String' type or a collection of 'System.String'."); + "Could not create a binder for type '{ModelType}' as this binder only supports simple types (like string, int, bool, enum) or a collection of simple types."); _noFilesFoundInRequest = LoggerMessage.Define( LogLevel.Debug, @@ -597,6 +598,11 @@ static MvcCoreLoggerExtensions() LogLevel.Debug, 42, "Done attempting to validate the bound property '{PropertyContainerType}.{PropertyName}' of type '{ModelType}'."); + + _cannotCreateHeaderModelBinderCompatVersion_2_0 = LoggerMessage.Define( + LogLevel.Debug, + 43, + "Could not create a binder for type '{ModelType}' as this binder only supports 'System.String' type or a collection of 'System.String'."); } public static void RegisteredOutputFormatters(this ILogger logger, IEnumerable outputFormatters) @@ -1169,6 +1175,11 @@ public static void CannotBindToFilesCollectionDueToUnsupportedContentType(this I _cannotBindToFilesCollectionDueToUnsupportedContentType(logger, bindingContext.ModelName, bindingContext.ModelType, null); } + public static void CannotCreateHeaderModelBinderCompatVersion_2_0(this ILogger logger, Type modelType) + { + _cannotCreateHeaderModelBinderCompatVersion_2_0(logger, modelType, null); + } + public static void CannotCreateHeaderModelBinder(this ILogger logger, Type modelType) { _cannotCreateHeaderModelBinder(logger, modelType, null); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinder.cs index 5be7fceedc..48c3ae2304 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinder.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; +using System.Globalization; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Internal; @@ -21,11 +23,11 @@ public class HeaderModelBinder : IModelBinder /// /// This constructor is obsolete and will be removed in a future version. The recommended alternative - /// is the overload that takes an . + /// is the overload that takes an and an . /// Initializes a new instance of . /// [Obsolete("This constructor is obsolete and will be removed in a future version. The recommended alternative" - + " is the overload that takes an " + nameof(ILoggerFactory) + ".")] + + " is the overload that takes an " + nameof(ILoggerFactory) + " and an " + nameof(IModelBinder) + ".")] public HeaderModelBinder() : this(NullLoggerFactory.Instance) { @@ -36,35 +38,117 @@ public HeaderModelBinder() /// /// The . public HeaderModelBinder(ILoggerFactory loggerFactory) + { + _logger = loggerFactory.CreateLogger(); + } + + /// + /// Initializes a new instance of . + /// + /// The . + /// The which does the actual + /// binding of values. + public HeaderModelBinder(ILoggerFactory loggerFactory, IModelBinder innerModelBinder) { if (loggerFactory == null) { throw new ArgumentNullException(nameof(loggerFactory)); } + if (innerModelBinder == null) + { + throw new ArgumentNullException(nameof(innerModelBinder)); + } + _logger = loggerFactory.CreateLogger(); + InnerModelBinder = innerModelBinder; } + // to enable unit testing + internal IModelBinder InnerModelBinder { get; } + /// - public Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { if (bindingContext == null) { throw new ArgumentNullException(nameof(bindingContext)); } - var request = bindingContext.HttpContext.Request; + _logger.AttemptingToBindModel(bindingContext); // Property name can be null if the model metadata represents a type (rather than a property or parameter). var headerName = bindingContext.FieldName; - _logger.AttemptingToBindModel(bindingContext); - + // Do not set ModelBindingResult to Failed on not finding the value in the header as we want the inner + // modelbinder to do that. This would give a chance to the inner binder to add more useful information. + // For example, SimpleTypeModelBinder adds a model error when binding to let's say an integer and the + // model is null. + var request = bindingContext.HttpContext.Request; if (!request.Headers.ContainsKey(headerName)) { _logger.FoundNoValueInRequest(bindingContext); } + if (InnerModelBinder == null) + { + BindWithoutInnerBinder(bindingContext); + return; + } + + var headerValueProvider = GetHeaderValueProvider(headerName, bindingContext); + + // Capture the top level object here as entering nested scope would make it 'false'. + var isTopLevelObject = bindingContext.IsTopLevelObject; + + // Create a new binding scope in order to supply the HeaderValueProvider so that the binders like + // SimpleTypeModelBinder can find values from header. + ModelBindingResult result; + using (bindingContext.EnterNestedScope( + bindingContext.ModelMetadata, + fieldName: bindingContext.FieldName, + modelName: bindingContext.ModelName, + model: bindingContext.Model)) + { + bindingContext.IsTopLevelObject = isTopLevelObject; + bindingContext.ValueProvider = headerValueProvider; + + await InnerModelBinder.BindModelAsync(bindingContext); + result = bindingContext.Result; + } + + bindingContext.Result = result; + + _logger.DoneAttemptingToBindModel(bindingContext); + } + + private HeaderValueProvider GetHeaderValueProvider(string headerName, ModelBindingContext bindingContext) + { + var request = bindingContext.HttpContext.Request; + + // Prevent breaking existing users in scenarios where they are binding to a 'string' property + // and expect the whole comma separated string, if any, as a single string and not as a string array. + var values = Array.Empty(); + if (request.Headers.ContainsKey(headerName)) + { + if (bindingContext.ModelMetadata.IsEnumerableType) + { + values = request.Headers.GetCommaSeparatedValues(headerName); + } + else + { + values = new[] { (string)request.Headers[headerName] }; + } + } + + return new HeaderValueProvider(values); + } + + private void BindWithoutInnerBinder(ModelBindingContext bindingContext) + { + var headerName = bindingContext.FieldName; + var request = bindingContext.HttpContext.Request; + object model; if (bindingContext.ModelType == typeof(string)) { @@ -101,7 +185,6 @@ public Task BindModelAsync(ModelBindingContext bindingContext) } _logger.DoneAttemptingToBindModel(bindingContext); - return Task.CompletedTask; } private static object GetCompatibleCollection(ModelBindingContext bindingContext, string[] values) @@ -126,5 +209,34 @@ private static object GetCompatibleCollection(ModelBindingContext bindingContext return collection; } + + private class HeaderValueProvider : IValueProvider + { + private readonly string[] _values; + + public HeaderValueProvider(string[] values) + { + Debug.Assert(values != null); + + _values = values; + } + + public bool ContainsPrefix(string prefix) + { + return _values.Length != 0; + } + + public ValueProviderResult GetValue(string key) + { + if (_values.Length == 0) + { + return ValueProviderResult.None; + } + else + { + return new ValueProviderResult(_values, CultureInfo.InvariantCulture); + } + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs index adefc620c6..c0f46ff93c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders { @@ -21,26 +22,65 @@ public IModelBinder GetBinder(ModelBinderProviderContext context) throw new ArgumentNullException(nameof(context)); } - if (context.BindingInfo.BindingSource != null && - context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Header)) + var bindingInfo = context.BindingInfo; + if (bindingInfo.BindingSource == null || + !bindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Header)) { - var loggerFactory = context.Services.GetRequiredService(); - var logger = loggerFactory.CreateLogger(); + return null; + } + + var modelMetadata = context.Metadata; + var loggerFactory = context.Services.GetRequiredService(); + var logger = loggerFactory.CreateLogger(); - // We only support strings and collections of strings. Some cases can fail - // at runtime due to collections we can't modify. - if (context.Metadata.ModelType == typeof(string) || - context.Metadata.ElementType == typeof(string)) + var options = context.Services.GetRequiredService>().Value; + if (!options.AllowBindingHeaderValuesToNonStringModelTypes) + { + if (modelMetadata.ModelType == typeof(string) || + modelMetadata.ElementType == typeof(string)) { return new HeaderModelBinder(loggerFactory); } else { - logger.CannotCreateHeaderModelBinder(context.Metadata.ModelType); + logger.CannotCreateHeaderModelBinderCompatVersion_2_0(modelMetadata.ModelType); } + + return null; } - return null; + + if (!IsSimpleType(modelMetadata)) + { + logger.CannotCreateHeaderModelBinder(modelMetadata.ModelType); + return null; + } + + // Since we are delegating the binding of the current model type to other binders, modify the + // binding source of the current model type to a non-FromHeader binding source in order to avoid an + // infinite recursion into this binder provider. + var nestedBindingInfo = new BindingInfo(bindingInfo) + { + BindingSource = BindingSource.ModelBinding + }; + + var innerModelBinder = context.CreateBinder( + modelMetadata.GetMetadataForType(modelMetadata.ModelType), + nestedBindingInfo); + + if (innerModelBinder == null) + { + return null; + } + + return new HeaderModelBinder(loggerFactory, innerModelBinder); + } + + // Support binding only to simple types or collection of simple types. + private bool IsSimpleType(ModelMetadata modelMetadata) + { + var metadata = modelMetadata.ElementMetadata ?? modelMetadata; + return !metadata.IsComplexType; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs index e21d62dac1..a6735bdaa4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs @@ -252,21 +252,15 @@ public DefaultModelBinderProviderContext( private DefaultModelBinderProviderContext( DefaultModelBinderProviderContext parent, - ModelMetadata metadata) + ModelMetadata metadata, + BindingInfo bindingInfo) { Metadata = metadata; _factory = parent._factory; MetadataProvider = parent.MetadataProvider; Visited = parent.Visited; - - BindingInfo = new BindingInfo() - { - BinderModelName = metadata.BinderModelName, - BinderType = metadata.BinderType, - BindingSource = metadata.BindingSource, - PropertyFilterProvider = metadata.PropertyFilterProvider, - }; + BindingInfo = bindingInfo; } public override BindingInfo BindingInfo { get; } @@ -280,18 +274,36 @@ private DefaultModelBinderProviderContext( public override IServiceProvider Services => _factory._serviceProvider; public override IModelBinder CreateBinder(ModelMetadata metadata) + { + return CreateBinder( + metadata, + new BindingInfo() + { + BinderModelName = metadata.BinderModelName, + BinderType = metadata.BinderType, + BindingSource = metadata.BindingSource, + PropertyFilterProvider = metadata.PropertyFilterProvider, + }); + } + + public override IModelBinder CreateBinder(ModelMetadata metadata, BindingInfo bindingInfo) { if (metadata == null) { throw new ArgumentNullException(nameof(metadata)); } + if (bindingInfo == null) + { + throw new ArgumentNullException(nameof(bindingInfo)); + } + // For non-root nodes we use the ModelMetadata as the cache token. This ensures that all non-root // nodes with the same metadata will have the same binder. This is OK because for an non-root // node there's no opportunity to customize binding info like there is for a parameter. var token = metadata; - var nestedContext = new DefaultModelBinderProviderContext(this, metadata); + var nestedContext = new DefaultModelBinderProviderContext(this, metadata, bindingInfo); return _factory.CreateBinderCoreCached(nestedContext, token); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index b5db7caa1f..e7e55a8bbb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; @@ -24,6 +25,7 @@ public class MvcOptions : IEnumerable private int _maxModelStateErrors = ModelStateDictionary.DefaultMaxAllowedErrors; // See CompatibilitySwitch.cs for guide on how to implement these. + private readonly CompatibilitySwitch _allowBindingHeaderValuesToNonStringModelTypes; private readonly CompatibilitySwitch _allowCombiningAuthorizeFilters; private readonly CompatibilitySwitch _inputFormatterExceptionPolicy; private readonly CompatibilitySwitch _suppressBindingUndefinedValueToEnumType; @@ -47,12 +49,14 @@ public MvcOptions() ValueProviderFactories = new List(); _allowCombiningAuthorizeFilters = new CompatibilitySwitch(nameof(AllowCombiningAuthorizeFilters)); + _allowBindingHeaderValuesToNonStringModelTypes = new CompatibilitySwitch(nameof(AllowBindingHeaderValuesToNonStringModelTypes)); _inputFormatterExceptionPolicy = new CompatibilitySwitch(nameof(InputFormatterExceptionPolicy), InputFormatterExceptionPolicy.AllExceptions); _suppressBindingUndefinedValueToEnumType = new CompatibilitySwitch(nameof(SuppressBindingUndefinedValueToEnumType)); - + _switches = new ICompatibilitySwitch[] { - _allowCombiningAuthorizeFilters, + _allowCombiningAuthorizeFilters, + _allowBindingHeaderValuesToNonStringModelTypes, _inputFormatterExceptionPolicy, _suppressBindingUndefinedValueToEnumType, }; @@ -108,6 +112,38 @@ public bool AllowCombiningAuthorizeFilters set => _allowCombiningAuthorizeFilters.Value = value; } + /// + /// 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 + /// 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 + /// 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 false unless explicitly configured. + /// + /// + /// If the application's compatibility version is set to or + /// higher then this setting will have the value true unless explicitly configured. + /// + /// + public bool AllowBindingHeaderValuesToNonStringModelTypes + { + get => _allowBindingHeaderValuesToNonStringModelTypes.Value; + set => _allowBindingHeaderValuesToNonStringModelTypes.Value = value; + } + /// /// Gets a Dictionary of CacheProfile Names, which are pre-defined settings for /// response caching. diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderProviderTest.cs index 34b4dd5222..ac60c4b0cd 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderProviderTest.cs @@ -4,12 +4,65 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.ComponentModel; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders { public class HeaderModelBinderProviderTest { + [Theory] + [InlineData(typeof(string))] + [InlineData(typeof(string[]))] + [InlineData(typeof(List))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsBinder_ForStringTypes_And_CompatVersion_2_0( + Type modelType) + { + // Arrange + var provider = new HeaderModelBinderProvider(); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext( + modelType, + allowBindingHeaderValuesToNonStringModelTypes: false); + context.BindingInfo.BindingSource = BindingSource.Header; + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.IsType(result); + } + + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(int?))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(double))] + [InlineData(typeof(double?))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(CarEnumType))] + [InlineData(typeof(CarEnumType?))] + [InlineData(typeof(IEnumerable))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsNull_ForNonStringTypes_And_CompatVersion_2_0( + Type modelType) + { + // Arrange + var provider = new HeaderModelBinderProvider(); + var context = GetTestModelBinderProviderContext( + modelType, + allowBindingHeaderValuesToNonStringModelTypes: false); + context.BindingInfo.BindingSource = BindingSource.Header; + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.Null(result); + } + public static TheoryData NonHeaderBindingSources { get @@ -29,8 +82,9 @@ public void Create_WhenBindingSourceIsNotFromHeader_ReturnsNull(BindingSource so { // Arrange var provider = new HeaderModelBinderProvider(); - - var context = new TestModelBinderProviderContext(typeof(string)); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext(typeof(string)); + context.OnCreatingBinder(modelMetadata => testBinder); context.BindingInfo.BindingSource = source; // Act @@ -40,52 +94,130 @@ public void Create_WhenBindingSourceIsNotFromHeader_ReturnsNull(BindingSource so Assert.Null(result); } - [Fact] - public void Create_WhenBindingSourceIsFromHeader_ReturnsBinder() + [Theory] + [InlineData(typeof(string))] + [InlineData(typeof(bool))] + [InlineData(typeof(int))] + [InlineData(typeof(DateTime))] + [InlineData(typeof(double))] + [InlineData(typeof(CarEnumType))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsBinder_ForSimpleTypes(Type modelType) { // Arrange var provider = new HeaderModelBinderProvider(); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext(modelType); + context.OnCreatingBinder(modelMetadata => testBinder); + context.BindingInfo.BindingSource = BindingSource.Header; + + // Act + var result = provider.GetBinder(context); - var context = new TestModelBinderProviderContext(typeof(string)); + // Assert + var headerModelBinder = Assert.IsType(result); + Assert.Same(testBinder, headerModelBinder.InnerModelBinder); + } + + [Theory] + [InlineData(typeof(bool?))] + [InlineData(typeof(int?))] + [InlineData(typeof(DateTime?))] + [InlineData(typeof(double?))] + [InlineData(typeof(CarEnumType?))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsBinder_ForNullableSimpleTypes(Type modelType) + { + // Arrange + var provider = new HeaderModelBinderProvider(); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext(modelType); + context.OnCreatingBinder(modelMetadata => testBinder); context.BindingInfo.BindingSource = BindingSource.Header; // Act var result = provider.GetBinder(context); // Assert - Assert.IsType(result); + var headerModelBinder = Assert.IsType(result); + Assert.Same(testBinder, headerModelBinder.InnerModelBinder); } [Theory] - [InlineData(typeof(string))] - [InlineData(typeof(IEnumerable))] [InlineData(typeof(string[]))] - [InlineData(typeof(Collection))] - public void Create_WhenModelTypeIsSupportedType_ReturnsBinder(Type modelType) + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(List))] + [InlineData(typeof(Collection))] + [InlineData(typeof(float[]))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(List))] + [InlineData(typeof(ICollection))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsBinder_ForCollectionOfSimpleTypes(Type modelType) { // Arrange var provider = new HeaderModelBinderProvider(); - - var context = new TestModelBinderProviderContext(modelType); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext(modelType); + context.OnCreatingBinder(modelMetadata => testBinder); context.BindingInfo.BindingSource = BindingSource.Header; // Act var result = provider.GetBinder(context); // Assert - Assert.IsType(result); + var headerModelBinder = Assert.IsType(result); + Assert.Same(testBinder, headerModelBinder.InnerModelBinder); } [Theory] - [InlineData(typeof(Dictionary))] - [InlineData(typeof(Collection))] + [InlineData(typeof(CustomerStruct))] + [InlineData(typeof(IEnumerable))] [InlineData(typeof(Person))] - public void Create_WhenModelTypeIsUnsupportedType_ReturnsNull(Type modelType) + [InlineData(typeof(IEnumerable))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsNull_ForNonSimpleModelType(Type modelType) { // Arrange var provider = new HeaderModelBinderProvider(); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext(modelType); + context.OnCreatingBinder(modelMetadata => testBinder); + context.BindingInfo.BindingSource = BindingSource.Header; - var context = new TestModelBinderProviderContext(modelType); + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.Null(result); + } + + [Theory] + [InlineData(typeof(ProductWithTypeConverter))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(CustomerStructWithTypeConverter))] + [InlineData(typeof(IEnumerable))] + public void Create_WhenBindingSourceIsFromHeader_ReturnsBinder_ForNonSimpleModelType_HavingTypeConverter( + Type modelType) + { + // Arrange + var provider = new HeaderModelBinderProvider(); + var testBinder = Mock.Of(); + var context = GetTestModelBinderProviderContext(modelType); + context.OnCreatingBinder(modelMetadata => testBinder); + context.BindingInfo.BindingSource = BindingSource.Header; + + // Act + var result = provider.GetBinder(context); + + // Assert + var headerModelBinder = Assert.IsType(result); + Assert.Same(testBinder, headerModelBinder.InnerModelBinder); + } + + [Fact] + public void Create_WhenBindingSourceIsFromHeader_NoInnerBinderAvailable_ReturnsNull() + { + // Arrange + var provider = new HeaderModelBinderProvider(); + var context = GetTestModelBinderProviderContext(typeof(string)); + context.OnCreatingBinder(modelMetadata => null); context.BindingInfo.BindingSource = BindingSource.Header; // Act @@ -95,11 +227,50 @@ public void Create_WhenModelTypeIsUnsupportedType_ReturnsNull(Type modelType) Assert.Null(result); } + private TestModelBinderProviderContext GetTestModelBinderProviderContext( + Type modelType, + bool allowBindingHeaderValuesToNonStringModelTypes = true) + { + var context = new TestModelBinderProviderContext(modelType); + var options = context.Services.GetRequiredService>().Value; + options.AllowBindingHeaderValuesToNonStringModelTypes = allowBindingHeaderValuesToNonStringModelTypes; + return context; + } + + private enum CarEnumType + { + Sedan, + Coupe + } + + private struct CustomerStruct + { + public string Name { get; set; } + } + + [TypeConverter(typeof(CanConvertFromStringConverter))] + private struct CustomerStructWithTypeConverter + { + public string Name { get; set; } + } + private class Person { public string Name { get; set; } + } + + [TypeConverter(typeof(CanConvertFromStringConverter))] + private class ProductWithTypeConverter + { + public string Name { get; set; } + } - public int Age { get; set; } + private class CanConvertFromStringConverter : TypeConverter + { + public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) + { + return sourceType == typeof(string); + } } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderTests.cs index dcc751d20a..2b750fd67f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/HeaderModelBinderTests.cs @@ -3,44 +3,46 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using Microsoft.Extensions.Primitives; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders { public class HeaderModelBinderTests { - [Theory] - [InlineData(typeof(string))] - [InlineData(typeof(string[]))] - [InlineData(typeof(object))] - [InlineData(typeof(int))] - [InlineData(typeof(int[]))] - [InlineData(typeof(BindingSource))] - public async Task BindModelAsync_ReturnsNonEmptyResult_ForAllTypes_WithHeaderBindingSource(Type type) + public static TheoryData HeaderModelBinderWithoutInnerBinderData { - // Arrange - var binder = new HeaderModelBinder(NullLoggerFactory.Instance); - var bindingContext = GetBindingContext(type); - - // Act - await binder.BindModelAsync(bindingContext); - - // Assert - Assert.False(bindingContext.Result.IsModelSet); + get + { + var data = new TheoryData(); +#pragma warning disable CS0618 + data.Add(new HeaderModelBinder()); +#pragma warning restore CS0618 + data.Add(new HeaderModelBinder(NullLoggerFactory.Instance)); + return data; + } } - [Fact] - public async Task HeaderBinder_BindsHeaders_ToStringCollection() + [Theory] + [MemberData(nameof(HeaderModelBinderWithoutInnerBinderData))] + public async Task HeaderBinder_BindsHeaders_ToStringCollection_WithoutInnerModelBinder( + HeaderModelBinder binder) { // Arrange var type = typeof(string[]); var header = "Accept"; var headerValue = "application/json,text/json"; - var binder = new HeaderModelBinder(NullLoggerFactory.Instance); - var bindingContext = GetBindingContext(type); + + var bindingContext = CreateContext(type); bindingContext.FieldName = header; bindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); @@ -53,15 +55,15 @@ public async Task HeaderBinder_BindsHeaders_ToStringCollection() Assert.Equal(headerValue.Split(','), bindingContext.Result.Model); } - [Fact] - public async Task HeaderBinder_BindsHeaders_ToStringType() + [Theory] + [MemberData(nameof(HeaderModelBinderWithoutInnerBinderData))] + public async Task HeaderBinder_BindsHeaders_ToStringType_WithoutInnerModelBinder(HeaderModelBinder binder) { // Arrange var type = typeof(string); var header = "User-Agent"; var headerValue = "UnitTest"; - var binder = new HeaderModelBinder(NullLoggerFactory.Instance); - var bindingContext = GetBindingContext(type); + var bindingContext = CreateContext(type); bindingContext.FieldName = header; bindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); @@ -81,13 +83,16 @@ public async Task HeaderBinder_BindsHeaders_ToStringType() [InlineData(typeof(List))] [InlineData(typeof(LinkedList))] [InlineData(typeof(StringList))] - public async Task HeaderBinder_BindsHeaders_ForCollectionsItCanCreate(Type destinationType) + public async Task HeaderBinder_BindsHeaders_ForCollectionsItCanCreate_WithoutInnerModelBinder( + Type destinationType) { // Arrange var header = "Accept"; var headerValue = "application/json,text/json"; - var binder = new HeaderModelBinder(NullLoggerFactory.Instance); - var bindingContext = GetBindingContext(destinationType); +#pragma warning disable CS0618 + var binder = new HeaderModelBinder(); +#pragma warning restore CS0618 + var bindingContext = CreateContext(destinationType); bindingContext.FieldName = header; bindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); @@ -102,36 +107,105 @@ public async Task HeaderBinder_BindsHeaders_ForCollectionsItCanCreate(Type desti } [Fact] - public async Task HeaderBinder_ReturnsResult_ForReadOnlyDestination() + public async Task HeaderBinder_BindsHeaders_ToStringCollection() { // Arrange - var header = "Accept"; + var type = typeof(string[]); var headerValue = "application/json,text/json"; - var binder = new HeaderModelBinder(NullLoggerFactory.Instance); - var bindingContext = GetBindingContextForReadOnlyArray(); + var bindingContext = CreateContext(type); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", new[] { headerValue }); - bindingContext.FieldName = header; - bindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal(headerValue.Split(','), bindingContext.Result.Model); + } + + public static TheoryData BinderHeaderToSimpleTypesData + { + get + { + var guid = new Guid("3916A5B1-5FE4-4E09-9812-5CDC127FA5B1"); + + return new TheoryData() + { + { "10", typeof(int), 10 }, + { "10.50", typeof(double), 10.50 }, + { "10.50", typeof(IEnumerable), new List() { 10.50 } }, + { "Sedan", typeof(CarType), CarType.Sedan }, + { "", typeof(CarType?), null }, + { "", typeof(string[]), Array.Empty() }, + { null, typeof(string[]), Array.Empty() }, + { "", typeof(IEnumerable), new List() }, + { null, typeof(IEnumerable), new List() }, + { guid.ToString(), typeof(Guid), guid }, + { "foo", typeof(string), "foo" }, + { "foo, bar", typeof(string), "foo, bar" }, + { "foo, bar", typeof(string[]), new[]{ "foo", "bar" } }, + { "foo, \"bar\"", typeof(string[]), new[]{ "foo", "bar" } }, + { "\"foo,bar\"", typeof(string[]), new[]{ "foo,bar" } } + }; + } + } + + [Theory] + [MemberData(nameof(BinderHeaderToSimpleTypesData))] + public async Task HeaderBinder_BindsHeaders_ToSimpleTypes( + string headerValue, + Type modelType, + object expectedModel) + { + // Arrange + var bindingContext = CreateContext(modelType); + var binder = CreateBinder(bindingContext); + + if (headerValue != null) + { + bindingContext.HttpContext.Request.Headers.Add("Header", headerValue); + } // Act await binder.BindModelAsync(bindingContext); // Assert Assert.True(bindingContext.Result.IsModelSet); - Assert.NotNull(bindingContext.Result.Model); + Assert.Equal(expectedModel, bindingContext.Result.Model); } - [Fact] - public async Task HeaderBinder_ReturnsFailedResult_ForCollectionsItCannotCreate() + [Theory] + [InlineData(typeof(CarType?), null)] + [InlineData(typeof(int?), null)] + public async Task HeaderBinder_DoesNotSetModel_ForHeaderNotPresentOnRequest( + Type modelType, + object expectedModel) { // Arrange - var header = "Accept"; - var headerValue = "application/json,text/json"; - var binder = new HeaderModelBinder(NullLoggerFactory.Instance); - var bindingContext = GetBindingContext(typeof(ISet)); + var bindingContext = CreateContext(modelType); + var binder = CreateBinder(bindingContext); - bindingContext.FieldName = header; - bindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + } + + [Theory] + [InlineData(typeof(string[]), null)] + [InlineData(typeof(IEnumerable), null)] + public async Task HeaderBinder_DoesNotCreateEmptyCollection_ForNonTopLevelObjects( + Type modelType, + object expectedModel) + { + // Arrange + var bindingContext = CreateContext(modelType); + bindingContext.IsTopLevelObject = false; + var binder = CreateBinder(bindingContext); + // No header on the request that the header value provider is looking for // Act await binder.BindModelAsync(bindingContext); @@ -141,47 +215,227 @@ public async Task HeaderBinder_ReturnsFailedResult_ForCollectionsItCannotCreate( Assert.Null(bindingContext.Result.Model); } - private static DefaultModelBindingContext GetBindingContext(Type modelType) + [Theory] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(ICollection))] + [InlineData(typeof(IList))] + [InlineData(typeof(List))] + [InlineData(typeof(LinkedList))] + [InlineData(typeof(StringList))] + public async Task HeaderBinder_BindsHeaders_ForCollectionsItCanCreate(Type destinationType) { - var metadataProvider = new TestModelMetadataProvider(); - metadataProvider.ForType(modelType).BindingDetails(d => d.BindingSource = BindingSource.Header); - var modelMetadata = metadataProvider.GetMetadataForType(modelType); + // Arrange + var headerValue = "application/json,text/json"; + var bindingContext = CreateContext(destinationType); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", new[] { headerValue }); - return GetBindingContext(metadataProvider, modelMetadata); + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.IsAssignableFrom(destinationType, bindingContext.Result.Model); + Assert.Equal(headerValue.Split(','), bindingContext.Result.Model as IEnumerable); } - private static DefaultModelBindingContext GetBindingContextForReadOnlyArray() + [Fact] + public async Task HeaderBinder_ReturnsResult_ForReadOnlyDestination() { - var metadataProvider = new TestModelMetadataProvider(); - metadataProvider - .ForProperty(nameof(ModelWithReadOnlyArray.ArrayProperty)) - .BindingDetails(bd => bd.BindingSource = BindingSource.Header); - var modelMetadata = metadataProvider.GetMetadataForProperty( - typeof(ModelWithReadOnlyArray), - nameof(ModelWithReadOnlyArray.ArrayProperty)); + // Arrange + var bindingContext = CreateContext(GetMetadataForReadOnlyArray()); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", "application/json,text/json"); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.NotNull(bindingContext.Result.Model); + } + + [Fact] + public async Task HeaderBinder_ResetsTheBindingScope_GivingOriginalValueProvider() + { + // Arrange + var expectedValueProvider = Mock.Of(); + var bindingContext = CreateContext(GetMetadataForType(typeof(string)), expectedValueProvider); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", "application/json,text/json"); + + // Act + await binder.BindModelAsync(bindingContext); - return GetBindingContext(metadataProvider, modelMetadata); + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal("application/json,text/json", bindingContext.Result.Model); + Assert.Same(expectedValueProvider, bindingContext.ValueProvider); } - private static DefaultModelBindingContext GetBindingContext( - IModelMetadataProvider metadataProvider, - ModelMetadata modelMetadata) + [Fact] + public async Task HeaderBinder_UsesValues_OnlyFromHeaderValueProvider() { - var bindingContext = new DefaultModelBindingContext + // Arrange + var testValueProvider = new Mock(); + testValueProvider + .Setup(vp => vp.ContainsPrefix(It.IsAny())) + .Returns(true); + testValueProvider + .Setup(vp => vp.GetValue(It.IsAny())) + .Returns(new ValueProviderResult(new StringValues("foo,bar"))); + var bindingContext = CreateContext(GetMetadataForType(typeof(string)), testValueProvider.Object); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", "application/json,text/json"); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Equal("application/json,text/json", bindingContext.Result.Model); + Assert.Same(testValueProvider.Object, bindingContext.ValueProvider); + } + + [Theory] + [InlineData(typeof(int), "not-an-integer")] + [InlineData(typeof(double), "not-an-double")] + [InlineData(typeof(CarType?), "boo")] + public async Task HeaderBinder_BindModelAsync_AddsErrorToModelState_OnInvalidInput( + Type modelType, + string headerValue) + { + // Arrange + var bindingContext = CreateContext(modelType); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", headerValue); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + var entry = bindingContext.ModelState["someprefix.Header"]; + Assert.NotNull(entry); + var error = Assert.Single(entry.Errors); + Assert.Equal($"The value '{headerValue}' is not valid.", error.ErrorMessage); + } + + [Theory] + [InlineData(typeof(int[]), "a, b")] + [InlineData(typeof(IEnumerable), "a, b")] + [InlineData(typeof(ICollection), "a, b")] + public async Task HeaderBinder_BindModelAsync_AddsErrorToModelState_OnInvalid_CollectionInput( + Type modelType, + string headerValue) + { + // Arrange + var headerValues = headerValue.Split(',').Select(s => s.Trim()).ToArray(); + var bindingContext = CreateContext(modelType); + var binder = CreateBinder(bindingContext); + bindingContext.HttpContext.Request.Headers.Add("Header", headerValue); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + var entry = bindingContext.ModelState["someprefix.Header"]; + Assert.NotNull(entry); + Assert.Equal(2, entry.Errors.Count); + Assert.Equal($"The value '{headerValues[0]}' is not valid.", entry.Errors[0].ErrorMessage); + Assert.Equal($"The value '{headerValues[1]}' is not valid.", entry.Errors[1].ErrorMessage); + } + + private static DefaultModelBindingContext CreateContext( + Type modelType, + bool allowBindingHeaderValuesToNonStringModelTypes = true) + { + return CreateContext( + metadata: GetMetadataForType(modelType), + valueProvider: null, + allowBindingHeaderValuesToNonStringModelTypes: allowBindingHeaderValuesToNonStringModelTypes); + } + + private static DefaultModelBindingContext CreateContext( + ModelMetadata metadata, + IValueProvider valueProvider = null, + bool allowBindingHeaderValuesToNonStringModelTypes = true) + { + if (valueProvider == null) + { + valueProvider = Mock.Of(); + } + + var options = new MvcOptions() { + AllowBindingHeaderValuesToNonStringModelTypes = allowBindingHeaderValuesToNonStringModelTypes + }; + var setup = new MvcCoreMvcOptionsSetup(new TestHttpRequestStreamReaderFactory()); + setup.Configure(options); + + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddSingleton(Options.Create(options)); + var serviceProvider = services.BuildServiceProvider(); + + var headerName = "Header"; + + return new DefaultModelBindingContext() + { + IsTopLevelObject = true, + ModelMetadata = metadata, + BinderModelName = metadata.BinderModelName, + BindingSource = metadata.BindingSource, + + // HeaderModelBinder must always use the field name when getting the values from header value provider + // but add keys into ModelState using the ModelName. This is for back compat reasons. + ModelName = $"somePrefix.{headerName}", + FieldName = headerName, + + ValueProvider = valueProvider, + ModelState = new ModelStateDictionary(), ActionContext = new ActionContext() { - HttpContext = new DefaultHttpContext(), + HttpContext = new DefaultHttpContext() + { + RequestServices = serviceProvider + } }, - ModelMetadata = modelMetadata, - ModelName = "modelName", - FieldName = "modelName", - ModelState = new ModelStateDictionary(), - BinderModelName = modelMetadata.BinderModelName, - BindingSource = modelMetadata.BindingSource, }; + } - return bindingContext; + private static IModelBinder CreateBinder(DefaultModelBindingContext bindingContext) + { + var factory = TestModelBinderFactory.Create(bindingContext.HttpContext.RequestServices); + var metadata = bindingContext.ModelMetadata; + return factory.CreateBinder(new ModelBinderFactoryContext() + { + Metadata = metadata, + BindingInfo = new BindingInfo() + { + BinderModelName = metadata.BinderModelName, + BinderType = metadata.BinderType, + BindingSource = metadata.BindingSource, + PropertyFilterProvider = metadata.PropertyFilterProvider, + }, + }); + } + + private static ModelMetadata GetMetadataForType(Type modelType) + { + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider.ForType(modelType).BindingDetails(d => d.BindingSource = BindingSource.Header); + return metadataProvider.GetMetadataForType(modelType); + } + + private static ModelMetadata GetMetadataForReadOnlyArray() + { + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForProperty(nameof(ModelWithReadOnlyArray.ArrayProperty)) + .BindingDetails(bd => bd.BindingSource = BindingSource.Header); + return metadataProvider.GetMetadataForProperty( + typeof(ModelWithReadOnlyArray), + nameof(ModelWithReadOnlyArray.ArrayProperty)); } private class ModelWithReadOnlyArray @@ -192,5 +446,11 @@ private class ModelWithReadOnlyArray private class StringList : List { } + + private enum CarType + { + Sedan, + Coupe + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs index 707c277d10..514e012c76 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs @@ -6,11 +6,14 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.ModelBinding { public class TestModelBinderProviderContext : ModelBinderProviderContext { + private BindingInfo _bindingInfo; + // Has to be internal because TestModelMetadataProvider is 'shared' code. internal static readonly TestModelMetadataProvider CachedMetadataProvider = new TestModelMetadataProvider(); @@ -18,10 +21,15 @@ public class TestModelBinderProviderContext : ModelBinderProviderContext new List>(); public TestModelBinderProviderContext(Type modelType) + : this(modelType, bindingInfo: null) + { + } + + public TestModelBinderProviderContext(Type modelType, BindingInfo bindingInfo) { Metadata = CachedMetadataProvider.GetMetadataForType(modelType); MetadataProvider = CachedMetadataProvider; - BindingInfo = new BindingInfo() + _bindingInfo = bindingInfo ?? new BindingInfo { BinderModelName = Metadata.BinderModelName, BinderType = Metadata.BinderType, @@ -31,22 +39,7 @@ public TestModelBinderProviderContext(Type modelType) Services = GetServices(); } - public TestModelBinderProviderContext(ModelMetadata metadata, BindingInfo bindingInfo) - { - Metadata = metadata; - BindingInfo = bindingInfo ?? new BindingInfo - { - BinderModelName = metadata.BinderModelName, - BinderType = metadata.BinderType, - BindingSource = metadata.BindingSource, - PropertyFilterProvider = metadata.PropertyFilterProvider, - }; - - MetadataProvider = CachedMetadataProvider; - Services = GetServices(); - } - - public override BindingInfo BindingInfo { get; } + public override BindingInfo BindingInfo => _bindingInfo; public override ModelMetadata Metadata { get; } @@ -68,6 +61,12 @@ public override IModelBinder CreateBinder(ModelMetadata metadata) return null; } + public override IModelBinder CreateBinder(ModelMetadata metadata, BindingInfo bindingInfo) + { + _bindingInfo = bindingInfo; + return this.CreateBinder(metadata); + } + public void OnCreatingBinder(Func binderCreator) { _binderCreators.Add(binderCreator); @@ -82,6 +81,7 @@ private static IServiceProvider GetServices() { var services = new ServiceCollection(); services.AddSingleton(); + services.AddSingleton(Options.Create(new MvcOptions())); return services.BuildServiceProvider(); } } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs index edf3d7bec3..8088c0302f 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs @@ -3,7 +3,9 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.ComponentModel.DataAnnotations; +using System.Globalization; using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -31,7 +33,6 @@ private class Address public async Task BindPropertyFromHeader_NoData_UsesFullPathAsKeyForModelStateErrors() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); var parameter = new ParameterDescriptor() { Name = "Parameter1", @@ -43,7 +44,8 @@ public async Task BindPropertyFromHeader_NoData_UsesFullPathAsKeyForModelStateEr }; // Do not add any headers. - var testContext = ModelBindingTestHelper.GetTestContext(); + var testContext = GetModelBindingTestContext(); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var modelState = testContext.ModelState; // Act @@ -70,7 +72,6 @@ public async Task BindPropertyFromHeader_NoData_UsesFullPathAsKeyForModelStateEr public async Task BindPropertyFromHeader_WithPrefix_GetsBound() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); var parameter = new ParameterDescriptor() { Name = "Parameter1", @@ -81,8 +82,9 @@ public async Task BindPropertyFromHeader_WithPrefix_GetsBound() ParameterType = typeof(Person) }; - var testContext = ModelBindingTestHelper.GetTestContext( + var testContext = GetModelBindingTestContext( request => request.Headers.Add("Header", new[] { "someValue" })); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var modelState = testContext.ModelState; // Act @@ -106,7 +108,7 @@ public async Task BindPropertyFromHeader_WithPrefix_GetsBound() Assert.Empty(entry.Value.Errors); Assert.Equal(ModelValidationState.Valid, entry.Value.ValidationState); Assert.Equal("someValue", entry.Value.AttemptedValue); - Assert.Equal(new string[] { "someValue" }, entry.Value.RawValue); + Assert.Equal("someValue", entry.Value.RawValue); } // The scenario is interesting as we to bind the top level model we fallback to empty prefix, @@ -115,7 +117,6 @@ public async Task BindPropertyFromHeader_WithPrefix_GetsBound() public async Task BindPropertyFromHeader_WithData_WithEmptyPrefix_GetsBound() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); var parameter = new ParameterDescriptor() { Name = "Parameter1", @@ -123,8 +124,9 @@ public async Task BindPropertyFromHeader_WithData_WithEmptyPrefix_GetsBound() ParameterType = typeof(Person) }; - var testContext = ModelBindingTestHelper.GetTestContext( + var testContext = GetModelBindingTestContext( request => request.Headers.Add("Header", new[] { "someValue" })); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var modelState = testContext.ModelState; // Act @@ -148,7 +150,7 @@ public async Task BindPropertyFromHeader_WithData_WithEmptyPrefix_GetsBound() Assert.Empty(entry.Value.Errors); Assert.Equal(ModelValidationState.Valid, entry.Value.ValidationState); Assert.Equal("someValue", entry.Value.AttemptedValue); - Assert.Equal(new string[] { "someValue" }, entry.Value.RawValue); + Assert.Equal("someValue", entry.Value.RawValue); } private class ListContainer1 @@ -161,7 +163,6 @@ private class ListContainer1 public async Task BindCollectionPropertyFromHeader_WithData_IsBound() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); var parameter = new ParameterDescriptor { Name = "Parameter1", @@ -169,8 +170,9 @@ public async Task BindCollectionPropertyFromHeader_WithData_IsBound() ParameterType = typeof(ListContainer1), }; - var testContext = ModelBindingTestHelper.GetTestContext( + var testContext = GetModelBindingTestContext( request => request.Headers.Add("Header", new[] { "someValue" })); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var modelState = testContext.ModelState; // Act @@ -195,7 +197,7 @@ public async Task BindCollectionPropertyFromHeader_WithData_IsBound() Assert.Empty(modelStateEntry.Errors); Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState); Assert.Equal("someValue", modelStateEntry.AttemptedValue); - Assert.Equal(new[] { "someValue" }, modelStateEntry.RawValue); + Assert.Equal("someValue", modelStateEntry.RawValue); } private class ListContainer2 @@ -208,7 +210,6 @@ private class ListContainer2 public async Task BindReadOnlyCollectionPropertyFromHeader_WithData_IsBound() { // Arrange - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); var parameter = new ParameterDescriptor { Name = "Parameter1", @@ -216,8 +217,9 @@ public async Task BindReadOnlyCollectionPropertyFromHeader_WithData_IsBound() ParameterType = typeof(ListContainer2), }; - var testContext = ModelBindingTestHelper.GetTestContext( + var testContext = GetModelBindingTestContext( request => request.Headers.Add("Header", new[] { "someValue" })); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); var modelState = testContext.ModelState; // Act @@ -242,7 +244,7 @@ public async Task BindReadOnlyCollectionPropertyFromHeader_WithData_IsBound() Assert.Empty(modelStateEntry.Errors); Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState); Assert.Equal("someValue", modelStateEntry.AttemptedValue); - Assert.Equal(new[] { "someValue" }, modelStateEntry.RawValue); + Assert.Equal("someValue", modelStateEntry.RawValue); } [Theory] @@ -251,20 +253,19 @@ public async Task BindReadOnlyCollectionPropertyFromHeader_WithData_IsBound() public async Task BindParameterFromHeader_WithData_WithPrefix_ModelGetsBound(Type modelType, string value) { // Arrange - object expectedValue; + string expectedAttemptedValue; object expectedRawValue; if (modelType == typeof(string)) { - expectedValue = value; - expectedRawValue = new string[] { value }; + expectedAttemptedValue = value; + expectedRawValue = value; } else { - expectedValue = value.Split(',').Select(v => v.Trim()).ToArray(); - expectedRawValue = expectedValue; + expectedAttemptedValue = value.Replace(" ", ""); + expectedRawValue = value.Split(',').Select(v => v.Trim()).ToArray(); } - var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); var parameter = new ParameterDescriptor { Name = "Parameter1", @@ -276,8 +277,9 @@ public async Task BindParameterFromHeader_WithData_WithPrefix_ModelGetsBound(Typ ParameterType = modelType }; - Action action = r => r.Headers.Add("CustomParameter", new[] { value }); - var testContext = ModelBindingTestHelper.GetTestContext(action); + Action action = r => r.Headers.Add("CustomParameter", new[] { expectedAttemptedValue }); + var testContext = GetModelBindingTestContext(action); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); // Do not add any headers. var httpContext = testContext.HttpContext; @@ -301,8 +303,221 @@ public async Task BindParameterFromHeader_WithData_WithPrefix_ModelGetsBound(Typ Assert.Equal("CustomParameter", entry.Key); Assert.Empty(entry.Value.Errors); Assert.Equal(ModelValidationState.Valid, entry.Value.ValidationState); - Assert.Equal(value, entry.Value.AttemptedValue); + Assert.Equal(expectedAttemptedValue, entry.Value.AttemptedValue); Assert.Equal(expectedRawValue, entry.Value.RawValue); } + + [Fact] + public async Task BindPropertyFromHeader_WithPrefix_GetsBound_ForSimpleTypes() + { + // Arrange + var parameter = new ParameterDescriptor() + { + Name = "Parameter1", + BindingInfo = new BindingInfo() + { + BinderModelName = "prefix", + }, + ParameterType = typeof(Product) + }; + + var testContext = GetModelBindingTestContext( + request => + { + request.Headers.Add("NoCommaString", "someValue"); + request.Headers.Add("OneCommaSeparatedString", "one, two, three"); + request.Headers.Add("IntProperty", "10"); + request.Headers.Add("NullableIntProperty", "300"); + request.Headers.Add("ArrayOfString", "first, second"); + request.Headers.Add("EnumerableOfDouble", "10.51, 45.44"); + request.Headers.Add("ListOfEnum", "Sedan, Coupe"); + request.Headers.Add("ListOfOrderWithTypeConverter", "10"); + }); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + var product = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(product); + Assert.NotNull(product.Manufacturer); + Assert.Equal("someValue", product.Manufacturer.NoCommaString); + Assert.Equal("one, two, three", product.Manufacturer.OneCommaSeparatedStringProperty); + Assert.Equal(10, product.Manufacturer.IntProperty); + Assert.Equal(300, product.Manufacturer.NullableIntProperty); + Assert.Null(product.Manufacturer.NullableLongProperty); + Assert.Equal(new[] { "first", "second" }, product.Manufacturer.ArrayOfString); + Assert.Equal(new double[] { 10.51, 45.44 }, product.Manufacturer.EnumerableOfDoubleProperty); + Assert.Equal(new CarType[] { CarType.Sedan, CarType.Coupe }, product.Manufacturer.ListOfEnum); + var orderWithTypeConverter = Assert.Single(product.Manufacturer.ListOfOrderWithTypeConverterProperty); + Assert.Equal(10, orderWithTypeConverter.Id); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Collection( + modelState.OrderBy(kvp => kvp.Key), + kvp => + { + Assert.Equal("prefix.Manufacturer.ArrayOfString", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("first,second", entry.AttemptedValue); + Assert.Equal(new[] { "first", "second" }, entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.EnumerableOfDouble", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("10.51,45.44", entry.AttemptedValue); + Assert.Equal(new[] { "10.51", "45.44" }, entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.IntProperty", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("10", entry.AttemptedValue); + Assert.Equal("10", entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.ListOfEnum", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("Sedan,Coupe", entry.AttemptedValue); + Assert.Equal(new[] { "Sedan", "Coupe" }, entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.ListOfOrderWithTypeConverter", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("10", entry.AttemptedValue); + Assert.Equal("10", entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.NoCommaString", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("someValue", entry.AttemptedValue); + Assert.Equal("someValue", entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.NullableIntProperty", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("300", entry.AttemptedValue); + Assert.Equal("300", entry.RawValue); + }, + kvp => + { + Assert.Equal("prefix.Manufacturer.OneCommaSeparatedString", kvp.Key); + var entry = kvp.Value; + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.Equal("one, two, three", entry.AttemptedValue); + Assert.Equal("one, two, three", entry.RawValue); + }); + } + + private ModelBindingTestContext GetModelBindingTestContext( + Action updateRequest = null, + Action updateOptions = null) + { + if (updateOptions == null) + { + updateOptions = o => + { + o.AllowBindingHeaderValuesToNonStringModelTypes = true; + }; + } + + return ModelBindingTestHelper.GetTestContext(updateRequest, updateOptions); + } + + private class Product + { + public Manufacturer Manufacturer { get; set; } + } + + private class Manufacturer + { + [FromHeader] + public string NoCommaString { get; set; } + + [FromHeader(Name = "OneCommaSeparatedString")] + public string OneCommaSeparatedStringProperty { get; set; } + + [FromHeader] + public int IntProperty { get; set; } + + [FromHeader] + public int? NullableIntProperty { get; set; } + + [FromHeader] + public long? NullableLongProperty { get; set; } + + [FromHeader] + public string[] ArrayOfString { get; set; } + + [FromHeader(Name = "EnumerableOfDouble")] + public IEnumerable EnumerableOfDoubleProperty { get; set; } + + [FromHeader] + public List ListOfEnum { get; set; } + + [FromHeader(Name = "ListOfOrderWithTypeConverter")] + public List ListOfOrderWithTypeConverterProperty { get; set; } + } + + private enum CarType + { + Coupe, + Sedan + } + + [TypeConverter(typeof(CanConvertFromStringConverter))] + private class OrderWithTypeConverter : IEquatable + { + public int Id { get; set; } + + public int ItemCount { get; set; } + + public bool Equals(OrderWithTypeConverter other) + { + return Id == other.Id; + } + } + + private class CanConvertFromStringConverter : TypeConverter + { + public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) + { + return sourceType == typeof(string); + } + + public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value) + { + var id = value.ToString(); + return new OrderWithTypeConverter() { Id = int.Parse(id) }; + } + } } } \ 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 0285f79a63..d5e069c864 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -60,6 +60,18 @@ public static ParameterBinder GetParameterBinder( } } + public static ParameterBinder GetParameterBinder(IServiceProvider serviceProvider) + { + var metadataProvider = serviceProvider.GetRequiredService(); + var options = serviceProvider.GetRequiredService>(); + + return new ParameterBinder( + metadataProvider, + new ModelBinderFactory(metadataProvider, options, serviceProvider), + new CompositeModelValidatorProvider(GetModelValidatorProviders(options)), + NullLoggerFactory.Instance); + } + public static ParameterBinder GetParameterBinder( IModelMetadataProvider metadataProvider, IModelBinderProvider binderProvider = null, @@ -77,7 +89,7 @@ public static ParameterBinder GetParameterBinder( return new ParameterBinder( metadataProvider, - GetModelBinderFactory(metadataProvider, options), + new ModelBinderFactory(metadataProvider, options, services), new CompositeModelValidatorProvider(GetModelValidatorProviders(options)), NullLoggerFactory.Instance); } @@ -130,7 +142,7 @@ private static HttpContext GetHttpContext( return httpContext; } - private static IServiceProvider GetServices(Action updateOptions = null) + public static IServiceProvider GetServices(Action updateOptions = null) { var serviceCollection = new ServiceCollection(); serviceCollection.AddAuthorization(); diff --git a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs index f57e48c487..998343d7bb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs @@ -34,6 +34,7 @@ public void CompatibilitySwitches_Version_2_0() // Assert Assert.False(mvcOptions.AllowCombiningAuthorizeFilters); + Assert.False(mvcOptions.AllowBindingHeaderValuesToNonStringModelTypes); Assert.False(mvcOptions.SuppressBindingUndefinedValueToEnumType); Assert.Equal(InputFormatterExceptionPolicy.AllExceptions, mvcOptions.InputFormatterExceptionPolicy); Assert.False(jsonOptions.AllowInputFormatterExceptionMessages); @@ -57,6 +58,7 @@ public void CompatibilitySwitches_Version_2_1() // Assert Assert.True(mvcOptions.AllowCombiningAuthorizeFilters); + Assert.True(mvcOptions.AllowBindingHeaderValuesToNonStringModelTypes); Assert.True(mvcOptions.SuppressBindingUndefinedValueToEnumType); Assert.Equal(InputFormatterExceptionPolicy.MalformedInputExceptions, mvcOptions.InputFormatterExceptionPolicy); Assert.True(jsonOptions.AllowInputFormatterExceptionMessages); @@ -80,6 +82,7 @@ public void CompatibilitySwitches_Version_Latest() // Assert Assert.True(mvcOptions.AllowCombiningAuthorizeFilters); + Assert.True(mvcOptions.AllowBindingHeaderValuesToNonStringModelTypes); Assert.True(mvcOptions.SuppressBindingUndefinedValueToEnumType); Assert.Equal(InputFormatterExceptionPolicy.MalformedInputExceptions, mvcOptions.InputFormatterExceptionPolicy); Assert.True(jsonOptions.AllowInputFormatterExceptionMessages); diff --git a/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelBinderFactory.cs b/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelBinderFactory.cs index d524adc1ee..87ed105988 100644 --- a/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelBinderFactory.cs +++ b/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelBinderFactory.cs @@ -3,15 +3,24 @@ using System; using Microsoft.AspNetCore.Mvc.Internal; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.ModelBinding { public class TestModelBinderFactory : ModelBinderFactory { + public static TestModelBinderFactory Create(IServiceProvider serviceProvider) + { + var options = serviceProvider.GetRequiredService>(); + return new TestModelBinderFactory( + TestModelMetadataProvider.CreateDefaultProvider(), + options, + serviceProvider); + } + public static TestModelBinderFactory Create(params IModelBinderProvider[] providers) { return Create(null, providers); @@ -58,14 +67,23 @@ public static TestModelBinderFactory CreateDefault( } protected TestModelBinderFactory(IModelMetadataProvider metadataProvider, IOptions options) - : base(metadataProvider, options, GetServices()) + : this(metadataProvider, options, GetServices(options)) + { + } + + protected TestModelBinderFactory( + IModelMetadataProvider metadataProvider, + IOptions options, + IServiceProvider serviceProvider) + : base(metadataProvider, options, serviceProvider) { } - private static IServiceProvider GetServices() + private static IServiceProvider GetServices(IOptions options) { var services = new ServiceCollection(); services.AddSingleton(); + services.AddSingleton(options); return services.BuildServiceProvider(); } }