Make [FromBody] treat empty request bodies as invalid (#4750)

src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/IModelBindingMessageProvider.cs

@@ -24,6 +24,13 @@ public interface IModelBindingMessageProvider
         /// <value>Default <see cref="string"/> is "A value is required.".</value>
         Func<string> MissingKeyOrValueAccessor { get; }
 
+        /// <summary>
+        /// Error message the model binding system adds when no value is provided for the request body,
+        /// but a value is required.
+        /// </summary>
+        /// <value>Default <see cref="string"/> is "A value for the request body was not provided.".</value>
+        Func<string> MissingRequestBodyRequiredValueAccessor { get; }
+
         /// <summary>
         /// Error message the model binding system adds when a <c>null</c> value is bound to a
         /// non-<see cref="Nullable"/> property.

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs

@@ -20,6 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
     public class BodyModelBinder : IModelBinder
     {
         private readonly IList<IInputFormatter> _formatters;
+        private readonly bool _isBindingRequired;
         private readonly Func<Stream, Encoding, TextReader> _readerFactory;
         private readonly ILogger _logger;
 
@@ -31,8 +32,12 @@ public class BodyModelBinder : IModelBinder
         /// The <see cref="IHttpRequestStreamReaderFactory"/>, used to create <see cref="System.IO.TextReader"/>
         /// instances for reading the request body.
         /// </param>
-        public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamReaderFactory readerFactory)
-            : this(formatters, readerFactory, loggerFactory: null)
+        /// <param name="isBindingRequired">Specifies whether a null input (e.g., due to an empty request body) should be treated as a validation error.</param>
+        public BodyModelBinder(
+            IList<IInputFormatter> formatters,
+            IHttpRequestStreamReaderFactory readerFactory,
+            bool isBindingRequired
+            ) : this(formatters, readerFactory, loggerFactory: null, isBindingRequired: isBindingRequired)
         {
         }
 
@@ -45,7 +50,12 @@ public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamRead
         /// instances for reading the request body.
         /// </param>
         /// <param name="loggerFactory">The <see cref="ILoggerFactory"/>.</param>
-        public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamReaderFactory readerFactory, ILoggerFactory loggerFactory)
+        /// <param name="isBindingRequired">Specifies whether a null input (e.g., due to an empty request body) should be treated as a validation error.</param>
+        public BodyModelBinder(
+            IList<IInputFormatter> formatters,
+            IHttpRequestStreamReaderFactory readerFactory,
+            ILoggerFactory loggerFactory,
+            bool isBindingRequired)
         {
             if (formatters == null)
             {
@@ -58,6 +68,7 @@ public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamRead
             }
 
             _formatters = formatters;
+            _isBindingRequired = isBindingRequired;
             _readerFactory = readerFactory.CreateReader;
 
             if (loggerFactory != null)
@@ -66,6 +77,11 @@ public BodyModelBinder(IList<IInputFormatter> formatters, IHttpRequestStreamRead
             }
         }
 
+        /// <summary>
+        /// Gets a flag indicating whether the binder will require non-null input values to be supplied.
+        /// </summary>
+        public bool IsBindingRequired => _isBindingRequired;
+
         /// <inheritdoc />
         public async Task BindModelAsync(ModelBindingContext bindingContext)
         {
@@ -132,7 +148,21 @@ public async Task BindModelAsync(ModelBindingContext bindingContext)
                     return;
                 }
 
-                bindingContext.Result = ModelBindingResult.Success(model);
+                if (_isBindingRequired && model == null)
+                {
+                    bindingContext.Result = ModelBindingResult.Failed();
+
+                    var message = bindingContext
+                        .ModelMetadata
+                        .ModelBindingMessageProvider
+                        .MissingRequestBodyRequiredValueAccessor();
+                    bindingContext.ModelState.AddModelError(modelBindingKey, message);
+                }
+                else
+                {
+                    bindingContext.Result = ModelBindingResult.Success(model);
+                }
+
                 return;
             }
             catch (Exception ex)

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinderProvider.cs

@@ -52,6 +52,17 @@ public BodyModelBinderProvider(IList<IInputFormatter> formatters, IHttpRequestSt
             _loggerFactory = loggerFactory;
         }
 
+        /// <summary>
+        /// Gets or sets a flag that controls whether the binder should require non-null
+        /// input values to be supplied. Defaults to <c>true</c>.
+        /// </summary>
+        /// <example>
+        /// If <c>true</c>, and an associated <see cref="IInputFormatter"/> supplies <c>null</c>
+        /// as the value to bind (for example, if an incoming POST or PUT request supplies a
+        /// zero-length body), the binder will register a validation error.
+        /// </example>
+        public bool IsBindingRequired { get; set; } = true;
+
         /// <inheritdoc />
         public IModelBinder GetBinder(ModelBinderProviderContext context)
         {
@@ -71,7 +82,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
                         typeof(IInputFormatter).FullName));
                 }
 
-                return new BodyModelBinder(_formatters, _readerFactory, _loggerFactory);
+                return new BodyModelBinder(_formatters, _readerFactory, _loggerFactory, IsBindingRequired);
             }
 
             return null;

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ModelBindingMessageProvider.cs

