diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs index 400e97c76a..e487da16af 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs @@ -46,6 +46,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal options.ModelBinderProviders.Add(new BodyModelBinderProvider(options.InputFormatters, _readerFactory, _loggerFactory, options)); options.ModelBinderProviders.Add(new HeaderModelBinderProvider()); options.ModelBinderProviders.Add(new FloatingPointTypeModelBinderProvider()); + options.ModelBinderProviders.Add(new EnumTypeModelBinderProvider(options.AllowBindingUndefinedValueToEnumType)); options.ModelBinderProviders.Add(new SimpleTypeModelBinderProvider()); options.ModelBinderProviders.Add(new CancellationTokenModelBinderProvider()); options.ModelBinderProviders.Add(new ByteArrayModelBinderProvider()); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/EnumTypeModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/EnumTypeModelBinder.cs new file mode 100644 index 0000000000..4787e0528e --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/EnumTypeModelBinder.cs @@ -0,0 +1,84 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders +{ + /// + /// implementation to bind models for types deriving from . + /// + public class EnumTypeModelBinder : SimpleTypeModelBinder + { + private readonly bool _allowBindingUndefinedValueToEnumType; + + public EnumTypeModelBinder(bool allowBindingUndefinedValueToEnumType, Type modelType) + : base(modelType) + { + if (modelType == null) + { + throw new ArgumentNullException(nameof(modelType)); + } + + _allowBindingUndefinedValueToEnumType = allowBindingUndefinedValueToEnumType; + } + + protected override void CheckModel( + ModelBindingContext bindingContext, + ValueProviderResult valueProviderResult, + object model) + { + if (model == null || _allowBindingUndefinedValueToEnumType) + { + base.CheckModel(bindingContext, valueProviderResult, model); + } + else + { + if (IsDefinedInEnum(model, bindingContext)) + { + bindingContext.Result = ModelBindingResult.Success(model); + } + else + { + bindingContext.ModelState.TryAddModelError( + bindingContext.ModelName, + bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueIsInvalidAccessor( + valueProviderResult.ToString())); + } + } + } + + private bool IsDefinedInEnum(object model, ModelBindingContext bindingContext) + { + var modelType = bindingContext.ModelMetadata.UnderlyingOrModelType; + + // Check if the converted value is indeed defined on the enum as EnumTypeConverter + // converts value to the backing type (ex: integer) and does not check if the value is defined on the enum. + if (bindingContext.ModelMetadata.IsFlagsEnum) + { + // Enum.IsDefined does not work with combined flag enum values. + // From EnumDataTypeAttribute.cs in CoreFX. + // Exmaples: + // + // [Flags] + // enum FlagsEnum { Value1 = 1, Value2 = 2, Value4 = 4 } + // + // Valid Scenarios: + // 1. valueproviderresult="Value2,Value4", model=Value2 | Value4, underlying=6, converted=Value2, Value4 + // 2. valueproviderresult="2,4", model=Value2 | Value4, underlying=6, converted=Value2, Value4 + // + // Invalid Scenarios: + // 1. valueproviderresult="2,10", model=12, underlying=12, converted=12 + // + var underlying = Convert.ChangeType( + model, + Enum.GetUnderlyingType(modelType), + CultureInfo.InvariantCulture).ToString(); + var converted = model.ToString(); + return !string.Equals(underlying, converted, StringComparison.OrdinalIgnoreCase); + } + return Enum.IsDefined(modelType, model); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/EnumTypeModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/EnumTypeModelBinderProvider.cs new file mode 100644 index 0000000000..c7ef8be646 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/EnumTypeModelBinderProvider.cs @@ -0,0 +1,38 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders +{ + /// + /// A for types deriving from . + /// + public class EnumTypeModelBinderProvider : IModelBinderProvider + { + private readonly bool _allowBindingUndefinedValueToEnumType; + + public EnumTypeModelBinderProvider(bool allowBindingUndefinedValueToEnumType) + { + _allowBindingUndefinedValueToEnumType = allowBindingUndefinedValueToEnumType; + } + + /// + public IModelBinder GetBinder(ModelBinderProviderContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (context.Metadata.IsEnum) + { + return new EnumTypeModelBinder( + _allowBindingUndefinedValueToEnumType, + context.Metadata.UnderlyingOrModelType); + } + + return null; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs index cdcaaedf28..74abfada0e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs @@ -72,23 +72,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders value: value); } - // When converting newModel a null value may indicate a failed conversion for an otherwise required - // model (can't set a ValueType to null). This detects if a null model value is acceptable given the - // current bindingContext. If not, an error is logged. - if (model == null && !bindingContext.ModelMetadata.IsReferenceOrNullableType) - { - bindingContext.ModelState.TryAddModelError( - bindingContext.ModelName, - bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor( - valueProviderResult.ToString())); + CheckModel(bindingContext, valueProviderResult, model); - return Task.CompletedTask; - } - else - { - bindingContext.Result = ModelBindingResult.Success(model); - return Task.CompletedTask; - } + return Task.CompletedTask; } catch (Exception exception) { @@ -109,5 +95,26 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders return Task.CompletedTask; } } + + protected virtual void CheckModel( + ModelBindingContext bindingContext, + ValueProviderResult valueProviderResult, + object model) + { + // When converting newModel a null value may indicate a failed conversion for an otherwise required + // model (can't set a ValueType to null). This detects if a null model value is acceptable given the + // current bindingContext. If not, an error is logged. + if (model == null && !bindingContext.ModelMetadata.IsReferenceOrNullableType) + { + bindingContext.ModelState.TryAddModelError( + bindingContext.ModelName, + bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor( + valueProviderResult.ToString())); + } + else + { + bindingContext.Result = ModelBindingResult.Success(model); + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs index 2eec2a0d53..2244af1c51 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/MvcOptions.cs @@ -162,5 +162,11 @@ namespace Microsoft.AspNetCore.Mvc /// Gets or sets the default value for the Permanent property of . /// public bool RequireHttpsPermanent { get; set; } + + /// + /// Gets or sets an indication whether the model binding system will bind undefined values to enumeration types. + /// by default. + /// + public bool AllowBindingUndefinedValueToEnumType { get; set; } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/EnumTypeModelBinderProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/EnumTypeModelBinderProviderTest.cs new file mode 100644 index 0000000000..6773822709 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/EnumTypeModelBinderProviderTest.cs @@ -0,0 +1,76 @@ +// 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 Microsoft.AspNetCore.Mvc.ModelBinding.Binders; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding +{ + public class EnumTypeModelBinderProviderTest + { + [Theory] + [InlineData(typeof(CarType))] + [InlineData(typeof(CarType?))] + public void ReturnsBinder_ForEnumType(Type modelType) + { + // Arrange + var provider = new EnumTypeModelBinderProvider(allowBindingUndefinedValueToEnumType: true); + var context = new TestModelBinderProviderContext(modelType); + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.IsType(result); + } + + [Theory] + [InlineData(typeof(CarOptions))] + [InlineData(typeof(CarOptions?))] + public void ReturnsBinder_ForFlagsEnumType(Type modelType) + { + // Arrange + var provider = new EnumTypeModelBinderProvider(allowBindingUndefinedValueToEnumType: true); + var context = new TestModelBinderProviderContext(modelType); + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.IsType(result); + } + + [Theory] + [InlineData(typeof(string))] + [InlineData(typeof(int))] + [InlineData(typeof(int?))] + public void DoesNotReturnBinder_ForNonEnumTypes(Type modelType) + { + // Arrange + var provider = new EnumTypeModelBinderProvider(allowBindingUndefinedValueToEnumType: false); + var context = new TestModelBinderProviderContext(modelType); + + // Act + var result = provider.GetBinder(context); + + // Assert + Assert.Null(result); + } + + enum CarType + { + Sedan, + Coupe + } + + [Flags] + public enum CarOptions + { + SunRoof = 0x01, + Spoiler = 0x02, + FogLights = 0x04, + TintedWindows = 0x08, + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/EnumTypeModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/EnumTypeModelBinderTest.cs new file mode 100644 index 0000000000..88d461d78b --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/EnumTypeModelBinderTest.cs @@ -0,0 +1,316 @@ +// 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.ComponentModel; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding +{ + public class EnumTypeModelBinderTest + { + [Theory] + [InlineData(true, typeof(IntEnum?))] + [InlineData(true, typeof(FlagsEnum?))] + [InlineData(false, typeof(IntEnum?))] + [InlineData(false, typeof(FlagsEnum?))] + public async Task BindModel_SetsModel_ForEmptyValue_AndNullableEnumTypes( + bool allowBindingUndefinedValueToEnumType, + Type modelType) + { + // Arrange + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType, + valueProviderValue: ""); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + } + + [Theory] + [InlineData(true, typeof(IntEnum))] + [InlineData(true, typeof(FlagsEnum))] + [InlineData(false, typeof(IntEnum))] + [InlineData(false, typeof(FlagsEnum))] + public async Task BindModel_AddsErrorToModelState_ForEmptyValue_AndNonNullableEnumTypes( + bool allowBindingUndefinedValueToEnumType, + Type modelType) + { + // Arrange + var message = "The value '' is invalid."; + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType, + valueProviderValue: ""); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + Assert.False(bindingContext.ModelState.IsValid); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal(message, error.ErrorMessage); + } + + [Theory] + [InlineData(true, "Value1")] + [InlineData(true, "1")] + [InlineData(false, "Value1")] + [InlineData(false, "1")] + public async Task BindModel_BindsEnumModels_ForValuesInArray( + bool allowBindingUndefinedValueToEnumType, + string enumValue) + { + // Arrange + var modelType = typeof(IntEnum); + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType, + valueProviderValue: new object[] { enumValue }); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(IntEnum.Value1, boundModel); + } + + [Theory] + [InlineData("1", true)] + [InlineData("8, 1", true)] + [InlineData("Value2, Value8", true)] + [InlineData("value8,value4,value2,value1", true)] + [InlineData("1", false)] + [InlineData("8, 1", false)] + [InlineData("Value2, Value8", false)] + [InlineData("value8,value4,value2,value1", false)] + public async Task BindModel_BindsTo_NonNullableFlagsEnumType(string flagsEnumValue, bool allowBindingUndefinedValueToEnumType) + { + // Arrange + var modelType = typeof(FlagsEnum); + var enumConverter = TypeDescriptor.GetConverter(modelType); + var expected = enumConverter.ConvertFrom(flagsEnumValue).ToString(); + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType, + valueProviderValue: new object[] { flagsEnumValue }); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(expected, boundModel.ToString()); + } + + [Theory] + [InlineData("1", true)] + [InlineData("8, 1", true)] + [InlineData("Value2, Value8", true)] + [InlineData("value8,value4,value2,value1", true)] + [InlineData("1", false)] + [InlineData("8, 1", false)] + [InlineData("Value2, Value8", false)] + [InlineData("value8,value4,value2,value1", false)] + public async Task BindModel_BindsTo_NullableFlagsEnumType(string flagsEnumValue, bool allowBindingUndefinedValueToEnumType) + { + // Arrange + var modelType = typeof(FlagsEnum?); + var enumConverter = TypeDescriptor.GetConverter(GetUnderlyingType(modelType)); + var expected = enumConverter.ConvertFrom(flagsEnumValue).ToString(); + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType, + valueProviderValue: new object[] { flagsEnumValue }); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + var boundModel = Assert.IsType(bindingContext.Result.Model); + Assert.Equal(expected, boundModel.ToString()); + } + + [Theory] + [InlineData(typeof(FlagsEnum), "Value10")] + [InlineData(typeof(FlagsEnum), "Value1, Value10")] + [InlineData(typeof(FlagsEnum), "value10, value1")] + [InlineData(typeof(FlagsEnum?), "Value10")] + [InlineData(typeof(FlagsEnum?), "Value1, Value10")] + [InlineData(typeof(FlagsEnum?), "value10, value1")] + public async Task BindModel_AddsErrorToModelState_ForInvalidEnumValues_IsNotValidMessage(Type modelType, string suppliedValue) + { + // Arrange + var message = $"The value '{suppliedValue}' is not valid."; + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType: true, + valueProviderValue: new object[] { suppliedValue }); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + Assert.False(bindingContext.ModelState.IsValid); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal(message, error.ErrorMessage); + } + + [Theory] + [InlineData(typeof(IntEnum), "")] + [InlineData(typeof(IntEnum), "3")] + [InlineData(typeof(FlagsEnum), "19")] + [InlineData(typeof(FlagsEnum), "0")] + [InlineData(typeof(FlagsEnum), "1, 16")] + // These two values look like big integers but are treated as two separate enum values that are + // or'd together. + [InlineData(typeof(FlagsEnum), "32,015")] + [InlineData(typeof(FlagsEnum), "32,128")] + [InlineData(typeof(IntEnum?), "3")] + [InlineData(typeof(FlagsEnum?), "19")] + [InlineData(typeof(FlagsEnum?), "0")] + [InlineData(typeof(FlagsEnum?), "1, 16")] + // These two values look like big integers but are treated as two separate enum values that are + // or'd together. + [InlineData(typeof(FlagsEnum?), "32,015")] + [InlineData(typeof(FlagsEnum?), "32,128")] + public async Task BindModel_AddsErrorToModelState_ForInvalidEnumValues(Type modelType, string suppliedValue) + { + // Arrange + var message = $"The value '{suppliedValue}' is invalid."; + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType: false, + valueProviderValue: new object[] { suppliedValue }); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.False(bindingContext.Result.IsModelSet); + Assert.Null(bindingContext.Result.Model); + Assert.False(bindingContext.ModelState.IsValid); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); + Assert.Equal(message, error.ErrorMessage); + } + + [Theory] + [InlineData(typeof(IntEnum), "3", 3)] + [InlineData(typeof(FlagsEnum), "19", 19)] + [InlineData(typeof(FlagsEnum), "0", 0)] + [InlineData(typeof(FlagsEnum), "1, 16", 17)] + // These two values look like big integers but are treated as two separate enum values that are + // or'd together. + [InlineData(typeof(FlagsEnum), "32,015", 47)] + [InlineData(typeof(FlagsEnum), "32,128", 160)] + [InlineData(typeof(IntEnum?), "3", 3)] + [InlineData(typeof(FlagsEnum?), "19", 19)] + [InlineData(typeof(FlagsEnum?), "0", 0)] + [InlineData(typeof(FlagsEnum?), "1, 16", 17)] + // These two values look like big integers but are treated as two separate enum values that are + // or'd together. + [InlineData(typeof(FlagsEnum?), "32,015", 47)] + [InlineData(typeof(FlagsEnum?), "32,128", 160)] + public async Task BindModel_AllowsBindingUndefinedValues_ToEnumTypes( + Type modelType, + string suppliedValue, + long expected) + { + // Arrange + var binderProviderContext = new TestModelBinderProviderContext(modelType); + var binderInfo = GetBinderAndContext( + modelType, + allowBindingUndefinedValueToEnumType: true, + valueProviderValue: new object[] { suppliedValue }); + var bindingContext = binderInfo.Item1; + var binder = binderInfo.Item2; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + Assert.IsType(GetUnderlyingType(modelType), bindingContext.Result.Model); + Assert.Equal(expected, Convert.ToInt64(bindingContext.Result.Model)); + } + + private static (DefaultModelBindingContext, IModelBinder) GetBinderAndContext( + Type modelType, + bool allowBindingUndefinedValueToEnumType, + object valueProviderValue) + { + var binderProviderContext = new TestModelBinderProviderContext(modelType); + var modelName = "theModelName"; + var bindingContext = new DefaultModelBindingContext + { + ModelMetadata = binderProviderContext.Metadata, + ModelName = modelName, + ModelState = new ModelStateDictionary(), + ValueProvider = new SimpleValueProvider() + { + { modelName, valueProviderValue } + } + }; + var binderProvider = new EnumTypeModelBinderProvider(allowBindingUndefinedValueToEnumType); + var binder = binderProvider.GetBinder(binderProviderContext); + return (bindingContext, binder); + } + + private static Type GetUnderlyingType(Type modelType) + { + var underlyingType = Nullable.GetUnderlyingType(modelType); + if (underlyingType != null) + { + return underlyingType; + } + return modelType; + } + + [Flags] + private enum FlagsEnum + { + Value1 = 1, + Value2 = 2, + Value4 = 4, + Value8 = 8, + } + + private enum IntEnum + { + Value0 = 0, + Value1 = 1, + Value2 = 2, + MaxValue = int.MaxValue + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs index 5cd8b88d0b..df3b617c95 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs @@ -59,6 +59,7 @@ namespace Microsoft.AspNetCore.Mvc binder => Assert.IsType(binder), binder => Assert.IsType(binder), binder => Assert.IsType(binder), + binder => Assert.IsType(binder), binder => Assert.IsType(binder), binder => Assert.IsType(binder), binder => Assert.IsType(binder),