diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterContext.cs index 4c40b4eb77..e2ddf78a30 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterContext.cs @@ -36,6 +36,36 @@ public InputFormatterContext( ModelStateDictionary modelState, ModelMetadata metadata, Func readerFactory) + : this(httpContext, modelName, modelState, metadata, readerFactory, allowEmptyInput: false) + { + } + + /// + /// Creates a new instance of . + /// + /// + /// The for the current operation. + /// + /// The name of the model. + /// + /// The for recording errors. + /// + /// + /// The of the model to deserialize. + /// + /// + /// A delegate which can create a for the request body. + /// + /// + /// A value for . + /// + public InputFormatterContext( + HttpContext httpContext, + string modelName, + ModelStateDictionary modelState, + ModelMetadata metadata, + Func readerFactory, + bool allowEmptyInput) { if (httpContext == null) { @@ -67,9 +97,19 @@ public InputFormatterContext( ModelState = modelState; Metadata = metadata; ReaderFactory = readerFactory; + AllowEmptyInput = allowEmptyInput; ModelType = metadata.ModelType; } + /// + /// Gets a flag to indicate whether the input formatter should allow no value to be provided. + /// If , the input formatter should handle empty input by returning + /// . If , the input + /// formatter should handle empty input by returning the default value for the type + /// . + /// + public bool AllowEmptyInput { get; } + /// /// Gets the associated with the current operation. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterResult.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterResult.cs index bcbd77e62c..2304cd5671 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Formatters/InputFormatterResult.cs @@ -10,17 +10,20 @@ namespace Microsoft.AspNetCore.Mvc.Formatters /// public class InputFormatterResult { - private static readonly InputFormatterResult _failure = new InputFormatterResult(); + private static readonly InputFormatterResult _failure = new InputFormatterResult(hasError: true); + private static readonly InputFormatterResult _noValue = new InputFormatterResult(hasError: false); private static readonly Task _failureAsync = Task.FromResult(_failure); + private static readonly Task _noValueAsync = Task.FromResult(_noValue); - private InputFormatterResult() + private InputFormatterResult(bool hasError) { - HasError = true; + HasError = hasError; } private InputFormatterResult(object model) { Model = model; + IsModelSet = true; } /// @@ -28,6 +31,11 @@ private InputFormatterResult(object model) /// public bool HasError { get; } + /// + /// Gets an indication whether a value for the property was supplied. + /// + public bool IsModelSet { get; } + /// /// Gets the deserialized . /// @@ -89,5 +97,31 @@ public static Task SuccessAsync(object model) { return Task.FromResult(Success(model)); } + + /// + /// Returns an indicating the + /// operation produced no value. + /// + /// + /// An indicating the + /// operation produced no value. + /// + public static InputFormatterResult NoValue() + { + return _noValue; + } + + /// + /// Returns a that on completion provides an indicating + /// the operation produced no value. + /// + /// + /// A that on completion provides an indicating the + /// operation produced no value. + /// + public static Task NoValueAsync() + { + return _noValueAsync; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/InputFormatter.cs b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/InputFormatter.cs index c6fe0e94a1..359615191b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Formatters/InputFormatter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Formatters/InputFormatter.cs @@ -105,7 +105,14 @@ public virtual Task ReadAsync(InputFormatterContext contex var request = context.HttpContext.Request; if (request.ContentLength == 0) { - return InputFormatterResult.SuccessAsync(GetDefaultValueForType(context.ModelType)); + if (context.AllowEmptyInput) + { + return InputFormatterResult.SuccessAsync(GetDefaultValueForType(context.ModelType)); + } + else + { + return InputFormatterResult.NoValueAsync(); + } } return ReadRequestBodyAsync(context); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs index 2c87426fc8..3ad585126c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs @@ -20,9 +20,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders public class BodyModelBinder : IModelBinder { private readonly IList _formatters; - private readonly MvcOptions _options; private readonly Func _readerFactory; private readonly ILogger _logger; + private readonly MvcOptions _options; /// /// Creates a new . @@ -114,12 +114,15 @@ public async Task BindModelAsync(ModelBindingContext bindingContext) var httpContext = bindingContext.HttpContext; + var allowEmptyInput = _options?.AllowEmptyInputInInputFormatter == true; + var formatterContext = new InputFormatterContext( httpContext, modelBindingKey, bindingContext.ModelState, bindingContext.ModelMetadata, - _readerFactory); + _readerFactory, + allowEmptyInput); var formatter = (IInputFormatter)null; for (var i = 0; i < _formatters.Count; i++) @@ -156,10 +159,17 @@ public async Task BindModelAsync(ModelBindingContext bindingContext) // Formatter encountered an error. Do not use the model it returned. return; } - - if (model == null && _options?.AllowEmptyInputInInputFormatter != true) + + if (result.IsModelSet) + { + bindingContext.Result = ModelBindingResult.Success(model); + } + else { - // Body model binding is required, but no value was supplied. + // If the input formatter gives a "no value" result, that's always a validation error. + // If instead the input formatter wants to treat the input as optional, it must do so by + // returning InputFormatterResult.Success(defaultForModelType), because input formatters + // are responsible for choosing a default value for the model type. bindingContext.Result = ModelBindingResult.Failed(); var message = bindingContext @@ -168,10 +178,6 @@ public async Task BindModelAsync(ModelBindingContext bindingContext) .MissingRequestBodyRequiredValueAccessor(); bindingContext.ModelState.AddModelError(modelBindingKey, message); } - else - { - bindingContext.Result = ModelBindingResult.Success(model); - } return; } diff --git a/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs b/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs index e146e7cce1..dc25ea5711 100644 --- a/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs +++ b/src/Microsoft.AspNetCore.Mvc.Formatters.Json/JsonInputFormatter.cs @@ -158,7 +158,18 @@ public override Task ReadRequestBodyAsync( if (successful) { - return InputFormatterResult.SuccessAsync(model); + if (model == null && !context.AllowEmptyInput) + { + // Some nonempty inputs might deserialize as null, for example whitespace, + // or the JSON-encoded value "null". The upstream BodyModelBinder needs to + // be notified that we don't regard this as a real input so it can register + // a validation error. + return InputFormatterResult.NoValueAsync(); + } + else + { + return InputFormatterResult.SuccessAsync(model); + } } return InputFormatterResult.FailureAsync(); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/InputFormatterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/InputFormatterTest.cs index d235bc9ea4..bcc3f76f03 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/InputFormatterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Formatters/InputFormatterTest.cs @@ -410,6 +410,31 @@ public void GetSupportedContentTypes_ThrowsInvalidOperationException_IfMediaType () => formatter.GetSupportedContentTypes("application/json", typeof(object))); } + [Theory] + [InlineData(true, true)] + [InlineData(false, false)] + public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bool allowEmptyInputValue, bool expectedIsModelSet) + { + // Arrange + var formatter = new TestFormatter(); + var context = new InputFormatterContext( + new DefaultHttpContext(), + "", + new ModelStateDictionary(), + new EmptyModelMetadataProvider().GetMetadataForType(typeof(object)), + (s, e) => new StreamReader(s, e), + allowEmptyInputValue); + context.HttpContext.Request.ContentLength = 0; + + // Act + var result = await formatter.ReadAsync(context); + + // Assert + Assert.False(result.HasError); + Assert.Null(result.Model); + Assert.Equal(expectedIsModelSet, result.IsModelSet); + } + private class BadConfigurationFormatter : InputFormatter { public override Task ReadRequestBodyAsync(InputFormatterContext context) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/BodyModelBinderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/BodyModelBinderTests.cs index c0e5aa6d16..4517febae7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/BodyModelBinderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/BodyModelBinderTests.cs @@ -115,17 +115,15 @@ public async Task BindModel_IsGreedy() Assert.False(bindingContext.Result.IsModelSet); } - [Theory] - [InlineData(false, true)] - [InlineData(true, false)] - public async Task BindModel_NoInput_SetsModelStateErrorWhenExpected(bool allowEmptyInputSetting, bool expectValidationError) + [Fact] + public async Task BindModel_NoValueResult_SetsModelStateError() { // Arrange var mockInputFormatter = new Mock(); mockInputFormatter.Setup(f => f.CanRead(It.IsAny())) .Returns(true); mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny())) - .Returns(InputFormatterResult.SuccessAsync(null)); + .Returns(InputFormatterResult.NoValueAsync()); var inputFormatter = mockInputFormatter.Object; var provider = new TestModelMetadataProvider(); @@ -136,29 +134,53 @@ public async Task BindModel_NoInput_SetsModelStateErrorWhenExpected(bool allowEm metadataProvider: provider); bindingContext.BinderModelName = "custom"; - var binder = CreateBinder(new[] { inputFormatter }, allowEmptyInputSetting); + var binder = CreateBinder(new[] { inputFormatter }); // Act await binder.BindModelAsync(bindingContext); // Assert - var expectIsModelSet = !expectValidationError; // If we allow null as a valid input, then IsModelSet==true in that case - Assert.Equal(expectIsModelSet, bindingContext.Result.IsModelSet); Assert.Null(bindingContext.Result.Model); + Assert.False(bindingContext.Result.IsModelSet); + Assert.False(bindingContext.ModelState.IsValid); - if (expectValidationError) - { - // Key is the bindermodelname because this was a top-level binding. - Assert.False(bindingContext.ModelState.IsValid); - var entry = Assert.Single(bindingContext.ModelState); - Assert.Equal("custom", entry.Key); - Assert.Single(entry.Value.Errors); - } - else - { - Assert.True(bindingContext.ModelState.IsValid); - Assert.Empty(bindingContext.ModelState); - } + // Key is the bindermodelname because this was a top-level binding. + var entry = Assert.Single(bindingContext.ModelState); + Assert.Equal("custom", entry.Key); + Assert.Single(entry.Value.Errors); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task BindModel_PassesAllowEmptyInputOptionViaContext(bool allowEmptyInputValue) + { + // Arrange + var mockInputFormatter = new Mock(); + mockInputFormatter.Setup(f => f.CanRead(It.IsAny())) + .Returns(true); + mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny())) + .Returns(InputFormatterResult.NoValueAsync()) + .Verifiable(); + var inputFormatter = mockInputFormatter.Object; + + var provider = new TestModelMetadataProvider(); + provider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); + + var bindingContext = GetBindingContext( + typeof(Person), + metadataProvider: provider); + bindingContext.BinderModelName = "custom"; + + var binder = CreateBinder(new[] { inputFormatter }, allowEmptyInputValue); + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + mockInputFormatter.Verify(formatter => formatter.ReadAsync( + It.Is(ctx => ctx.AllowEmptyInput == allowEmptyInputValue)), + Times.Once); } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs b/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs index 75dd819910..448c0f1025 100644 --- a/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Formatters.Json.Test/JsonInputFormatterTest.cs @@ -342,6 +342,40 @@ public async Task ReadAsync_UsesTryAddModelValidationErrorsToModelState() Assert.IsType(error.Exception); } + [Theory] + [InlineData("null", true, true)] + [InlineData("null", false, false)] + [InlineData(" ", true, true)] + [InlineData(" ", false, false)] + public async Task ReadAsync_WithInputThatDeserializesToNull_SetsModelOnlyIfAllowingEmptyInput(string content, bool allowEmptyInput, bool expectedIsModelSet) + { + // Arrange + var logger = GetLogger(); + var formatter = + new JsonInputFormatter(logger, _serializerSettings, ArrayPool.Shared, _objectPoolProvider); + var contentBytes = Encoding.UTF8.GetBytes(content); + + var modelState = new ModelStateDictionary(); + var httpContext = GetHttpContext(contentBytes); + var provider = new EmptyModelMetadataProvider(); + var metadata = provider.GetMetadataForType(typeof(object)); + var context = new InputFormatterContext( + httpContext, + modelName: string.Empty, + modelState: modelState, + metadata: metadata, + readerFactory: new TestHttpRequestStreamReaderFactory().CreateReader, + allowEmptyInput: allowEmptyInput); + + // Act + var result = await formatter.ReadAsync(context); + + // Assert + Assert.False(result.HasError); + Assert.Equal(expectedIsModelSet, result.IsModelSet); + Assert.Null(result.Model); + } + [Fact] public void Constructor_UsesSerializerSettings() {