@@ -13,6 +13,7 @@ public class ModelBindingMessageProvider : IModelBindingMessageProvider
     {
         private Func<string, string> _missingBindRequiredValueAccessor;
         private Func<string> _missingKeyOrValueAccessor;
+        private Func<string> _missingRequestBodyRequiredValueAccessor;
         private Func<string, string> _valueMustNotBeNullAccessor;
         private Func<string, string, string> _attemptedValueIsInvalidAccessor;
         private Func<string, string> _unknownValueIsInvalidAccessor;
@@ -26,6 +27,7 @@ public ModelBindingMessageProvider()
         {
             MissingBindRequiredValueAccessor = Resources.FormatModelBinding_MissingBindRequiredMember;
             MissingKeyOrValueAccessor = Resources.FormatKeyValuePair_BothKeyAndValueMustBePresent;
+            MissingRequestBodyRequiredValueAccessor = Resources.FormatModelBinding_MissingRequestBodyRequiredMember;
             ValueMustNotBeNullAccessor = Resources.FormatModelBinding_NullValueNotValid;
             AttemptedValueIsInvalidAccessor = Resources.FormatModelState_AttemptedValueIsInvalid;
             UnknownValueIsInvalidAccessor = Resources.FormatModelState_UnknownValueIsInvalid;
@@ -47,6 +49,7 @@ public ModelBindingMessageProvider(ModelBindingMessageProvider originalProvider)
 
             MissingBindRequiredValueAccessor = originalProvider.MissingBindRequiredValueAccessor;
             MissingKeyOrValueAccessor = originalProvider.MissingKeyOrValueAccessor;
+            MissingRequestBodyRequiredValueAccessor = originalProvider.MissingRequestBodyRequiredValueAccessor;
             ValueMustNotBeNullAccessor = originalProvider.ValueMustNotBeNullAccessor;
             AttemptedValueIsInvalidAccessor = originalProvider.AttemptedValueIsInvalidAccessor;
             UnknownValueIsInvalidAccessor = originalProvider.UnknownValueIsInvalidAccessor;
@@ -90,6 +93,24 @@ public Func<string> MissingKeyOrValueAccessor
             }
         }
 
+        /// <inheritdoc/>
+        public Func<string> MissingRequestBodyRequiredValueAccessor
+        {
+            get
+            {
+                return _missingRequestBodyRequiredValueAccessor;
+            }
+            set
+            {
+                if (value == null)
+                {
+                    throw new ArgumentNullException(nameof(value));
+                }
+
+                _missingRequestBodyRequiredValueAccessor = value;
+            }
+        }
+
         /// <inheritdoc/>
         public Func<string, string> ValueMustNotBeNullAccessor
         {

src/Microsoft.AspNetCore.Mvc.Core/Resources.resx

@@ -296,6 +296,9 @@
   </data>
   <data name="ModelBinding_MissingBindRequiredMember" xml:space="preserve">
     <value>A value for the '{0}' property was not provided.</value>
+  </data>
+    <data name="ModelBinding_MissingRequestBodyRequiredMember" xml:space="preserve">
+    <value>A value for the request body was not provided.</value>
   </data>
   <data name="ValueProviderResult_NoConverterExists" xml:space="preserve">
     <value>The parameter conversion from type '{0}' to type '{1}' failed because no type converter can convert between these types.</value>

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/BodyModelBinderProviderTest.cs

@@ -71,6 +71,35 @@ public void GetBinder_WhenBindingSourceIsFromBody_ReturnsBinder()
             Assert.IsType<BodyModelBinder>(result);
         }
 
+        public void GetBinder_ProducesBinderWithIsBindingRequiredTrueByDefault()
+        {
+            // Arrange
+            var provider = CreateProvider(new TestInputFormatter());
+            var context = new TestModelBinderProviderContext(typeof(Person));
+            context.BindingInfo.BindingSource = BindingSource.Body;
+
+            // Act
+            var result = provider.GetBinder(context) as BodyModelBinder;
+
+            // Assert
+            Assert.True(result.IsBindingRequired);
+        }
+
+        public void GetBinder_CanBeConfiguredToProduceBinderWithIsBindingRequiredFalse()
+        {
+            // Arrange
+            var provider = CreateProvider(new TestInputFormatter());
+            var context = new TestModelBinderProviderContext(typeof(Person));
+            context.BindingInfo.BindingSource = BindingSource.Body;
+            provider.IsBindingRequired = false;
+
+            // Act
+            var result = provider.GetBinder(context) as BodyModelBinder;
+
+            // Assert
+            Assert.False(result.IsBindingRequired);
+        }
+
         [Fact]
         public void GetBinder_DoesNotThrowNullReferenceException()
         {

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/BodyModelBinderTests.cs

@@ -115,6 +115,52 @@ public async Task BindModel_IsGreedy()
             Assert.False(bindingContext.Result.IsModelSet);
         }
 
