diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs index be2d6cebc8..7de72be66b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs @@ -92,8 +92,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public abstract BindingSource BindingSource { get; } /// - /// Gets a value indicating whether or not to convert an empty string value to null when - /// representing a model as text. + /// Gets a value indicating whether or not to convert an empty string value or one containing only whitespace + /// characters to null when representing a model as text. /// public abstract bool ConvertEmptyStringToNull { get; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs index ad70420966..92ab83ad2f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/SimpleTypeModelBinder.cs @@ -47,8 +47,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders { var value = valueProviderResult.FirstValue; - object model = null; - if (!string.IsNullOrWhiteSpace(value)) + object model; + if (bindingContext.ModelType == typeof(string)) + { + // Already have a string. No further conversion required but handle ConvertEmptyStringToNull. + if (bindingContext.ModelMetadata.ConvertEmptyStringToNull && string.IsNullOrWhiteSpace(value)) + { + model = null; + } + else + { + model = value; + } + } + else if (string.IsNullOrWhiteSpace(value)) + { + // Other than the StringConverter, converters Trim() the value then throw if the result is empty. + model = null; + } + else { model = _typeConverter.ConvertFrom( context: null, @@ -56,16 +73,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders value: value); } - if (bindingContext.ModelType == typeof(string)) - { - var modelAsString = model as string; - if (bindingContext.ModelMetadata.ConvertEmptyStringToNull && - string.IsNullOrEmpty(modelAsString)) - { - model = null; - } - } - // 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. @@ -75,7 +82,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders bindingContext.ModelName, bindingContext.ModelMetadata.ModelBindingMessageProvider.ValueMustNotBeNullAccessor( valueProviderResult.ToString())); - + return TaskCache.CompletedTask; } else diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs index 4d02a152e8..f1fd3fef08 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DisplayMetadata.cs @@ -17,8 +17,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata public IDictionary AdditionalValues { get; } = new Dictionary(); /// - /// Gets or sets a value indicating whether or not empty strings should be treated as null. - /// See + /// Gets or sets a value indicating whether or not to convert an empty string value or one containing only + /// whitespace characters to null when representing a model as text. See + /// /// public bool ConvertEmptyStringToNull { get; set; } = true; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs index 9941b572ec..7ab5c007ef 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/SimpleTypeModelBinderTest.cs @@ -11,6 +11,58 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders { public class SimpleTypeModelBinderTest { + [Theory] + [InlineData(null)] + [InlineData("value")] + [InlineData("intermediate whitespace")] + [InlineData("\t untrimmed whitespace \t\r\n")] + public async Task BindModelAsync_ReturnsProvidedString(string value) + { + // Arrange + var bindingContext = GetBindingContext(typeof(string)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", value } + }; + + var binder = new SimpleTypeModelBinder(typeof(string)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Same(value, bindingContext.Result.Model); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + + [Theory] + [InlineData("")] + [InlineData(" \t \r\n ")] + public async Task BindModel_ReturnsProvidedWhitespaceString_WhenNotConvertEmptyStringToNull(string value) + { + // Arrange + var bindingContext = GetBindingContext(typeof(string)); + bindingContext.ValueProvider = new SimpleValueProvider + { + { "theModelName", value } + }; + + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForType(typeof(string)) + .DisplayDetails(d => d.ConvertEmptyStringToNull = false); + bindingContext.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string)); + + var binder = new SimpleTypeModelBinder(typeof(string)); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Same(value, bindingContext.Result.Model); + Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); + } + public static TheoryData ConvertableTypeData { get @@ -125,26 +177,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders Assert.Empty(bindingContext.ModelState); } - [Fact] - public async Task BindModel_ValidValueProviderResult_ConvertEmptyStringsToNull() - { - // Arrange - var bindingContext = GetBindingContext(typeof(string)); - bindingContext.ValueProvider = new SimpleValueProvider - { - { "theModelName", string.Empty } - }; - - var binder = new SimpleTypeModelBinder(typeof(string)); - - // Act - await binder.BindModelAsync(bindingContext); - - // Assert - Assert.Null(bindingContext.Result.Model); - Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); - } - [Theory] [InlineData("")] [InlineData(" \t \r\n ")] @@ -215,7 +247,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders // Arrange var bindingContext = GetBindingContext(typeof(int)); bindingContext.ValueProvider = new SimpleValueProvider - { + { { "theModelName", "42" } };