Skip to content

Commit c72f78e

Browse files
brunolins16pranavkm
brunolins16
and
pranavkm
authored
Allowing empty FromBody parameters input based on nullability (#39917)
* Adding NullableEmptyBodyProvider and inferring EmptyBehavior=Allow * Adding/Updating tests * Allowing empty input * Fix how to check for empty input * Updating tests * Updating unittests * Fix formatting * nit * Remove content-length null check * Remove content-length null check * Changing to use the nullabilityState * Fix build and updating unit tests * fixes * Apply suggestions from code review Co-authored-by: Pranav K <prkrishn@hotmail.com> * Spliting FromBodyWithEmptyBody_AddsModelErrorWhenExpected * Change variable name to canHaveBody Co-authored-by: Pranav K <prkrishn@hotmail.com>
1 parent 82eb680 commit c72f78e

File tree

17 files changed

+439
-40
lines changed

17 files changed

+439
-40
lines changed

‎src/Mvc/Mvc.Abstractions/src/ModelBinding/BindingInfo.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Linq;
5+
using System.Reflection;
56
using Microsoft.AspNetCore.Mvc.Abstractions;
67

78
namespace Microsoft.AspNetCore.Mvc.ModelBinding;
@@ -246,8 +247,16 @@ public bool TryApplyBindingInfo(ModelMetadata modelMetadata)
246247
PropertyFilterProvider = modelMetadata.PropertyFilterProvider;
247248
}
248249

249-
// There isn't a ModelMetadata feature to configure AllowEmptyInputInBodyModelBinding,
250-
// so nothing to infer from it.
250+
// If the EmptyBody behavior is not configured will be inferred
251+
// as Allow when the NullablityState == NullablityStateNull or HasDefaultValue
252+
// https://github.com/dotnet/aspnetcore/issues/39754
253+
if (EmptyBodyBehavior == EmptyBodyBehavior.Default &&
254+
BindingSource == BindingSource.Body &&
255+
(modelMetadata.NullabilityState == NullabilityState.Nullable || modelMetadata.IsNullableValueType || modelMetadata.HasDefaultValue))
256+
{
257+
isBindingInfoPresent = true;
258+
EmptyBodyBehavior = EmptyBodyBehavior.Allow;
259+
}
251260

252261
return isBindingInfoPresent;
253262
}

‎src/Mvc/Mvc.Abstractions/src/ModelBinding/ModelMetadata.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -509,6 +509,12 @@ internal IReadOnlyDictionary<ModelMetadata, ModelMetadata> BoundConstructorPrope
509509
/// </summary>
510510
internal virtual string? ValidationModelName { get; }
511511

512+
/// <summary>
513+
/// Gets the value that indicates if the parameter has a default value set.
514+
/// This is only available when <see cref="MetadataKind"/> is <see cref="ModelMetadataKind.Parameter"/> otherwise it will be false.
515+
/// </summary>
516+
internal bool HasDefaultValue { get; private set; }
517+
512518
/// <summary>
513519
/// Throws if the ModelMetadata is for a record type with validation on properties.
514520
/// </summary>
@@ -620,6 +626,7 @@ private void InitializeTypeInformation()
620626
IsNullableValueType = Nullable.GetUnderlyingType(ModelType) != null;
621627
IsReferenceOrNullableType = !ModelType.IsValueType || IsNullableValueType;
622628
UnderlyingOrModelType = Nullable.GetUnderlyingType(ModelType) ?? ModelType;
629+
HasDefaultValue = MetadataKind == ModelMetadataKind.Parameter && Identity.ParameterInfo!.HasDefaultValue;
623630

624631
var collectionType = ClosedGenericMatcher.ExtractGenericInterface(ModelType, typeof(ICollection<>));
625632
IsCollectionType = collectionType != null;

‎src/Mvc/Mvc.Abstractions/test/ModelBinding/BindingInfoTest.cs‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Licensed to the .NET Foundation under one or more agreements.
1+
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
@@ -44,6 +44,23 @@ public void GetBindingInfo_ReadsPropertyPredicateProvider()
4444
Assert.Same(bindAttribute, bindingInfo.PropertyFilterProvider);
4545
}
4646

