From 46f71aa5e527285250cddfba4d3e56206bfb4246 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jan 2022 13:19:30 -0800 Subject: [PATCH 01/16] Adding NullableEmptyBodyProvider and inferring EmptyBehavior=Allow --- .../src/ModelBinding/BindingInfo.cs | 7 ++- .../src/ModelBinding/ModelMetadata.cs | 5 ++ .../Infrastructure/MvcCoreMvcOptionsSetup.cs | 2 + .../ModelBinding/Metadata/BindingMetadata.cs | 5 ++ .../Metadata/DefaultModelMetadata.cs | 3 ++ ...ullableEmptyBodyBindingMetadataProvider.cs | 48 +++++++++++++++++++ src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 3 ++ 7 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index 7ea6315c6945..84c25e4eba26 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -246,8 +246,11 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) PropertyFilterProvider = modelMetadata.PropertyFilterProvider; } - // There isn't a ModelMetadata feature to configure AllowEmptyInputInBodyModelBinding, - // so nothing to infer from it. + if (EmptyBodyBehavior == EmptyBodyBehavior.Default && modelMetadata.IsEmptyBodyAllowed != null) + { + isBindingInfoPresent = true; + EmptyBodyBehavior = modelMetadata.IsEmptyBodyAllowed == true ? EmptyBodyBehavior.Allow : EmptyBodyBehavior.Disallow; + } return isBindingInfoPresent; } diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs index 7f9e1d99a79e..6bfbd509d685 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -499,6 +499,11 @@ internal IReadOnlyDictionary BoundConstructorPrope /// internal virtual string? ValidationModelName { get; } + /// + /// Gets or sets the value which decides if empty bodies are treated as valid inputs. + /// + internal virtual bool? IsEmptyBodyAllowed { get; set; } + /// /// Throws if the ModelMetadata is for a record type with validation on properties. /// diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs index 652af4ed3507..49f501e86bfe 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs @@ -123,6 +123,8 @@ internal static void ConfigureAdditionalModelMetadataDetailsProviders(IList public bool? IsReadOnly { get; set; } + /// + /// Gets or sets the value which decides if empty bodies are treated as valid inputs. + /// + internal bool? IsEmptyBodyAllowed { get; set; } + /// /// Gets the instance. See /// . diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs index 429c18fef155..0d88d9a37ceb 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -474,6 +474,9 @@ public override bool? HasValidators /// internal override string? ValidationModelName => ValidationMetadata.ValidationModelName; + /// + internal override bool? IsEmptyBodyAllowed => BindingMetadata.IsEmptyBodyAllowed; + internal static bool CalculateHasValidators(HashSet visited, ModelMetadata metadata) { RuntimeHelpers.EnsureSufficientExecutionStack(); diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs new file mode 100644 index 000000000000..8fb3f13b168c --- /dev/null +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs @@ -0,0 +1,48 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +#nullable enable + +using System.Reflection; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; + +/// +/// An which configures to +/// true for Nullable or with default value types. +/// +public class NullableEmptyBodyBindingMetadataProvider : IBindingMetadataProvider +{ + private readonly NullabilityInfoContext _nullabilityContext = new(); + + /// + public void CreateBindingMetadata(BindingMetadataProviderContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + // No-op if the metadata is not for the Parameter Metadatakind + if (context.Key.MetadataKind == ModelMetadataKind.Parameter + && !context.BindingMetadata.IsEmptyBodyAllowed.HasValue + && IsOptional(context.Key)) + { + context.BindingMetadata.IsEmptyBodyAllowed = true; + } + } + + // internal for testing + internal bool IsOptional(ModelMetadataIdentity identity) + { + // If the parameter has a default value we don't need to + // work with the NullabilityInfoContext + if (identity.ParameterInfo!.HasDefaultValue) + { + return true; + } + + var nullability = _nullabilityContext.Create(identity.ParameterInfo!); + return nullability.ReadState == NullabilityState.Nullable; + } +} diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index d326535ca4d1..70ff8c728524 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -1,4 +1,7 @@ #nullable enable +Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.NullableEmptyBodyBindingMetadataProvider +Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.NullableEmptyBodyBindingMetadataProvider.CreateBindingMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.BindingMetadataProviderContext! context) -> void +Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.NullableEmptyBodyBindingMetadataProvider.NullableEmptyBodyBindingMetadataProvider() -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateDisplayMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DisplayMetadataProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateValidationMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadataProviderContext! context) -> void From f7ccefb3098849680e68adcda00af191d1537a80 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jan 2022 13:19:48 -0800 Subject: [PATCH 02/16] Adding/Updating tests --- ...bleEmptyBodyBindingMetadataProviderTest.cs | 110 ++++++++++++++++++ .../InputFormatterTests.cs | 50 ++++++++ .../Controllers/HomeController.cs | 10 ++ 3 files changed, 170 insertions(+) create mode 100644 src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs new file mode 100644 index 000000000000..d03024d4852f --- /dev/null +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs @@ -0,0 +1,110 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +using System.Reflection; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; + +#nullable enable +public class NullableEmptyBodyBindingMetadataProviderTest +{ + [Fact] + public void IsEmptyBodyAllowed_LeftAlone_WhenAlreadySet() + { + // Arrange + var provider = new NullableEmptyBodyBindingMetadataProvider(); + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.NullableParameter), + new ModelAttributes(Array.Empty())); + + context.BindingMetadata.IsEmptyBodyAllowed = false; + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.False(context.BindingMetadata.IsEmptyBodyAllowed); + } + + [Fact] + public void IsEmptyBodyAllowed_LeftAlone_WhenNotOptional() + { + // Arrange + var provider = new NullableEmptyBodyBindingMetadataProvider(); + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.NonNullableParameter), + new ModelAttributes(Array.Empty())); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.Null(context.BindingMetadata.IsEmptyBodyAllowed); + } + + + [Fact] + public void IsEmptyBodyAllowed_IsTrue_WhenDefaultValue() + { + // Arrange + var provider = new NullableEmptyBodyBindingMetadataProvider(); + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.ParameterWithDefaultValue), + new ModelAttributes(Array.Empty())); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.True(context.BindingMetadata.IsEmptyBodyAllowed); + + } + + [Fact] + public void IsEmptyBodyAllowed_IsTrue_WhenNullable() + { + // Arrange + var provider = new NullableEmptyBodyBindingMetadataProvider(); + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.NullableParameter), + new ModelAttributes(Array.Empty())); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.True(context.BindingMetadata.IsEmptyBodyAllowed); + + } + + private class ParameterInfos + { + public void Method(object param1, object? param2) + { + } + +#nullable disable + public void Method2(object param1 = null) + { + } +#nullable restore + + public static ParameterInfo NonNullableParameter + = typeof(ParameterInfos)! + .GetMethod(nameof(ParameterInfos.Method))! + .GetParameters()[0]; + + public static ParameterInfo NullableParameter + = typeof(ParameterInfos)! + .GetMethod(nameof(ParameterInfos.Method))! + .GetParameters()[1]; + + public static ParameterInfo ParameterWithDefaultValue + = typeof(ParameterInfos)! + .GetMethod(nameof(ParameterInfos.Method2))! + .GetParameters()[0]; + } +} diff --git a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs index 5b18d213ccfd..9f1ac17a23a5 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs @@ -209,6 +209,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 +243,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/WebSites/FormatterWebSite/Controllers/HomeController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs index 77afc496bbfa..4f9e77f73519 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs @@ -44,4 +44,14 @@ 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 NullableBody([FromBody] DummyClass? dummy) + => ModelState.IsValid ? Ok() : ValidationProblem(); +#nullable restore } From 752b53962707bbfe4632395eb03025944cca63fe Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 09:35:52 -0800 Subject: [PATCH 03/16] Allowing empty input --- .../src/ModelBinding/BindingInfo.cs | 9 +++- .../src/ModelBinding/ModelMetadata.cs | 6 ++- .../InferParameterBindingInfoConvention.cs | 32 +++++++++++++ .../Infrastructure/MvcCoreMvcOptionsSetup.cs | 2 - .../Metadata/DefaultModelMetadata.cs | 3 -- ...ullableEmptyBodyBindingMetadataProvider.cs | 48 ------------------- src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt | 3 -- 7 files changed, 43 insertions(+), 60 deletions(-) delete mode 100644 src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index 84c25e4eba26..ff060103fd59 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -246,10 +246,15 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) PropertyFilterProvider = modelMetadata.PropertyFilterProvider; } - if (EmptyBodyBehavior == EmptyBodyBehavior.Default && modelMetadata.IsEmptyBodyAllowed != null) + // If the EmptyBody behavior is not configured will be infer + // as Allow when the ModelMetadata.IsRequired is false or HasDefaultValue + // https://github.com/dotnet/aspnetcore/issues/39754 + if (EmptyBodyBehavior == EmptyBodyBehavior.Default && + BindingSource == BindingSource.Body && + (!modelMetadata.IsRequired || modelMetadata.HasDefaultValue)) { isBindingInfoPresent = true; - EmptyBodyBehavior = modelMetadata.IsEmptyBodyAllowed == true ? EmptyBodyBehavior.Allow : EmptyBodyBehavior.Disallow; + 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 6bfbd509d685..849018609ef1 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs @@ -500,9 +500,10 @@ internal IReadOnlyDictionary BoundConstructorPrope internal virtual string? ValidationModelName { get; } /// - /// Gets or sets the value which decides if empty bodies are treated as valid inputs. + /// 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 virtual bool? IsEmptyBodyAllowed { get; set; } + internal bool HasDefaultValue { get; private set; } /// /// Throws if the ModelMetadata is for a record type with validation on properties. @@ -615,6 +616,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.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index fa8c9fd2245b..af7250269f13 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -88,6 +88,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. @@ -132,4 +138,30 @@ 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.IsRequired; + } + 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 use the Nullability context + + // Waiting for PR https://github.com/dotnet/aspnetcore/pull/39804 + // No need for information from attributes on the parameter. Just use its type. + // var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); + // return metadata.NullabilityState != NullabilityState.NotNull; + return false; + } + } } diff --git a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs index 49f501e86bfe..652af4ed3507 100644 --- a/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs +++ b/src/Mvc/Mvc.Core/src/Infrastructure/MvcCoreMvcOptionsSetup.cs @@ -123,8 +123,6 @@ internal static void ConfigureAdditionalModelMetadataDetailsProviders(IList internal override string? ValidationModelName => ValidationMetadata.ValidationModelName; - /// - internal override bool? IsEmptyBodyAllowed => BindingMetadata.IsEmptyBodyAllowed; - internal static bool CalculateHasValidators(HashSet visited, ModelMetadata metadata) { RuntimeHelpers.EnsureSufficientExecutionStack(); diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs deleted file mode 100644 index 8fb3f13b168c..000000000000 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProvider.cs +++ /dev/null @@ -1,48 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -#nullable enable - -using System.Reflection; - -namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; - -/// -/// An which configures to -/// true for Nullable or with default value types. -/// -public class NullableEmptyBodyBindingMetadataProvider : IBindingMetadataProvider -{ - private readonly NullabilityInfoContext _nullabilityContext = new(); - - /// - public void CreateBindingMetadata(BindingMetadataProviderContext context) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - // No-op if the metadata is not for the Parameter Metadatakind - if (context.Key.MetadataKind == ModelMetadataKind.Parameter - && !context.BindingMetadata.IsEmptyBodyAllowed.HasValue - && IsOptional(context.Key)) - { - context.BindingMetadata.IsEmptyBodyAllowed = true; - } - } - - // internal for testing - internal bool IsOptional(ModelMetadataIdentity identity) - { - // If the parameter has a default value we don't need to - // work with the NullabilityInfoContext - if (identity.ParameterInfo!.HasDefaultValue) - { - return true; - } - - var nullability = _nullabilityContext.Create(identity.ParameterInfo!); - return nullability.ReadState == NullabilityState.Nullable; - } -} diff --git a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt index 70ff8c728524..d326535ca4d1 100644 --- a/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt +++ b/src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt @@ -1,7 +1,4 @@ #nullable enable -Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.NullableEmptyBodyBindingMetadataProvider -Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.NullableEmptyBodyBindingMetadataProvider.CreateBindingMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.BindingMetadataProviderContext! context) -> void -Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.NullableEmptyBodyBindingMetadataProvider.NullableEmptyBodyBindingMetadataProvider() -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateDisplayMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.DisplayMetadataProviderContext! context) -> void Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.SystemTextJsonValidationMetadataProvider.CreateValidationMetadata(Microsoft.AspNetCore.Mvc.ModelBinding.Metadata.ValidationMetadataProviderContext! context) -> void From a5a47b00190802dd05beaed17fc5b84574712f88 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 09:36:26 -0800 Subject: [PATCH 04/16] Fix how to check for empty input --- src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs index a7f278a8fa8b..7257d6df6c32 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 hasBody = context.HttpContext.Features.Get()?.CanHaveBody; + // In case the feature is not registered + hasBody ??= context.HttpContext.Request.ContentLength is not null && context.HttpContext.Request.ContentLength != 0; + + if (hasBody == false) { if (context.TreatEmptyInputAsDefaultValue) { From 51433f05ef632752447a3d519e431270ca482b24 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 09:37:13 -0800 Subject: [PATCH 05/16] Updating tests --- .../test/ModelBinding/BindingInfoTest.cs | 41 ++++++- ...InferParameterBindingInfoConventionTest.cs | 63 +++++++++- .../test/Formatters/InputFormatterTest.cs | 14 ++- .../Formatters/JsonInputFormatterTestBase.cs | 1 + .../Binders/BodyModelBinderTests.cs | 8 ++ ...bleEmptyBodyBindingMetadataProviderTest.cs | 110 ------------------ ...ataContractSerializerInputFormatterTest.cs | 9 ++ .../test/XmlSerializerInputFormatterTest.cs | 10 +- .../test/NewtonsoftJsonInputFormatterTest.cs | 8 ++ .../NewtonsoftJsonPatchInputFormatterTest.cs | 6 +- .../InputFormatterTests.cs | 42 ++++++- .../NewtonsoftJsonInputFormatterTest.cs | 19 ++- .../BodyValidationIntegrationTests.cs | 38 +++--- .../ModelBindingTestHelper.cs | 13 +++ .../Controllers/HomeController.cs | 9 ++ .../Controllers/JsonFormatterController.cs | 15 ++- 16 files changed, 271 insertions(+), 135 deletions(-) delete mode 100644 src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs index 07199c7dd0d9..4d9f68c1e0a9 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() { @@ -205,4 +222,26 @@ 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 ModelBinderAttribute(typeof(ComplexObjectModelBinder)), + new ControllerAttribute(), + new BindNeverAttribute(), + }; + 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); + } } diff --git a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs index 70832d7ec47e..422aafeadce6 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.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 System.ComponentModel; @@ -107,6 +107,7 @@ public void InferParameterBindingSources_InfersSources() var bindingInfo = parameter.BindingInfo; Assert.NotNull(bindingInfo); + Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior); Assert.Same(BindingSource.Body, bindingInfo.BindingSource); }, parameter => @@ -119,6 +120,58 @@ public void InferParameterBindingSources_InfersSources() }); } + [Fact] + public void InferParameterBindingSources_InfersSourcesFromNonNullable() + { + // 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_InfersSourcesWithDefaultValue() + { + // 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() { @@ -825,6 +878,14 @@ 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(); +#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/InputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs index 80eb5c4abc19..e37f3056eb4a 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs @@ -405,9 +405,11 @@ public void GetSupportedContentTypes_ThrowsInvalidOperationException_IfMediaType } [Theory] - [InlineData(true, true)] - [InlineData(false, false)] - public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bool allowEmptyInputValue, bool expectedIsModelSet) + [InlineData(true, true, true)] + [InlineData(true, true, false)] + [InlineData(false, false, true)] + [InlineData(false, false, false)] + public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bool allowEmptyInputValue, bool expectedIsModelSet, bool isContentLengthSet) { // Arrange var formatter = new TestFormatter(); @@ -418,7 +420,11 @@ public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bo new EmptyModelMetadataProvider().GetMetadataForType(typeof(object)), (s, e) => new StreamReader(s, e), allowEmptyInputValue); - context.HttpContext.Request.ContentLength = 0; + + if (isContentLengthSet) + { + context.HttpContext.Request.ContentLength = 0; + } // Act var result = await formatter.ReadAsync(context); 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.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs index 891ee2b88980..cefe1e11d3fc 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs @@ -252,6 +252,7 @@ public async Task BindModel_CustomFormatter_ThrowingInputFormatterException_Adds var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "text/xyz"; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -300,6 +301,7 @@ public async Task BindModel_BuiltInXmlInputFormatters_ThrowingInputFormatterExce var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -329,6 +331,7 @@ public async Task BindModel_BuiltInJsonInputFormatter_ThrowingInputFormatterExce var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -371,6 +374,7 @@ public async Task BindModel_DerivedXmlInputFormatters_AddsErrorToModelState(IInp var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -400,6 +404,7 @@ public async Task BindModel_DerivedJsonInputFormatter_AddsErrorToModelState() var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -447,6 +452,7 @@ public async Task BindModel_BuiltInInputFormatters_ThrowingNonInputFormatterExce var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("valid data!")); httpContext.Request.ContentType = contentType; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -482,6 +488,7 @@ public async Task BindModel_DerivedXmlInputFormatters_ThrowingNonInputFormatting var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("valid data!")); httpContext.Request.ContentType = contentType; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -511,6 +518,7 @@ public async Task BindModel_CustomFormatter_ThrowingNonInputFormatterException_T var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("valid data")); httpContext.Request.ContentType = "text/xyz"; + httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs deleted file mode 100644 index d03024d4852f..000000000000 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Metadata/NullableEmptyBodyBindingMetadataProviderTest.cs +++ /dev/null @@ -1,110 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -using System.Reflection; - -namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; - -#nullable enable -public class NullableEmptyBodyBindingMetadataProviderTest -{ - [Fact] - public void IsEmptyBodyAllowed_LeftAlone_WhenAlreadySet() - { - // Arrange - var provider = new NullableEmptyBodyBindingMetadataProvider(); - - var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForParameter(ParameterInfos.NullableParameter), - new ModelAttributes(Array.Empty())); - - context.BindingMetadata.IsEmptyBodyAllowed = false; - - // Act - provider.CreateBindingMetadata(context); - - // Assert - Assert.False(context.BindingMetadata.IsEmptyBodyAllowed); - } - - [Fact] - public void IsEmptyBodyAllowed_LeftAlone_WhenNotOptional() - { - // Arrange - var provider = new NullableEmptyBodyBindingMetadataProvider(); - - var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForParameter(ParameterInfos.NonNullableParameter), - new ModelAttributes(Array.Empty())); - - // Act - provider.CreateBindingMetadata(context); - - // Assert - Assert.Null(context.BindingMetadata.IsEmptyBodyAllowed); - } - - - [Fact] - public void IsEmptyBodyAllowed_IsTrue_WhenDefaultValue() - { - // Arrange - var provider = new NullableEmptyBodyBindingMetadataProvider(); - - var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForParameter(ParameterInfos.ParameterWithDefaultValue), - new ModelAttributes(Array.Empty())); - - // Act - provider.CreateBindingMetadata(context); - - // Assert - Assert.True(context.BindingMetadata.IsEmptyBodyAllowed); - - } - - [Fact] - public void IsEmptyBodyAllowed_IsTrue_WhenNullable() - { - // Arrange - var provider = new NullableEmptyBodyBindingMetadataProvider(); - - var context = new BindingMetadataProviderContext( - ModelMetadataIdentity.ForParameter(ParameterInfos.NullableParameter), - new ModelAttributes(Array.Empty())); - - // Act - provider.CreateBindingMetadata(context); - - // Assert - Assert.True(context.BindingMetadata.IsEmptyBodyAllowed); - - } - - private class ParameterInfos - { - public void Method(object param1, object? param2) - { - } - -#nullable disable - public void Method2(object param1 = null) - { - } -#nullable restore - - public static ParameterInfo NonNullableParameter - = typeof(ParameterInfos)! - .GetMethod(nameof(ParameterInfos.Method))! - .GetParameters()[0]; - - public static ParameterInfo NullableParameter - = typeof(ParameterInfos)! - .GetMethod(nameof(ParameterInfos.Method))! - .GetParameters()[1]; - - public static ParameterInfo ParameterWithDefaultValue - = typeof(ParameterInfos)! - .GetMethod(nameof(ParameterInfos.Method2))! - .GetParameters()[0]; - } -} diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs index f5cb07df3e75..c3aa7c9a268e 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs @@ -148,6 +148,7 @@ public async Task BuffersRequestBody_ByDefault() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: true); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -179,6 +180,7 @@ public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt() var httpContext = new DefaultHttpContext(); var testBufferedReadStream = new VerifyDisposeFileBufferingReadStream(new MemoryStream(contentBytes), 1024); httpContext.Request.Body = testBufferedReadStream; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -212,6 +214,7 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -247,6 +250,7 @@ public async Task BuffersRequestBody_ByDefaultUsingMvcOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -279,6 +283,7 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -338,6 +343,7 @@ public async Task SuppressInputFormatterBufferingSetToTrue_UsingMutatedOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -741,10 +747,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..960a908ef1a5 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs @@ -53,6 +53,7 @@ public async Task BuffersRequestBody_ByDefault() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: true); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -90,6 +91,7 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -126,6 +128,7 @@ public async Task BuffersRequestBody_ByDefaultUsingMvcOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -162,6 +165,7 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -204,6 +208,7 @@ public async Task SuppressInputFormatterBufferingSetToTrue_UsingMutatedOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -634,6 +639,7 @@ public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt() var httpContext = new DefaultHttpContext(); var testBufferedReadStream = new VerifyDisposeFileBufferingReadStream(new MemoryStream(contentBytes), 1024); httpContext.Request.Body = testBufferedReadStream; + httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -676,10 +682,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..be3f752996d5 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs @@ -40,6 +40,7 @@ public async Task Constructor_BuffersRequestBody_UsingDefaultOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); @@ -76,6 +77,7 @@ public async Task Constructor_SuppressInputFormatterBuffering_UsingMvcOptions_Do httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); @@ -112,6 +114,7 @@ public async Task Version_2_1_Constructor_SuppressInputFormatterBufferingSetToTr httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); @@ -414,11 +417,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(); @@ -470,6 +476,7 @@ public async Task ReadAsync_WithReadJsonWithRequestCulture_DeserializesUsingRequ httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(TypeWithPrimitives), httpContext); @@ -513,6 +520,7 @@ public async Task ReadAsync_AllowUserCodeToHandleDeserializationErrors() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(Location), httpContext); diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs index 8223e4e076fc..30bde5b0a7ac 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs @@ -39,6 +39,7 @@ public async Task Constructor_BuffersRequestBody_ByDefault() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(JsonPatchDocument), httpContext); @@ -76,6 +77,7 @@ public async Task Constructor_SuppressInputFormatterBuffering_DoesNotBufferReque httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; + httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(JsonPatchDocument), httpContext); @@ -260,10 +262,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 9f1ac17a23a5..6a6b924dfd36 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs @@ -168,11 +168,44 @@ public async Task ValidationUsesModelMetadataFromActualModelType_ForInputFormatt } [Fact] - public async Task BodyIsRequiredByDefault() + public async Task BodyIsOptionalByDefault() { // Act var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.DefaultBody)}", value: null); + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + + [Fact] + 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 BodyIsRequiredWhenRequiredAttributeIncluded() + { + // Act + var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.RequiredBody)}", value: null); + // Assert await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); var problemDetails = await response.Content.ReadFromJsonAsync(); @@ -182,6 +215,11 @@ public async Task BodyIsRequiredByDefault() { 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)); }); } @@ -193,7 +231,7 @@ public async Task BodyIsRequiredByDefaultFailsWithEmptyBody() Assert.Equal(0, content.Headers.ContentLength); // Act - var response = await Client.PostAsync($"Home/{nameof(HomeController.DefaultBody)}", content); + var response = await Client.PostAsync($"Home/{nameof(HomeController.NonNullableBody)}", content); // Assert await response.AssertStatusCodeAsync(HttpStatusCode.UnsupportedMediaType); diff --git a/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs index aec76e130eea..b5413509407c 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs @@ -40,9 +40,26 @@ public async Task JsonInputFormatter_ReturnsBadRequest_ForEmptyRequestBody( var content = new StringContent(jsonInput, Encoding.UTF8, requestContentType); // Act - var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnInput/", content); + 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); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } } diff --git a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index 20a25ffcbce2..871502fae3da 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,9 +596,12 @@ public async Task FromBodyWithInvalidPropertyData_JsonFormatterAddsModelError() } [Theory] - [InlineData(false, false)] - [InlineData(true, true)] - public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( + [InlineData(typeof(Person5), false, true)] + [InlineData(typeof(Person5), true, true)] + [InlineData(typeof(Person5WithNullableContext), true, false)] + [InlineData(typeof(Person5WithNullableContext), false, false)] + public async Task FromBodyWithEmptyBody_AddsModelErrorWhenExpected( + Type modelType, bool allowEmptyInputInBodyModelBindingSetting, bool expectedModelStateIsValid) { @@ -601,7 +613,7 @@ public async Task FromBodyWithEmptyBody_JsonFormatterAddsModelErrorWhenExpected( { BinderModelName = "CustomParameter", }, - ParameterType = typeof(Person5) + ParameterType = modelType }; var testContext = ModelBindingTestHelper.GetTestContext( @@ -609,6 +621,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,8 +633,7 @@ 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) { 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 4f9e77f73519..3fcd59fc2bca 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; @@ -41,6 +42,10 @@ public DummyClass GetDerivedDummyClass(int sampleInput) public IActionResult DefaultBody([FromBody] DummyClass dummy) => ModelState.IsValid ? Ok() : ValidationProblem(); + [HttpPost] + public IActionResult RequiredBody([FromBody][Required] DummyClass dummy) + => ModelState.IsValid ? Ok() : ValidationProblem(); + [HttpPost] public IActionResult OptionalBody([FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] DummyClass dummy) => ModelState.IsValid ? Ok() : ValidationProblem(); @@ -50,6 +55,10 @@ 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(); 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) From cecafdd4cf0389ea45e70abfc0b55eb9f96472b0 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 16:33:37 -0800 Subject: [PATCH 06/16] Updating unittests --- .../test/ModelBinding/BindingInfoTest.cs | 22 ++++++++++++++++++- .../InferParameterBindingInfoConvention.cs | 7 +++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs index 4d9f68c1e0a9..b2b1ef24e0a2 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs @@ -119,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() { @@ -229,9 +249,9 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesEmptyBodyFromModel // Arrange var attributes = new object[] { - new ModelBinderAttribute(typeof(ComplexObjectModelBinder)), new ControllerAttribute(), new BindNeverAttribute(), + new FromBodyAttribute(), }; var modelType = typeof(Guid?); var provider = new TestModelMetadataProvider(); diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index af7250269f13..da9efc190f0d 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 Resources = Microsoft.AspNetCore.Mvc.Core.Resources; @@ -157,11 +158,9 @@ private bool IsOptionalParameter(ParameterModel parameter) // does not provides an option to getMetadata from the parameter info // so, we will use the Nullability context - // Waiting for PR https://github.com/dotnet/aspnetcore/pull/39804 // No need for information from attributes on the parameter. Just use its type. - // var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); - // return metadata.NullabilityState != NullabilityState.NotNull; - return false; + var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); + return metadata.NullabilityState != NullabilityState.NotNull; } } } From c4a90657c4421ef9cc14b1005d9d0ee02a25724a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 16:34:57 -0800 Subject: [PATCH 07/16] Fix formatting --- src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index ff060103fd59..5c1e08982608 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -250,7 +250,7 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) // as Allow when the ModelMetadata.IsRequired is false or HasDefaultValue // https://github.com/dotnet/aspnetcore/issues/39754 if (EmptyBodyBehavior == EmptyBodyBehavior.Default && - BindingSource == BindingSource.Body && + BindingSource == BindingSource.Body && (!modelMetadata.IsRequired || modelMetadata.HasDefaultValue)) { isBindingInfoPresent = true; From 04c85149f24f994cb51e3268774e9830c405c4c5 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 16:39:03 -0800 Subject: [PATCH 08/16] nit --- src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs | 2 +- .../Mvc.Core/src/ModelBinding/Metadata/BindingMetadata.cs | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index 5c1e08982608..db4cfb727582 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -246,7 +246,7 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) PropertyFilterProvider = modelMetadata.PropertyFilterProvider; } - // If the EmptyBody behavior is not configured will be infer + // If the EmptyBody behavior is not configured will be inferred // as Allow when the ModelMetadata.IsRequired is false or HasDefaultValue // https://github.com/dotnet/aspnetcore/issues/39754 if (EmptyBodyBehavior == EmptyBodyBehavior.Default && diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/BindingMetadata.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/BindingMetadata.cs index 99705168bbbc..e3b1fab3f53a 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/BindingMetadata.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Metadata/BindingMetadata.cs @@ -77,11 +77,6 @@ public Type? BinderType /// public bool? IsReadOnly { get; set; } - /// - /// Gets or sets the value which decides if empty bodies are treated as valid inputs. - /// - internal bool? IsEmptyBodyAllowed { get; set; } - /// /// Gets the instance. See /// . From afb8063c984be2d2765417baa9a5a432a3cc629a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 1 Feb 2022 22:37:20 -0800 Subject: [PATCH 09/16] Remove content-length null check --- src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs | 2 +- .../Mvc.Core/test/Formatters/InputFormatterTest.cs | 14 ++++---------- .../XmlDataContractSerializerInputFormatterTest.cs | 6 ------ .../test/XmlSerializerInputFormatterTest.cs | 6 ------ .../test/NewtonsoftJsonInputFormatterTest.cs | 5 ----- .../test/NewtonsoftJsonPatchInputFormatterTest.cs | 2 -- 6 files changed, 5 insertions(+), 30 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs index 7257d6df6c32..be0cbedb1b6f 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs @@ -101,7 +101,7 @@ public virtual Task ReadAsync(InputFormatterContext contex var hasBody = context.HttpContext.Features.Get()?.CanHaveBody; // In case the feature is not registered - hasBody ??= context.HttpContext.Request.ContentLength is not null && context.HttpContext.Request.ContentLength != 0; + hasBody ??= context.HttpContext.Request.ContentLength != 0; if (hasBody == false) { diff --git a/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs b/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs index e37f3056eb4a..80eb5c4abc19 100644 --- a/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs +++ b/src/Mvc/Mvc.Core/test/Formatters/InputFormatterTest.cs @@ -405,11 +405,9 @@ public void GetSupportedContentTypes_ThrowsInvalidOperationException_IfMediaType } [Theory] - [InlineData(true, true, true)] - [InlineData(true, true, false)] - [InlineData(false, false, true)] - [InlineData(false, false, false)] - public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bool allowEmptyInputValue, bool expectedIsModelSet, bool isContentLengthSet) + [InlineData(true, true)] + [InlineData(false, false)] + public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bool allowEmptyInputValue, bool expectedIsModelSet) { // Arrange var formatter = new TestFormatter(); @@ -420,11 +418,7 @@ public async Task ReadAsync_WithEmptyRequest_ReturnsNoValueResultWhenExpected(bo new EmptyModelMetadataProvider().GetMetadataForType(typeof(object)), (s, e) => new StreamReader(s, e), allowEmptyInputValue); - - if (isContentLengthSet) - { - context.HttpContext.Request.ContentLength = 0; - } + context.HttpContext.Request.ContentLength = 0; // Act var result = await formatter.ReadAsync(context); diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs index d470fd8f7937..a4ebddf109e4 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs @@ -148,7 +148,6 @@ public async Task BuffersRequestBody_ByDefault() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: true); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -180,7 +179,6 @@ public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt() var httpContext = new DefaultHttpContext(); var testBufferedReadStream = new VerifyDisposeFileBufferingReadStream(new MemoryStream(contentBytes), 1024); httpContext.Request.Body = testBufferedReadStream; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -214,7 +212,6 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -250,7 +247,6 @@ public async Task BuffersRequestBody_ByDefaultUsingMvcOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -283,7 +279,6 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -343,7 +338,6 @@ public async Task SuppressInputFormatterBufferingSetToTrue_UsingMutatedOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act diff --git a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs index 960a908ef1a5..050b6fbfd956 100644 --- a/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs +++ b/src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs @@ -53,7 +53,6 @@ public async Task BuffersRequestBody_ByDefault() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: true); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -91,7 +90,6 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -128,7 +126,6 @@ public async Task BuffersRequestBody_ByDefaultUsingMvcOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -165,7 +162,6 @@ public async Task SuppressInputFormatterBufferingSetToTrue_DoesNotBufferRequestB httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -208,7 +204,6 @@ public async Task SuppressInputFormatterBufferingSetToTrue_UsingMutatedOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act @@ -639,7 +634,6 @@ public async Task ReadAsync_DoesNotDisposeBufferedStreamIfItDidNotCreateIt() var httpContext = new DefaultHttpContext(); var testBufferedReadStream = new VerifyDisposeFileBufferingReadStream(new MemoryStream(contentBytes), 1024); httpContext.Request.Body = testBufferedReadStream; - httpContext.Request.ContentLength = contentBytes.Length; var context = GetInputFormatterContext(httpContext, typeof(TestLevelOne)); // Act diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs index be3f752996d5..8d8d9cd83d2c 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs @@ -40,7 +40,6 @@ public async Task Constructor_BuffersRequestBody_UsingDefaultOptions() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); @@ -77,7 +76,6 @@ public async Task Constructor_SuppressInputFormatterBuffering_UsingMvcOptions_Do httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); @@ -114,7 +112,6 @@ public async Task Version_2_1_Constructor_SuppressInputFormatterBufferingSetToTr httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(User), httpContext); @@ -476,7 +473,6 @@ public async Task ReadAsync_WithReadJsonWithRequestCulture_DeserializesUsingRequ httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(TypeWithPrimitives), httpContext); @@ -520,7 +516,6 @@ public async Task ReadAsync_AllowUserCodeToHandleDeserializationErrors() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(Location), httpContext); diff --git a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs index 30bde5b0a7ac..499ab6ff8dfb 100644 --- a/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs +++ b/src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonPatchInputFormatterTest.cs @@ -39,7 +39,6 @@ public async Task Constructor_BuffersRequestBody_ByDefault() httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes, allowSyncReads: false); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(JsonPatchDocument), httpContext); @@ -77,7 +76,6 @@ public async Task Constructor_SuppressInputFormatterBuffering_DoesNotBufferReque httpContext.Features.Set(new TestResponseFeature()); httpContext.Request.Body = new NonSeekableReadStream(contentBytes); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = contentBytes.Length; var formatterContext = CreateInputFormatterContext(typeof(JsonPatchDocument), httpContext); From 4849d7f61c7ca4d15a3383fc79689a3547203cfa Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 2 Feb 2022 09:50:41 -0800 Subject: [PATCH 10/16] Remove content-length null check --- .../test/ModelBinding/Binders/BodyModelBinderTests.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs index cefe1e11d3fc..891ee2b88980 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Binders/BodyModelBinderTests.cs @@ -252,7 +252,6 @@ public async Task BindModel_CustomFormatter_ThrowingInputFormatterException_Adds var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "text/xyz"; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -301,7 +300,6 @@ public async Task BindModel_BuiltInXmlInputFormatters_ThrowingInputFormatterExce var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -331,7 +329,6 @@ public async Task BindModel_BuiltInJsonInputFormatter_ThrowingInputFormatterExce var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -374,7 +371,6 @@ public async Task BindModel_DerivedXmlInputFormatters_AddsErrorToModelState(IInp var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/xml"; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -404,7 +400,6 @@ public async Task BindModel_DerivedJsonInputFormatter_AddsErrorToModelState() var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("Bad data!")); httpContext.Request.ContentType = "application/json"; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -452,7 +447,6 @@ public async Task BindModel_BuiltInInputFormatters_ThrowingNonInputFormatterExce var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("valid data!")); httpContext.Request.ContentType = contentType; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -488,7 +482,6 @@ public async Task BindModel_DerivedXmlInputFormatters_ThrowingNonInputFormatting var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("valid data!")); httpContext.Request.ContentType = contentType; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); @@ -518,7 +511,6 @@ public async Task BindModel_CustomFormatter_ThrowingNonInputFormatterException_T var httpContext = new DefaultHttpContext(); httpContext.Request.Body = new MemoryStream(Encoding.UTF8.GetBytes("valid data")); httpContext.Request.ContentType = "text/xyz"; - httpContext.Request.ContentLength = httpContext.Request.Body.Length; var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType().BindingDetails(d => d.BindingSource = BindingSource.Body); From fc19f6e3a9183325530bedf1810c94d8f4a81fc1 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 10 Feb 2022 23:09:07 +0000 Subject: [PATCH 11/16] Changing to use the nullabilityState --- .../src/ModelBinding/BindingInfo.cs | 5 +++-- .../InferParameterBindingInfoConvention.cs | 18 +++--------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index db4cfb727582..74d86bfcc24b 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; @@ -247,11 +248,11 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) } // If the EmptyBody behavior is not configured will be inferred - // as Allow when the ModelMetadata.IsRequired is false or HasDefaultValue + // as Allow when the NullablityState == NullablityStateNull or HasDefaultValue // https://github.com/dotnet/aspnetcore/issues/39754 if (EmptyBodyBehavior == EmptyBodyBehavior.Default && BindingSource == BindingSource.Body && - (!modelMetadata.IsRequired || modelMetadata.HasDefaultValue)) + (modelMetadata.NullabilityState == NullablityState.Null || modelMetadata.HasDefaultValue)) { isBindingInfoPresent = true; EmptyBodyBehavior = EmptyBodyBehavior.Allow; diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index da9efc190f0d..453e3b6fcd58 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -147,20 +147,8 @@ private bool IsOptionalParameter(ParameterModel parameter) return true; } - if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider) - { - var metadata = modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo); - return !metadata.IsRequired; - } - 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 use the Nullability context - - // No need for information from attributes on the parameter. Just use its type. - var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); - return metadata.NullabilityState != NullabilityState.NotNull; - } + // No need for information from attributes on the parameter. Just use its type. + var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); + return metadata.NullabilityState == NullabilityState.Null; } } From 81be48ac48728e02b8fcfe26a3cd374ced1f8030 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 11 Feb 2022 14:53:21 -0800 Subject: [PATCH 12/16] Fix build and updating unit tests --- .../src/ModelBinding/BindingInfo.cs | 2 +- .../test/ModelBinding/BindingInfoTest.cs | 22 ++++++++++++ .../InferParameterBindingInfoConvention.cs | 15 ++++++-- .../Mvc.Core/src/Formatters/InputFormatter.cs | 2 +- ...InferParameterBindingInfoConventionTest.cs | 35 +++++++++++++++++-- .../InputFormatterTests.cs | 14 ++++++-- .../NewtonsoftJsonInputFormatterTest.cs | 2 +- .../BodyValidationIntegrationTests.cs | 2 +- 8 files changed, 81 insertions(+), 13 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index 74d86bfcc24b..f716a319b249 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -252,7 +252,7 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) // https://github.com/dotnet/aspnetcore/issues/39754 if (EmptyBodyBehavior == EmptyBodyBehavior.Default && BindingSource == BindingSource.Body && - (modelMetadata.NullabilityState == NullablityState.Null || modelMetadata.HasDefaultValue)) + (modelMetadata.NullabilityState == NullabilityState.Nullable || modelMetadata.IsNullableValueType || modelMetadata.HasDefaultValue)) { isBindingInfoPresent = true; EmptyBodyBehavior = EmptyBodyBehavior.Allow; diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs index b2b1ef24e0a2..2d959316751f 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs @@ -264,4 +264,26 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesEmptyBodyFromModel 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 407aed4b7b8c..3ecedf1757e5 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -170,8 +170,17 @@ private bool IsOptionalParameter(ParameterModel parameter) return true; } - // No need for information from attributes on the parameter. Just use its type. - var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); - return metadata.NullabilityState == NullabilityState.Null; + 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 the parameter as not optional + return false; + } } } diff --git a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs index be0cbedb1b6f..3e872133e6d1 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs @@ -103,7 +103,7 @@ public virtual Task ReadAsync(InputFormatterContext contex // In case the feature is not registered hasBody ??= context.HttpContext.Request.ContentLength != 0; - if (hasBody == false) + if (hasBody 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 dd878cde4477..68513623fffd 100644 --- a/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs @@ -109,7 +109,7 @@ public void InferParameterBindingSources_InfersSources() var bindingInfo = parameter.BindingInfo; Assert.NotNull(bindingInfo); - Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior); + Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior); Assert.Same(BindingSource.Body, bindingInfo.BindingSource); }, parameter => @@ -123,7 +123,7 @@ public void InferParameterBindingSources_InfersSources() } [Fact] - public void InferParameterBindingSources_InfersSourcesFromNonNullable() + public void InferParameterBindingSources_InfersSourcesFromRequiredComplexType() { // Arrange var actionName = nameof(ParameterBindingController.RequiredComplexType); @@ -149,7 +149,33 @@ public void InferParameterBindingSources_InfersSourcesFromNonNullable() } [Fact] - public void InferParameterBindingSources_InfersSourcesWithDefaultValue() + 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); @@ -903,6 +929,9 @@ private class ParameterBindingController #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")] diff --git a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs index 6a6b924dfd36..257a403bff06 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs @@ -168,13 +168,21 @@ public async Task ValidationUsesModelMetadataFromActualModelType_ForInputFormatt } [Fact] - public async Task BodyIsOptionalByDefault() + public async Task BodyIsRequiredByDefault() { // Act var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.DefaultBody)}", value: null); // Assert - await response.AssertStatusCodeAsync(HttpStatusCode.OK); + 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)); + }); } [Fact] @@ -224,7 +232,7 @@ public async Task BodyIsRequiredWhenRequiredAttributeIncluded() } [Fact] - public async Task BodyIsRequiredByDefaultFailsWithEmptyBody() + public async Task BodyIsRequiredByDefaultFailsWithContentLengthZero() { var content = new ByteArrayContent(Array.Empty()); Assert.Null(content.Headers.ContentType); diff --git a/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs b/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs index b5413509407c..9e8ed6e254d7 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/NewtonsoftJsonInputFormatterTest.cs @@ -60,6 +60,6 @@ public async Task JsonInputFormatter_ReturnsBadRequest_ForEmptyRequestBody_WhenN var response = await Client.PostAsync("http://localhost/JsonFormatter/ReturnInput/", content); // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); } } diff --git a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index 871502fae3da..09d1b8763850 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -596,7 +596,7 @@ public async Task FromBodyWithInvalidPropertyData_JsonFormatterAddsModelError() } [Theory] - [InlineData(typeof(Person5), false, true)] + [InlineData(typeof(Person5), false, false)] [InlineData(typeof(Person5), true, true)] [InlineData(typeof(Person5WithNullableContext), true, false)] [InlineData(typeof(Person5WithNullableContext), false, false)] From e0360f8a054ceedde5d2e9e1c77cd70b4dd99d33 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Fri, 11 Feb 2022 15:20:03 -0800 Subject: [PATCH 13/16] fixes --- .../InferParameterBindingInfoConvention.cs | 2 +- .../InputFormatterTests.cs | 25 +------------------ .../Controllers/HomeController.cs | 4 --- 3 files changed, 2 insertions(+), 29 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs index 3ecedf1757e5..23b302191651 100644 --- a/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -179,7 +179,7 @@ private bool IsOptionalParameter(ParameterModel parameter) { // 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 the parameter as not optional + // so, we will NOT treat the parameter as optional. return false; } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs index 257a403bff06..bc40e1afa013 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/InputFormatterTests.cs @@ -208,29 +208,6 @@ public async Task BodyIsRequiredByDefault_WhenNullableContextEnabled() }); } - [Fact] - public async Task BodyIsRequiredWhenRequiredAttributeIncluded() - { - // Act - var response = await Client.PostAsJsonAsync($"Home/{nameof(HomeController.RequiredBody)}", 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() { @@ -239,7 +216,7 @@ public async Task BodyIsRequiredByDefaultFailsWithContentLengthZero() Assert.Equal(0, content.Headers.ContentLength); // Act - var response = await Client.PostAsync($"Home/{nameof(HomeController.NonNullableBody)}", content); + var response = await Client.PostAsync($"Home/{nameof(HomeController.DefaultBody)}", content); // Assert await response.AssertStatusCodeAsync(HttpStatusCode.UnsupportedMediaType); diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs index 3fcd59fc2bca..4a27b56a6b82 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/HomeController.cs @@ -42,10 +42,6 @@ public DummyClass GetDerivedDummyClass(int sampleInput) public IActionResult DefaultBody([FromBody] DummyClass dummy) => ModelState.IsValid ? Ok() : ValidationProblem(); - [HttpPost] - public IActionResult RequiredBody([FromBody][Required] DummyClass dummy) - => ModelState.IsValid ? Ok() : ValidationProblem(); - [HttpPost] public IActionResult OptionalBody([FromBody(EmptyBodyBehavior = EmptyBodyBehavior.Allow)] DummyClass dummy) => ModelState.IsValid ? Ok() : ValidationProblem(); From 396ba687005bd646fc1f722d1c17a0da8e8af8cd Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 17 Feb 2022 10:54:54 -0800 Subject: [PATCH 14/16] Apply suggestions from code review Co-authored-by: Pranav K --- src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs | 4 ++-- src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs index f716a319b249..b5dded52a176 100644 --- a/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs +++ b/src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs @@ -251,8 +251,8 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata) // 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)) + BindingSource == BindingSource.Body && + (modelMetadata.NullabilityState == NullabilityState.Nullable || modelMetadata.IsNullableValueType || modelMetadata.HasDefaultValue)) { isBindingInfoPresent = true; EmptyBodyBehavior = EmptyBodyBehavior.Allow; diff --git a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs index 2d959316751f..a9680cd9e78c 100644 --- a/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs +++ b/src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs @@ -50,7 +50,7 @@ public void GetBindingInfo_ReadsEmptyBodyBehavior() // Arrange var attributes = new object[] { - new FromBodyAttribute { EmptyBodyBehavior = EmptyBodyBehavior.Allow }, + new FromBodyAttribute { EmptyBodyBehavior = EmptyBodyBehavior.Allow }, }; // Act From cbdb5955c1f46386ce098b3b3d8a38862e508a67 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Thu, 17 Feb 2022 13:49:26 -0800 Subject: [PATCH 15/16] Spliting FromBodyWithEmptyBody_AddsModelErrorWhenExpected --- .../BodyValidationIntegrationTests.cs | 72 +++++++++++++------ 1 file changed, 49 insertions(+), 23 deletions(-) diff --git a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index 09d1b8763850..b4b0ac899f2d 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -596,14 +596,12 @@ public async Task FromBodyWithInvalidPropertyData_JsonFormatterAddsModelError() } [Theory] - [InlineData(typeof(Person5), false, false)] - [InlineData(typeof(Person5), true, true)] - [InlineData(typeof(Person5WithNullableContext), true, false)] - [InlineData(typeof(Person5WithNullableContext), false, false)] - public async Task FromBodyWithEmptyBody_AddsModelErrorWhenExpected( + [InlineData(typeof(Person5), false)] + [InlineData(typeof(Person5WithNullableContext), true)] + [InlineData(typeof(Person5WithNullableContext), false)] + public async Task FromBodyWithEmptyBody_ModelStateIsInvalid_HasModelErrors( Type modelType, - bool allowEmptyInputInBodyModelBindingSetting, - bool expectedModelStateIsValid) + bool allowEmptyInputInBodyModelBindingSetting) { // Arrange var parameter = new ParameterDescriptor @@ -635,23 +633,51 @@ public async Task FromBodyWithEmptyBody_AddsModelErrorWhenExpected( Assert.True(modelBindingResult.IsModelSet); 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 From b058185db7de88838e9484f51164b6bf060a1099 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 23 Feb 2022 15:14:38 -0800 Subject: [PATCH 16/16] Change variable name to canHaveBody --- src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs index 3e872133e6d1..0edead1188b2 100644 --- a/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs +++ b/src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs @@ -99,11 +99,11 @@ public virtual Task ReadAsync(InputFormatterContext contex throw new ArgumentNullException(nameof(context)); } - var hasBody = context.HttpContext.Features.Get()?.CanHaveBody; + var canHaveBody = context.HttpContext.Features.Get()?.CanHaveBody; // In case the feature is not registered - hasBody ??= context.HttpContext.Request.ContentLength != 0; + canHaveBody ??= context.HttpContext.Request.ContentLength != 0; - if (hasBody is false) + if (canHaveBody is false) { if (context.TreatEmptyInputAsDefaultValue) {