+        [Theory]
+        [InlineData(true, true)]
+        [InlineData(false, false)]
+        public async Task BindModel_NoInput_SetsModelStateErrorWhenExpected(bool isBindingRequired, bool expectValidationError)
+        {
+            // Arrange
+            var mockInputFormatter = new Mock<IInputFormatter>();
+            mockInputFormatter.Setup(f => f.CanRead(It.IsAny<InputFormatterContext>()))
+                .Returns(true);
+            mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny<InputFormatterContext>()))
+                              .Returns(InputFormatterResult.SuccessAsync(null));
+            var inputFormatter = mockInputFormatter.Object;
+
+            var provider = new TestModelMetadataProvider();
+            provider.ForType<Person>().BindingDetails(d => d.BindingSource = BindingSource.Body);
+
+            var bindingContext = GetBindingContext(
+                typeof(Person),
+                metadataProvider: provider);
+            bindingContext.BinderModelName = "custom";
+
+            var binder = CreateBinder(new[] { inputFormatter }, isBindingRequired);
+
+            // 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);
+
+            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);
+            }
+        }
+
         [Fact]
         public async Task CustomFormatterDeserializationException_AddedToModelState()
         {
@@ -221,7 +267,7 @@ public async Task BindModelAsync_LogsFormatterRejectionAndSelection()
             provider.ForType<Person>().BindingDetails(d => d.BindingSource = BindingSource.Body);
             var bindingContext = GetBindingContext(typeof(Person), metadataProvider: provider);
             bindingContext.HttpContext.Request.ContentType = "application/json";
-            var binder = new BodyModelBinder(inputFormatters, new TestHttpRequestStreamReaderFactory(), loggerFactory);
+            var binder = new BodyModelBinder(inputFormatters, new TestHttpRequestStreamReaderFactory(), loggerFactory, isBindingRequired: true);
 
             // Act
             await binder.BindModelAsync(bindingContext);
@@ -248,7 +294,7 @@ public async Task BindModelAsync_LogsNoFormatterSelectedAndRemoveFromBodyAttribu
             var bindingContext = GetBindingContext(typeof(Person), metadataProvider: provider);
             bindingContext.HttpContext.Request.ContentType = "multipart/form-data";
             bindingContext.BinderModelName = bindingContext.ModelName;
-            var binder = new BodyModelBinder(inputFormatters, new TestHttpRequestStreamReaderFactory(), loggerFactory);
+            var binder = new BodyModelBinder(inputFormatters, new TestHttpRequestStreamReaderFactory(), loggerFactory, isBindingRequired: true);
 
             // Act
             await binder.BindModelAsync(bindingContext);
@@ -277,7 +323,7 @@ public async Task BindModelAsync_DoesNotThrowNullReferenceException()
                 typeof(Person),
                 httpContext: httpContext,
                 metadataProvider: provider);
-            var binder = new BodyModelBinder(new List<IInputFormatter>(), new TestHttpRequestStreamReaderFactory());
+            var binder = new BodyModelBinder(new List<IInputFormatter>(), new TestHttpRequestStreamReaderFactory(), isBindingRequired: true);
 
             // Act & Assert (does not throw)
             await binder.BindModelAsync(bindingContext);
@@ -316,11 +362,11 @@ private static DefaultModelBindingContext GetBindingContext(
             return bindingContext;
         }
 
-        private static BodyModelBinder CreateBinder(IList<IInputFormatter> formatters)
+        private static BodyModelBinder CreateBinder(IList<IInputFormatter> formatters, bool isBindingRequired = true)
         {
             var sink = new TestSink();
             var loggerFactory = new TestLoggerFactory(sink, enabled: true);
-            return new BodyModelBinder(formatters, new TestHttpRequestStreamReaderFactory(), loggerFactory);
+            return new BodyModelBinder(formatters, new TestHttpRequestStreamReaderFactory(), loggerFactory, isBindingRequired);
         }
 
         private class Person

test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputFormatterTests.cs

@@ -79,6 +79,24 @@ public async Task JsonInputFormatter_IsModelStateValid_ForValidContentType(
             Assert.Equal(expectedSampleIntValue.ToString(), responseBody);
         }
 
+        [Theory]
+        [InlineData("application/json", "")]
+        [InlineData("application/json", "    ")]
+        public async Task JsonInputFormatter_IsModelStateValid_FalseForEmptyRequestBody(
+            string requestContentType,
+            string jsonInput)
+        {
+            // Arrange
+            var content = new StringContent(jsonInput, Encoding.UTF8, requestContentType);
+
+            // Act
+            var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnInput/", content);
+            var responseBody = await response.Content.ReadAsStringAsync();
+
+            // Assert
+            Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
+        }
+
         [Theory]
         [InlineData("\"I'm a JSON string!\"")]
         [InlineData("true")]

test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs

@@ -45,6 +45,11 @@ public IActionResult ReturnsIndentedJson()
         [HttpPost]
         public IActionResult ReturnInput([FromBody]DummyClass dummyObject)
         {
+            if (!ModelState.IsValid)
+            {
+                return BadRequest();
+            }
+
             return Content(dummyObject.SampleInt.ToString());
         }