47+
[Fact]
48+
public void GetBindingInfo_ReadsEmptyBodyBehavior()
49+
{
50+
// Arrange
51+
var attributes = new object[]
52+
{
53+
new FromBodyAttribute { EmptyBodyBehavior = EmptyBodyBehavior.Allow },
54+
};
55+
56+
// Act
57+
var bindingInfo = BindingInfo.GetBindingInfo(attributes);
58+
59+
// Assert
60+
Assert.NotNull(bindingInfo);
61+
Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior);
62+
}
63+
4764
[Fact]
4865
public void GetBindingInfo_ReadsRequestPredicateProvider()
4966
{
@@ -102,6 +119,26 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesValuesFromBindingI
102119
Assert.Same("Test", bindingInfo.BinderModelName);
103120
}
104121

122+
[Fact]
123+
public void GetBindingInfo_WithAttributesAndModelMetadata_UsesEmptyBodyBehaviorFromBindingInfo_IfAttributesPresent()
124+
{
125+
// Arrange
126+
var attributes = new object[]
127+
{
128+
new FromBodyAttribute() { EmptyBodyBehavior = EmptyBodyBehavior.Disallow }
129+
};
130+
var modelType = typeof(Guid?);
131+
var provider = new TestModelMetadataProvider();
132+
var modelMetadata = provider.GetMetadataForType(modelType);
133+
134+
// Act
135+
var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata);
136+
137+
// Assert
138+
Assert.NotNull(bindingInfo);
139+
Assert.Equal(EmptyBodyBehavior.Disallow, bindingInfo.EmptyBodyBehavior);
140+
}
141+
105142
[Fact]
106143
public void GetBindingInfo_WithAttributesAndModelMetadata_UsesBinderNameFromModelMetadata_WhenNotFoundViaAttributes()
107144
{
@@ -205,4 +242,48 @@ public void GetBindingInfo_WithAttributesAndModelMetadata_UsesPropertyPredicateP
205242
Assert.NotNull(bindingInfo);
206243
Assert.Same(propertyFilterProvider, bindingInfo.PropertyFilterProvider);
207244
}
245+
246+
[Fact]
247+
public void GetBindingInfo_WithAttributesAndModelMetadata_UsesEmptyBodyFromModelMetadata_WhenNotFoundViaAttributes()
248+
{
249+
// Arrange
250+
var attributes = new object[]
251+
{
252+
new ControllerAttribute(),
253+
new BindNeverAttribute(),
254+
new FromBodyAttribute(),
255+
};
256+
var modelType = typeof(Guid?);
257+
var provider = new TestModelMetadataProvider();
258+
var modelMetadata = provider.GetMetadataForType(modelType);
259+
260+
// Act
261+
var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata);
262+
263+
// Assert
264+
Assert.NotNull(bindingInfo);
265+
Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior);
266+
}
267+
268+
[Fact]
269+
public void GetBindingInfo_WithAttributesAndModelMetadata_PreserveEmptyBodyDefault_WhenNotNullable()
270+
{
271+
// Arrange
272+
var attributes = new object[]
273+
{
274+
new ControllerAttribute(),
275+
new BindNeverAttribute(),
276+
new FromBodyAttribute(),
277+
};
278+
var modelType = typeof(Guid);
279+
var provider = new TestModelMetadataProvider();
280+
var modelMetadata = provider.GetMetadataForType(modelType);
281+
282+
// Act
283+
var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata);
284+
285+
// Assert
286+
Assert.NotNull(bindingInfo);
287+
Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior);
288+
}
208289
}

‎src/Mvc/Mvc.Core/src/ApplicationModels/InferParameterBindingInfoConvention.cs‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Linq;
5+
using System.Reflection;
56
using Microsoft.AspNetCore.Mvc.ModelBinding;
67
using Microsoft.AspNetCore.Routing.Template;
78
using Microsoft.Extensions.DependencyInjection;
@@ -106,6 +107,12 @@ internal void InferParameterBindingSources(ActionModel action)
106107
message += Environment.NewLine + parameters;
107108
throw new InvalidOperationException(message);
108109
}
110+
else if (fromBodyParameters.Count == 1 &&
111+
fromBodyParameters[0].BindingInfo!.EmptyBodyBehavior == EmptyBodyBehavior.Default &&
112+
IsOptionalParameter(fromBodyParameters[0]))
113+
{
114+
fromBodyParameters[0].BindingInfo!.EmptyBodyBehavior = EmptyBodyBehavior.Allow;
115+
}
109116
}
110117

111118
// Internal for unit testing.
@@ -155,4 +162,25 @@ private bool IsComplexTypeParameter(ParameterModel parameter)
155162

