diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index 7ea6315c6945..b5dded52a176 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Linq; +using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; namespace Microsoft.AspNetCore.Mvc.ModelBinding; @@ -246,8 +247,16 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) PropertyFilterProvider = modelMetadata.PropertyFilterProvider; } - // There isn't a ModelMetadata feature to configure AllowEmptyInputInBodyModelBinding, - // so nothing to infer from it. + // If the EmptyBody behavior is not configured will be inferred + // as Allow when the NullablityState == NullablityStateNull or HasDefaultValue + // https://github.com/dotnet/aspnetcore/issues/39754 + if (EmptyBodyBehavior == EmptyBodyBehavior.Default && + BindingSource == BindingSource.Body && + (modelMetadata.NullabilityState == NullabilityState.Nullable || modelMetadata.IsNullableValueType || modelMetadata.HasDefaultValue)) + { + isBindingInfoPresent = true; + EmptyBodyBehavior = EmptyBodyBehavior.Allow; + } return isBindingInfoPresent; } diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index bbcf91a97eda..1fd536c1c43b 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -509,6 +509,12 @@ internal IReadOnlyDictionary BoundConstructorPrope /// internal virtual string? ValidationModelName { get; } + /// + /// Gets the value that indicates if the parameter has a default value set. + /// This is only available when is otherwise it will be false. + /// + internal bool HasDefaultValue { get; private set; } + /// /// Throws if the ModelMetadata is for a record type with validation on properties. /// @@ -620,6 +626,7 @@ private void InitializeTypeInformation() IsNullableValueType = Nullable.GetUnderlyingType(ModelType) != null; IsReferenceOrNullableType = !ModelType.IsValueType || IsNullableValueType; UnderlyingOrModelType = Nullable.GetUnderlyingType(ModelType) ?? ModelType; + HasDefaultValue = MetadataKind == ModelMetadataKind.Parameter && Identity.ParameterInfo!.HasDefaultValue; var collectionType = ClosedGenericMatcher.ExtractGenericInterface(ModelType, typeof(ICollection<>)); IsCollectionType = collectionType != null; diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs index 07199c7dd0d9..a9680cd9e78c 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; @@ -44,6 +44,23 @@ public void GetBindingInfo_ReadsPropertyPredicateProvider() Assert.Same(bindAttribute, bindingInfo.PropertyFilterProvider); } + [Fact] + public void GetBindingInfo_ReadsEmptyBodyBehavior() + { + // Arrange + var attributes = new object[] + { + new FromBodyAttribute { EmptyBodyBehavior = EmptyBodyBehavior.Allow }, + }; + + // Act + var bindingInfo = BindingInfo.GetBindingInfo(attributes); + + // Assert + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior); + } + [Fact] public void GetBindingInfo_ReadsRequestPredicateProvider() { @@ -102,6 +119,26 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesValuesFromBindingI Assert.Same("Test", bindingInfo.BinderModelName); } + [Fact] + public void GetBindingInfo_WithAttributesAndModelMetadata_UsesEmptyBodyBehaviorFromBindingInfo_IfAttributesPresent() + { + // Arrange + var attributes = new object[] + { + new FromBodyAttribute() { EmptyBodyBehavior = EmptyBodyBehavior.Disallow } + }; + var modelType = typeof(Guid?); + var provider = new TestModelMetadataProvider(); + var modelMetadata = provider.GetMetadataForType(modelType); + + // Act + var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata); + + // Assert + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Disallow, bindingInfo.EmptyBodyBehavior); + } + [Fact] public void GetBindingInfo_WithAttributesAndModelMetadata_UsesBinderNameFromModelMetadata_WhenNotFoundViaAttributes() { @@ -205,4 +242,48 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesPropertyPredicateP Assert.NotNull(bindingInfo); Assert.Same(propertyFilterProvider, bindingInfo.PropertyFilterProvider); } + + [Fact] + public void GetBindingInfo_WithAttributesAndModelMetadata_UsesEmptyBodyFromModelMetadata_WhenNotFoundViaAttributes() + { + // Arrange + var attributes = new object[] + { + new ControllerAttribute(), + new BindNeverAttribute(), + new FromBodyAttribute(), + }; + var modelType = typeof(Guid?); + var provider = new TestModelMetadataProvider(); + var modelMetadata = provider.GetMetadataForType(modelType); + + // Act + var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata); + + // Assert + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior); + } + + [Fact] + public void GetBindingInfo_WithAttributesAndModelMetadata_PreserveEmptyBodyDefault_WhenNotNullable() + { + // Arrange + var attributes = new object[] + { + new ControllerAttribute(), + new BindNeverAttribute(), + new FromBodyAttribute(), + }; + var modelType = typeof(Guid); + var provider = new TestModelMetadataProvider(); + var modelMetadata = provider.GetMetadataForType(modelType); + + // Act + var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata); + + // Assert + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior); + } } diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index 11258a03b138..23b302191651 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Linq; +using System.Reflection; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing.Template; using Microsoft.Extensions.DependencyInjection; @@ -106,6 +107,12 @@ internal void InferParameterBindingSources(ActionModel action) message += Environment.NewLine + parameters; throw new InvalidOperationException(message); } + else if (fromBodyParameters.Count == 1 && + fromBodyParameters[0].BindingInfo!.EmptyBodyBehavior == EmptyBodyBehavior.Default && + IsOptionalParameter(fromBodyParameters[0])) + { + fromBodyParameters[0].BindingInfo!.EmptyBodyBehavior = EmptyBodyBehavior.Allow; + } } // Internal for unit testing. @@ -155,4 +162,25 @@ private bool IsComplexTypeParameter(ParameterModel parameter) return metadata.IsComplexType; } + + private bool IsOptionalParameter(ParameterModel parameter) + { + if (parameter.ParameterInfo.HasDefaultValue) + { + return true; + } + + if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider) + { + var metadata = modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo); + return metadata.NullabilityState == NullabilityState.Nullable || metadata.IsNullableValueType; + } + else + { + // Cannot be determine if the parameter is optional since the provider + // does not provides an option to getMetadata from the parameter info + // so, we will NOT treat the parameter as optional. + return false; + } + } } diff --git a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs index a7f278a8fa8b..0edead1188b2 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Core; @@ -98,8 +99,11 @@ public virtual Task ReadAsync(InputFormatterContext contex throw new ArgumentNullException(nameof(context)); } - var request = context.HttpContext.Request; - if (request.ContentLength == 0) + var canHaveBody = context.HttpContext.Features.Get()?.CanHaveBody; + // In case the feature is not registered + canHaveBody ??= context.HttpContext.Request.ContentLength != 0; + + if (canHaveBody is false) { if (context.TreatEmptyInputAsDefaultValue) { diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs index de561eed5c29..68513623fffd 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs @@ -109,6 +109,7 @@ public void InferParameterBindingSources_InfersSources() var bindingInfo = parameter.BindingInfo; Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior); Assert.Same(BindingSource.Body, bindingInfo.BindingSource); }, parameter => @@ -121,6 +122,84 @@ public void InferParameterBindingSources_InfersSources() }); } + [Fact] + public void InferParameterBindingSources_InfersSourcesFromRequiredComplexType() + { + // Arrange + var actionName = nameof(ParameterBindingController.RequiredComplexType); + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var convention = GetConvention(modelMetadataProvider); + var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); + + // Act + convention.InferParameterBindingSources(action); + + // Assert + Assert.Collection( + action.Parameters, + parameter => + { + Assert.Equal("model", parameter.Name); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior); + Assert.Same(BindingSource.Body, bindingInfo.BindingSource); + }); + } + + [Fact] + public void InferParameterBindingSources_InfersSourcesFromNullableComplexType() + { + // Arrange + var actionName = nameof(ParameterBindingController.NullableComplexType); + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var convention = GetConvention(modelMetadataProvider); + var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); + + // Act + convention.InferParameterBindingSources(action); + + // Assert + Assert.Collection( + action.Parameters, + parameter => + { + Assert.Equal("model", parameter.Name); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior); + Assert.Same(BindingSource.Body, bindingInfo.BindingSource); + }); + } + + [Fact] + public void InferParameterBindingSources_InfersSourcesFromComplexTypeWithDefaultValue() + { + // Arrange + var actionName = nameof(ParameterBindingController.ComplexTypeWithDefaultValue); + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var convention = GetConvention(modelMetadataProvider); + var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); + + // Act + convention.InferParameterBindingSources(action); + + // Assert + Assert.Collection( + action.Parameters, + parameter => + { + Assert.Equal("model", parameter.Name); + + var bindingInfo = parameter.BindingInfo; + Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior); + Assert.Same(BindingSource.Body, bindingInfo.BindingSource); + }); + } + [Fact] public void Apply_PreservesBindingInfo_WhenInferringFor_ParameterWithModelBinder_AndExplicitName() { @@ -847,6 +926,17 @@ private class ParameterBindingController [HttpPut("cancellation")] public IActionResult ComplexTypeModelWithCancellationToken(TestModel model, CancellationToken cancellationToken) => null; +#nullable enable + [HttpPut("parameter-notnull")] + public IActionResult RequiredComplexType(TestModel model) => new OkResult(); + + [HttpPut("parameter-null")] + public IActionResult NullableComplexType(TestModel? model) => new OkResult(); +#nullable restore + + [HttpPut("parameter-with-default-value")] + public IActionResult ComplexTypeWithDefaultValue(TestModel model = null) => null; + [HttpGet("parameter-with-model-binder-attribute")] public IActionResult ModelBinderAttribute([ModelBinder(Name = "top")] int value) => null; diff --git a/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs b/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs index f5ce229db607..6183239a5c0b 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs @@ -640,6 +640,7 @@ protected static HttpContext GetHttpContext( var httpContext = new DefaultHttpContext(); httpContext.Request.Body = requestStream; httpContext.Request.ContentType = contentType; + httpContext.Request.ContentLength = requestStream.Length; return httpContext; } diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs index 4f96e1c81b96..a4ebddf109e4 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs @@ -740,10 +740,13 @@ private static HttpContext GetHttpContext( request.SetupGet(r => r.Headers).Returns(headers.Object); request.SetupGet(f => f.Body).Returns(new MemoryStream(contentBytes)); request.SetupGet(f => f.ContentType).Returns(contentType); + request.SetupGet(f => f.ContentLength).Returns(contentBytes.Length); var httpContext = new Mock(); + var features = new Mock(); httpContext.SetupGet(c => c.Request).Returns(request.Object); httpContext.SetupGet(c => c.Request).Returns(request.Object); + httpContext.SetupGet(c => c.Features).Returns(features.Object); return httpContext.Object; } diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs index 54c67cec905b..050b6fbfd956 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs @@ -676,10 +676,12 @@ private static HttpContext GetHttpContext( request.SetupGet(r => r.Headers).Returns(headers.Object); request.SetupGet(f => f.Body).Returns(new MemoryStream(contentBytes)); request.SetupGet(f => f.ContentType).Returns(contentType); + request.SetupGet(f => f.ContentLength).Returns(contentBytes.Length); var httpContext = new Mock(); + var features = new Mock(); httpContext.SetupGet(c => c.Request).Returns(request.Object); - httpContext.SetupGet(c => c.Request).Returns(request.Object); + httpContext.SetupGet(c => c.Features).Returns(features.Object); return httpContext.Object; } diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs index 2e673bb2cb52..8d8d9cd83d2c 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs @@ -414,11 +414,14 @@ public async Task ReadAsync_RegistersFileStreamForDisposal() new MvcOptions(), new MvcNewtonsoftJsonOptions()); var httpContext = new Mock(); + var features = new Mock(); + httpContext.SetupGet(c => c.Features).Returns(features.Object); IDisposable registerForDispose = null; var content = Encoding.UTF8.GetBytes("\"Hello world\""); httpContext.Setup(h => h.Request.Body).Returns(new NonSeekableReadStream(content, allowSyncReads: false)); httpContext.Setup(h => h.Request.ContentType).Returns("application/json"); + httpContext.Setup(h => h.Request.ContentLength).Returns(content.Length); httpContext.Setup(h => h.Response.RegisterForDispose(It.IsAny())) .Callback((IDisposable disposable) => registerForDispose = disposable) .Verifiable(); diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs index 8223e4e076fc..499ab6ff8dfb 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs @@ -260,10 +260,12 @@ private static HttpContext CreateHttpContext( request.SetupGet(r => r.Headers).Returns(headers.Object); request.SetupGet(f => f.Body).Returns(new MemoryStream(contentBytes)); request.SetupGet(f => f.ContentType).Returns(contentType); + request.SetupGet(f => f.ContentLength).Returns(contentBytes.Length); var httpContext = new Mock(); + var features = new Mock(); httpContext.SetupGet(c => c.Request).Returns(request.Object); - httpContext.SetupGet(c => c.Request).Returns(request.Object); + httpContext.SetupGet(c => c.Features).Returns(features.Object); return httpContext.Object; } diff --git a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs index 5b18d213ccfd..bc40e1afa013 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs @@ -186,7 +186,30 @@ public async Task BodyIsRequiredByDefault() } [Fact] - public async Task BodyIsRequiredByDefaultFailsWithEmptyBody() + public async Task BodyIsRequiredByDefault_WhenNullableContextEnabled() + { + // Act + var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.NonNullableBody)}", value: null); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + var problemDetails = await response.Content.ReadFromJsonAsync(); + Assert.Collection( + problemDetails.Errors, + kvp => + { + Assert.Empty(kvp.Key); + Assert.Equal("A non-empty request body is required.", Assert.Single(kvp.Value)); + }, + kvp => + { + Assert.NotEmpty(kvp.Key); + Assert.Equal("The dummy field is required.", Assert.Single(kvp.Value)); + }); + } + + [Fact] + public async Task BodyIsRequiredByDefaultFailsWithContentLengthZero() { var content = new ByteArrayContent(Array.Empty()); Assert.Null(content.Headers.ContentType); @@ -209,6 +232,26 @@ public async Task OptionalFromBodyWorks() await response.AssertStatusCodeAsync(HttpStatusCode.OK); } + [Fact] + public async Task OptionalFromBodyWorks_WithDefaultValue() + { + // Act + var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.DefaultValueBody)}", value: null); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + + [Fact] + public async Task OptionalFromBodyWorks_WithNullable() + { + // Act + var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.NullableBody)}", value: null); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + [Fact] public async Task OptionalFromBodyWorksWithEmptyRequest() { @@ -223,4 +266,34 @@ public async Task OptionalFromBodyWorksWithEmptyRequest() // Assert await response.AssertStatusCodeAsync(HttpStatusCode.OK); } + + [Fact] + public async Task OptionalFromBodyWorksWithEmptyRequest_WithDefaultValue() + { + // Arrange + var content = new ByteArrayContent(Array.Empty()); + Assert.Null(content.Headers.ContentType); + Assert.Equal(0, content.Headers.ContentLength); + + // Act + var response = await Client.PostAsync($"Home/{nameof(HomeController.DefaultValueBody)}", content); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + + [Fact] + public async Task OptionalFromBodyWorksWithEmptyRequest_WithNullable() + { + // Arrange + var content = new ByteArrayContent(Array.Empty()); + Assert.Null(content.Headers.ContentType); + Assert.Equal(0, content.Headers.ContentLength); + + // Act + var response = await Client.PostAsync($"Home/{nameof(HomeController.NullableBody)}", content); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs index aec76e130eea..9e8ed6e254d7 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs @@ -39,6 +39,23 @@ public async Task JsonInputFormatter_ReturnsBadRequest_ForEmptyRequestBody( // Arrange var content = new StringContent(jsonInput, Encoding.UTF8, requestContentType); + // Act + var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnNonNullableInput/", content); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + + [Theory] + [InlineData("application/json", "")] + [InlineData("application/json", " ")] + public async Task JsonInputFormatter_ReturnsBadRequest_ForEmptyRequestBody_WhenNullableIsNotSet( + string requestContentType, + string jsonInput) + { + // Arrange + var content = new StringContent(jsonInput, Encoding.UTF8, requestContentType); + // Act var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnInput/", content); diff --git a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index 20a25ffcbce2..b4b0ac899f2d 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -370,6 +370,7 @@ public async Task FromBodyAllowingEmptyInputAndRequiredOnProperty_EmptyBody_Adds { request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; + request.ContentLength = 0; }); testContext.MvcOptions.AllowEmptyInputInBodyModelBinding = true; @@ -411,6 +412,7 @@ public async Task FromBodyAllowingEmptyInputOnActionParameter_EmptyBody_BindsToN { request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; + request.ContentLength = 0; }); testContext.MvcOptions.AllowEmptyInputInBodyModelBinding = true; @@ -435,8 +437,8 @@ private class Person4 public int Address { get; set; } } - [Fact] // This tests the 2.0 behavior. Error messages from JSON.NET are not preserved. - public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_JsonFormatterAddsModelStateError() + [Fact] + public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_AddsModelStateError() { // Arrange var testContext = ModelBindingTestHelper.GetTestContext( @@ -444,6 +446,7 @@ public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_JsonFormatter { request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; + request.ContentLength = 0; }); // Override the AllowInputFormatterExceptionMessages setting ModelBindingTestHelper chooses. @@ -473,13 +476,11 @@ public async Task FromBodyAndRequiredOnValueTypeProperty_EmptyBody_JsonFormatter Assert.False(modelState.IsValid); var entry = Assert.Single(modelState); Assert.Equal("CustomParameter.Address", entry.Key); - Assert.Null(entry.Value.AttemptedValue); + Assert.Null(entry.Value!.AttemptedValue); Assert.Null(entry.Value.RawValue); - var error = Assert.Single(entry.Value.Errors); - // Update me in 3.0 when MvcJsonOptions.AllowInputFormatterExceptionMessages is removed - Assert.IsType(error.Exception); - Assert.Empty(error.ErrorMessage); + var error = Assert.Single(entry.Value.Errors); + Assert.Equal("A non-empty request body is required.", error.ErrorMessage); } private class Person5 @@ -488,6 +489,14 @@ private class Person5 public Address5 Address { get; set; } } +#nullable enable + private class Person5WithNullableContext + { + [FromBody] + public Address5 Address { get; set; } = default!; + } +#nullable restore + private class Address5 { public int Number { get; set; } @@ -587,11 +596,12 @@ public async Task FromBodyWithInvalidPropertyData_JsonFormatterAddsModelError() } [Theory] - [InlineData(false, false)] - [InlineData(true, true)] - public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( - bool allowEmptyInputInBodyModelBindingSetting, - bool expectedModelStateIsValid) + [InlineData(typeof(Person5), false)] + [InlineData(typeof(Person5WithNullableContext), true)] + [InlineData(typeof(Person5WithNullableContext), false)] + public async Task FromBodyWithEmptyBody_ModelStateIsInvalid_HasModelErrors( + Type modelType, + bool allowEmptyInputInBodyModelBindingSetting) { // Arrange var parameter = new ParameterDescriptor @@ -601,7 +611,7 @@ public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( { BinderModelName = "CustomParameter", }, - ParameterType = typeof(Person5) + ParameterType = modelType }; var testContext = ModelBindingTestHelper.GetTestContext( @@ -609,6 +619,7 @@ public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( { request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; + request.ContentLength = 0; }, options => options.AllowEmptyInputInBodyModelBinding = allowEmptyInputInBodyModelBindingSetting); @@ -620,26 +631,53 @@ public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( // Assert Assert.True(modelBindingResult.IsModelSet); - var boundPerson = Assert.IsType(modelBindingResult.Model); - Assert.NotNull(boundPerson); + Assert.IsType(modelType, modelBindingResult.Model); - if (expectedModelStateIsValid) - { - Assert.True(modelState.IsValid); - } - else + Assert.False(modelState.IsValid); + var entry = Assert.Single(modelState); + Assert.Equal("CustomParameter.Address", entry.Key); + var street = entry.Value; + Assert.Equal(ModelValidationState.Invalid, street.ValidationState); + var error = Assert.Single(street.Errors); + + // Since the message doesn't come from DataAnnotations, we don't have a way to get the + // exact string, so just check it's nonempty. + Assert.NotEmpty(error.ErrorMessage); + } + + [Fact] + public async Task FromBodyWithEmptyBody_ModelStateIsValid_WhenAllowEmptyInput() + { + // Arrange + var parameter = new ParameterDescriptor { - Assert.False(modelState.IsValid); - var entry = Assert.Single(modelState); - Assert.Equal("CustomParameter.Address", entry.Key); - var street = entry.Value; - Assert.Equal(ModelValidationState.Invalid, street.ValidationState); - var error = Assert.Single(street.Errors); - - // Since the message doesn't come from DataAnnotations, we don't have a way to get the - // exact string, so just check it's nonempty. - Assert.NotEmpty(error.ErrorMessage); - } + Name = "Parameter1", + BindingInfo = new BindingInfo + { + BinderModelName = "CustomParameter", + }, + ParameterType = typeof(Person5) + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); + request.ContentType = "application/json"; + request.ContentLength = 0; + }, + options => options.AllowEmptyInputInBodyModelBinding = true); + + var modelState = testContext.ModelState; + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices); + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + Assert.IsType(modelBindingResult.Model); + Assert.True(modelState.IsValid); } private class Person2 diff --git a/src/Mvc/test/Mvc.IntegrationTests/ModelBindingTestHelper.cs b/src/Mvc/test/Mvc.IntegrationTests/ModelBindingTestHelper.cs index 1ff8f0ceb7bb..e4c155100d8d 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -151,6 +151,7 @@ private static HttpContext GetHttpContext( { var httpContext = new DefaultHttpContext(); httpContext.Features.Set(new CancellableRequestLifetimeFeature()); + httpContext.Features.Set(new NonZeroContentLengthRequestBodyDetectionFeature(httpContext)); updateRequest?.Invoke(httpContext.Request); @@ -215,4 +216,16 @@ public void Abort() _cts.Cancel(); } } + + private class NonZeroContentLengthRequestBodyDetectionFeature : IHttpRequestBodyDetectionFeature + { + private readonly HttpContext _context; + + public NonZeroContentLengthRequestBodyDetectionFeature(HttpContext context) + { + _context = context; + } + + public bool CanHaveBody => _context.Request.ContentLength != 0; + } } diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs index 77afc496bbfa..4a27b56a6b82 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.ComponentModel.DataAnnotations; using System.Globalization; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -44,4 +45,18 @@ public IActionResult DefaultBody([FromBody] DummyClass dummy) [HttpPost] public IActionResult OptionalBody([FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] DummyClass dummy) => ModelState.IsValid ? Ok() : ValidationProblem(); + + [HttpPost] + public IActionResult DefaultValueBody([FromBody] DummyClass dummy = null) + => ModelState.IsValid ? Ok() : ValidationProblem(); + +#nullable enable + [HttpPost] + public IActionResult NonNullableBody([FromBody] DummyClass dummy) + => ModelState.IsValid ? Ok() : ValidationProblem(); + + [HttpPost] + public IActionResult NullableBody([FromBody] DummyClass? dummy) + => ModelState.IsValid ? Ok() : ValidationProblem(); +#nullable restore } diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs index 8c1bd068e598..1376f67b3043 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/JsonFormatterController.cs @@ -44,8 +44,9 @@ public IActionResult ReturnsIndentedJson() return objectResult; } +#nullable enable [HttpPost] - public IActionResult ReturnInput([FromBody] DummyClass dummyObject) + public IActionResult ReturnNonNullableInput([FromBody] DummyClass dummyObject) { if (!ModelState.IsValid) { @@ -54,6 +55,18 @@ public IActionResult ReturnInput([FromBody] DummyClass dummyObject) return Content(dummyObject.SampleInt.ToString(CultureInfo.InvariantCulture)); } +#nullable restore + + [HttpPost] + public IActionResult ReturnInput([FromBody] DummyClass dummyObject) + { + if (!ModelState.IsValid) + { + return BadRequest(ModelState); + } + + return Content(dummyObject?.SampleInt.ToString(CultureInfo.InvariantCulture)); + } [HttpPost] public IActionResult ValueTypeAsBody([FromBody] int value)