156163
return metadata.IsComplexType;
157164
}
165+
166+
private bool IsOptionalParameter(ParameterModel parameter)
167+
{
168+
if (parameter.ParameterInfo.HasDefaultValue)
169+
{
170+
return true;
171+
}
172+
173+
if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider)
174+
{
175+
var metadata = modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo);
176+
return metadata.NullabilityState == NullabilityState.Nullable || metadata.IsNullableValueType;
177+
}
178+
else
179+
{
180+
// Cannot be determine if the parameter is optional since the provider
181+
// does not provides an option to getMetadata from the parameter info
182+
// so, we will NOT treat the parameter as optional.
183+
return false;
184+
}
185+
}
158186
}

‎src/Mvc/Mvc.Core/src/Formatters/InputFormatter.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using Microsoft.AspNetCore.Http.Features;
45
using Microsoft.AspNetCore.Mvc.ApiExplorer;
56
using Microsoft.AspNetCore.Mvc.Core;
67

@@ -98,8 +99,11 @@ public virtual Task<InputFormatterResult> ReadAsync(InputFormatterContext contex
9899
throw new ArgumentNullException(nameof(context));
99100
}
100101

101-
var request = context.HttpContext.Request;
102-
if (request.ContentLength == 0)
102+
var canHaveBody = context.HttpContext.Features.Get<IHttpRequestBodyDetectionFeature>()?.CanHaveBody;
103+
// In case the feature is not registered
104+
canHaveBody ??= context.HttpContext.Request.ContentLength != 0;
105+
106+
if (canHaveBody is false)
103107
{
104108
if (context.TreatEmptyInputAsDefaultValue)
105109
{

‎src/Mvc/Mvc.Core/test/ApplicationModels/InferParameterBindingInfoConventionTest.cs‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ public void InferParameterBindingSources_InfersSources()
109109

110110
var bindingInfo = parameter.BindingInfo;
111111
Assert.NotNull(bindingInfo);
112+
Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior);
112113
Assert.Same(BindingSource.Body, bindingInfo.BindingSource);
113114
},
114115
parameter =>
@@ -121,6 +122,84 @@ public void InferParameterBindingSources_InfersSources()
121122
});
122123
}
123124

125+
[Fact]
126+
public void InferParameterBindingSources_InfersSourcesFromRequiredComplexType()
127+
{
128+
// Arrange
129+
var actionName = nameof(ParameterBindingController.RequiredComplexType);
130+
var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
131+
var convention = GetConvention(modelMetadataProvider);
132+
var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider);
133+
134+
// Act
135+
convention.InferParameterBindingSources(action);
136+
137+
// Assert
138+
Assert.Collection(
139+
action.Parameters,
140+
parameter =>
141+
{
142+
Assert.Equal("model", parameter.Name);
143+
144+
var bindingInfo = parameter.BindingInfo;
145+
Assert.NotNull(bindingInfo);
146+
Assert.Equal(EmptyBodyBehavior.Default, bindingInfo.EmptyBodyBehavior);
147+
Assert.Same(BindingSource.Body, bindingInfo.BindingSource);
148+
});
149+
}
150+
151+
[Fact]
152+
public void InferParameterBindingSources_InfersSourcesFromNullableComplexType()
153+
{
154+
// Arrange
155+
var actionName = nameof(ParameterBindingController.NullableComplexType);
156+
var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
157+
var convention = GetConvention(modelMetadataProvider);
158+
var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider);
159+
160+
// Act
161+
convention.InferParameterBindingSources(action);
162+
163+
// Assert
164+
Assert.Collection(
165+
action.Parameters,
166+
parameter =>
167+
{
168+
Assert.Equal("model", parameter.Name);
169+
170+
var bindingInfo = parameter.BindingInfo;
171+
Assert.NotNull(bindingInfo);
172+
Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior);
173+
Assert.Same(BindingSource.Body, bindingInfo.BindingSource);
174+
});
175+
}
176+
177+
[Fact]
178+
public void InferParameterBindingSources_InfersSourcesFromComplexTypeWithDefaultValue()
179+
{
180+
// Arrange
181+
var actionName = nameof(ParameterBindingController.ComplexTypeWithDefaultValue);
182+
var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
183+
var convention = GetConvention(modelMetadataProvider);
184+
var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider);
185+
186+
// Act
187+
convention.InferParameterBindingSources(action);
188+
189+
// Assert
190+
Assert.Collection(
191+
action.Parameters,
192+
parameter =>
193+
{
194+
Assert.Equal("model", parameter.Name);
195+
196+
var bindingInfo = parameter.BindingInfo;
197+
Assert.NotNull(bindingInfo);
198+
Assert.Equal(EmptyBodyBehavior.Allow, bindingInfo.EmptyBodyBehavior);
199+
Assert.Same(BindingSource.Body, bindingInfo.BindingSource);
200+
});
201+
}
202+
124203
[Fact]
125204
public void Apply_PreservesBindingInfo_WhenInferringFor_ParameterWithModelBinder_AndExplicitName()
126205
{
@@ -847,6 +926,17 @@ private class ParameterBindingController
847926
[HttpPut("cancellation")]
848927
public IActionResult ComplexTypeModelWithCancellationToken(TestModel model, CancellationToken cancellationToken) => null;
849928

929+
#nullable enable
930+
[HttpPut("parameter-notnull")]
931+
public IActionResult RequiredComplexType(TestModel model) => new OkResult();
932+
933+
[HttpPut("parameter-null")]
934+
public IActionResult NullableComplexType(TestModel? model) => new OkResult();
935+
#nullable restore
936+
937+
[HttpPut("parameter-with-default-value")]
938+
public IActionResult ComplexTypeWithDefaultValue(TestModel model = null) => null;
939+
850940
[HttpGet("parameter-with-model-binder-attribute")]
851941
public IActionResult ModelBinderAttribute([ModelBinder(Name = "top")] int value) => null;
852942

‎src/Mvc/Mvc.Core/test/Formatters/JsonInputFormatterTestBase.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,7 @@ protected static HttpContext GetHttpContext(
640640
var httpContext = new DefaultHttpContext();
641641
httpContext.Request.Body = requestStream;
642642
httpContext.Request.ContentType = contentType;
643+
httpContext.Request.ContentLength = requestStream.Length;
643644

644645
return httpContext;
645646
}

‎src/Mvc/Mvc.Formatters.Xml/test/XmlDataContractSerializerInputFormatterTest.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,10 +740,13 @@ private static HttpContext GetHttpContext(
740740
request.SetupGet(r => r.Headers).Returns(headers.Object);
741741
request.SetupGet(f => f.Body).Returns(new MemoryStream(contentBytes));
742742
request.SetupGet(f => f.ContentType).Returns(contentType);
743+
request.SetupGet(f => f.ContentLength).Returns(contentBytes.Length);
743744

744745
var httpContext = new Mock<HttpContext>();
746+
var features = new Mock<IFeatureCollection>();
745747
httpContext.SetupGet(c => c.Request).Returns(request.Object);
746748
httpContext.SetupGet(c => c.Request).Returns(request.Object);
749+
httpContext.SetupGet(c => c.Features).Returns(features.Object);
747750
return httpContext.Object;
748751
}
749752

‎src/Mvc/Mvc.Formatters.Xml/test/XmlSerializerInputFormatterTest.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -676,10 +676,12 @@ private static HttpContext GetHttpContext(
676676
request.SetupGet(r => r.Headers).Returns(headers.Object);
677677
request.SetupGet(f => f.Body).Returns(new MemoryStream(contentBytes));
678678
request.SetupGet(f => f.ContentType).Returns(contentType);
679+
request.SetupGet(f => f.ContentLength).Returns(contentBytes.Length);
679680

680681
var httpContext = new Mock<HttpContext>();
682+
var features = new Mock<IFeatureCollection>();
681683
httpContext.SetupGet(c => c.Request).Returns(request.Object);
682-
httpContext.SetupGet(c => c.Request).Returns(request.Object);
684+
httpContext.SetupGet(c => c.Features).Returns(features.Object);
683685
return httpContext.Object;
684686
}
685687

‎src/Mvc/Mvc.NewtonsoftJson/test/NewtonsoftJsonInputFormatterTest.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -414,11 +414,14 @@ public async Task ReadAsync_RegistersFileStreamForDisposal()
414414
new MvcOptions(),
415415
new MvcNewtonsoftJsonOptions());
416416
var httpContext = new Mock<HttpContext>();
417+
var features = new Mock<IFeatureCollection>();
418+
httpContext.SetupGet(c => c.Features).Returns(features.Object);
417419
IDisposable registerForDispose = null;
418420

419421
var content = Encoding.UTF8.GetBytes("\"Hello world\"");
420422
httpContext.Setup(h => h.Request.Body).Returns(new NonSeekableReadStream(content, allowSyncReads: false));
421423
httpContext.Setup(h => h.Request.ContentType).Returns("application/json");
424+
httpContext.Setup(h => h.Request.ContentLength).Returns(content.Length);
422425
httpContext.Setup(h => h.Response.RegisterForDispose(It.IsAny<IDisposable>()))
423426
.Callback((IDisposable disposable) => registerForDispose = disposable)
424427
.Verifiable();

0 commit comments

Comments
 (0)