diff --git a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs
index fa1b482f708ae61ddcac55b09511e4913de62b13..bd2caaa62542f66d2797c0d53a8fc21f1c96fc37 100644
--- a/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs
+++ b/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs
@@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
+using System.Diagnostics;
using System.Linq.Expressions;
using System.Reflection;
using System.Threading.Tasks;
@@ -17,6 +18,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
/// </summary>
public class ComplexTypeModelBinder : IModelBinder
{
+ // Don't want a new public enum because communication between the private and internal methods of this class
+ // should not be exposed. Can't use an internal enum because types of [TheoryData] values must be public.
+
+ // Model contains only properties that are expected to bind from value providers and no value provider has
+ // matching data.
+ internal const int NoDataAvailable = 0;
+ // If model contains properties that are expected to bind from value providers, no value provider has matching
+ // data. Remaining (greedy) properties might bind successfully.
+ internal const int GreedyPropertiesMayHaveData = 1;
+ // Model contains at least one property that is expected to bind from value providers and a value provider has
+ // matching data.
+ internal const int ValueProviderDataAvailable = 2;
+
private readonly IDictionary<ModelMetadata, IModelBinder> _propertyBinders;
private readonly ILogger _logger;
private Func<object> _modelCreator;
@@ -76,18 +90,21 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
_logger.AttemptingToBindModel(bindingContext);
- if (!CanCreateModel(bindingContext))
+ var propertyData = CanCreateModel(bindingContext);
+ if (propertyData == NoDataAvailable)
{
return Task.CompletedTask;
}
// Perf: separated to avoid allocating a state machine when we don't
// need to go async.
- return BindModelCoreAsync(bindingContext);
+ return BindModelCoreAsync(bindingContext, propertyData);
}
- private async Task BindModelCoreAsync(ModelBindingContext bindingContext)
+ private async Task BindModelCoreAsync(ModelBindingContext bindingContext, int propertyData)
{
+ Debug.Assert(propertyData == GreedyPropertiesMayHaveData || propertyData == ValueProviderDataAvailable);
+
// Create model first (if necessary) to avoid reporting errors about properties when activation fails.
if (bindingContext.Model == null)
{
@@ -96,6 +113,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var modelMetadata = bindingContext.ModelMetadata;
var attemptedPropertyBinding = false;
+ var propertyBindingSucceeded = false;
+ var postponePlaceholderBinding = false;
for (var i = 0; i < modelMetadata.Properties.Count; i++)
{
var property = modelMetadata.Properties[i];
@@ -104,42 +123,62 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
continue;
}
- // Pass complex (including collection) values down so that binding system does not unnecessarily
- // recreate instances or overwrite inner properties that are not bound. No need for this with simple
- // values because they will be overwritten if binding succeeds. Arrays are never reused because they
- // cannot be resized.
- object propertyModel = null;
- if (property.PropertyGetter != null &&
- property.IsComplexType &&
- !property.ModelType.IsArray)
+ if (_propertyBinders[property] is PlaceholderBinder)
{
- propertyModel = property.PropertyGetter(bindingContext.Model);
+ if (postponePlaceholderBinding)
+ {
+ // Decided to postpone binding properties that complete a loop in the model types when handling
+ // an earlier loop-completing property. Postpone binding this property too.
+ continue;
+ }
+ else if (!bindingContext.IsTopLevelObject &&
+ !propertyBindingSucceeded &&
+ propertyData == GreedyPropertiesMayHaveData)
+ {
+ // Have no confirmation of data for the current instance. Postpone completing the loop until
+ // we _know_ the current instance is useful. Recursion would otherwise occur prior to the
+ // block with a similar condition after the loop.
+ //
+ // Example cases include an Employee class containing
+ // 1. a Manager property of type Employee
+ // 2. an Employees property of type IList<Employee>
+ postponePlaceholderBinding = true;
+ continue;
+ }
}
var fieldName = property.BinderModelName ?? property.PropertyName;
var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName);
-
- ModelBindingResult result;
- using (bindingContext.EnterNestedScope(
- modelMetadata: property,
- fieldName: fieldName,
- modelName: modelName,
- model: propertyModel))
- {
- await BindProperty(bindingContext);
- result = bindingContext.Result;
- }
+ var result = await BindProperty(bindingContext, property, fieldName, modelName);
if (result.IsModelSet)
{
attemptedPropertyBinding = true;
- SetProperty(bindingContext, modelName, property, result);
+ propertyBindingSucceeded = true;
}
else if (property.IsBindingRequired)
{
attemptedPropertyBinding = true;
- var message = property.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(fieldName);
- bindingContext.ModelState.TryAddModelError(modelName, message);
+ }
+ }
+
+ if (postponePlaceholderBinding && propertyBindingSucceeded)
+ {
+ // Have some data for this instance. Continue with the model type loop.
+ for (var i = 0; i < modelMetadata.Properties.Count; i++)
+ {
+ var property = modelMetadata.Properties[i];
+ if (!CanBindProperty(bindingContext, property))
+ {
+ continue;
+ }
+
+ if (_propertyBinders[property] is PlaceholderBinder)
+ {
+ var fieldName = property.BinderModelName ?? property.PropertyName;
+ var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName);
+ await BindProperty(bindingContext, property, fieldName, modelName);
+ }
}
}
@@ -157,8 +196,37 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, message);
}
- bindingContext.Result = ModelBindingResult.Success(bindingContext.Model);
_logger.DoneAttemptingToBindModel(bindingContext);
+
+ // Have all binders failed because no data was available?
+ //
+ // If CanCreateModel determined a property has data, failures are likely due to conversion errors. For
+ // example, user may submit ?[0].id=twenty&[1].id=twenty-one&[2].id=22 for a collection of a complex type
+ // with an int id property. In that case, the bound model should be [ {}, {}, { id = 22 }] and
+ // ModelState should contain errors about both [0].id and [1].id. Do not inform higher-level binders of the
+ // failure in this and similar cases.
+ //
+ // If CanCreateModel could not find data for non-greedy properties, failures indicate greedy binders were
+ // unsuccessful. For example, user may submit file attachments [0].File and [1].File but not [2].File for
+ // a collection of a complex type containing an IFormFile property. In that case, we have exhausted the
+ // attached files and checking for [3].File is likely be pointless. (And, if it had a point, would we stop
+ // after 10 failures, 100, or more -- all adding redundant errors to ModelState?) Inform higher-level
+ // binders of the failure.
+ //
+ // Required properties do not change the logic below. Missed required properties cause ModelState errors
+ // but do not necessarily prevent further attempts to bind.
+ //
+ // This logic is intended to maximize correctness but does not avoid infinite loops or recursion when a
+ // greedy model binder succeeds unconditionally.
+ if (!bindingContext.IsTopLevelObject &&
+ !propertyBindingSucceeded &&
+ propertyData == GreedyPropertiesMayHaveData)
+ {
+ bindingContext.Result = ModelBindingResult.Failed();
+ return;
+ }
+
+ bindingContext.Result = ModelBindingResult.Success(bindingContext.Model);
}
/// <summary>
@@ -194,6 +262,48 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
return true;
}
+ private async Task<ModelBindingResult> BindProperty(
+ ModelBindingContext bindingContext,
+ ModelMetadata property,
+ string fieldName,
+ string modelName)
+ {
+ // Pass complex (including collection) values down so that binding system does not unnecessarily
+ // recreate instances or overwrite inner properties that are not bound. No need for this with simple
+ // values because they will be overwritten if binding succeeds. Arrays are never reused because they
+ // cannot be resized.
+ object propertyModel = null;
+ if (property.PropertyGetter != null &&
+ property.IsComplexType &&
+ !property.ModelType.IsArray)
+ {
+ propertyModel = property.PropertyGetter(bindingContext.Model);
+ }
+
+ ModelBindingResult result;
+ using (bindingContext.EnterNestedScope(
+ modelMetadata: property,
+ fieldName: fieldName,
+ modelName: modelName,
+ model: propertyModel))
+ {
+ await BindProperty(bindingContext);
+ result = bindingContext.Result;
+ }
+
+ if (result.IsModelSet)
+ {
+ SetProperty(bindingContext, modelName, property, result);
+ }
+ else if (property.IsBindingRequired)
+ {
+ var message = property.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(fieldName);
+ bindingContext.ModelState.TryAddModelError(modelName, message);
+ }
+
+ return result;
+ }
+
/// <summary>
/// Attempts to bind a property of the model.
/// </summary>
@@ -208,7 +318,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
return binder.BindModelAsync(bindingContext);
}
- internal bool CanCreateModel(ModelBindingContext bindingContext)
+ internal int CanCreateModel(ModelBindingContext bindingContext)
{
var isTopLevelObject = bindingContext.IsTopLevelObject;
@@ -227,33 +337,28 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var bindingSource = bindingContext.BindingSource;
if (!isTopLevelObject && bindingSource != null && bindingSource.IsGreedy)
{
- return false;
+ return NoDataAvailable;
}
// Create the object if:
// 1. It is a top level model.
if (isTopLevelObject)
{
- return true;
+ return ValueProviderDataAvailable;
}
// 2. Any of the model properties can be bound.
- if (CanBindAnyModelProperties(bindingContext))
- {
- return true;
- }
-
- return false;
+ return CanBindAnyModelProperties(bindingContext);
}
- private bool CanBindAnyModelProperties(ModelBindingContext bindingContext)
+ private int CanBindAnyModelProperties(ModelBindingContext bindingContext)
{
// If there are no properties on the model, there is nothing to bind. We are here means this is not a top
// level object. So we return false.
if (bindingContext.ModelMetadata.Properties.Count == 0)
{
_logger.NoPublicSettableProperties(bindingContext);
- return false;
+ return NoDataAvailable;
}
// We want to check to see if any of the properties of the model can be bound using the value providers or
@@ -279,7 +384,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
// Bottom line, if any property meets the above conditions and has a value from ValueProviders, then we'll
// create the model and try to bind it. Of, if ANY properties of the model have a greedy source,
// then we go ahead and create it.
- //
+ var hasGreedyBinders = false;
for (var i = 0; i < bindingContext.ModelMetadata.Properties.Count; i++)
{
var propertyMetadata = bindingContext.ModelMetadata.Properties[i];
@@ -292,7 +397,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var bindingSource = propertyMetadata.BindingSource;
if (bindingSource != null && bindingSource.IsGreedy)
{
- return true;
+ hasGreedyBinders = true;
+ continue;
}
// Otherwise, check whether the (perhaps filtered) value providers have a match.
@@ -307,14 +413,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
// If any property can be bound from a value provider, then success.
if (bindingContext.ValueProvider.ContainsPrefix(bindingContext.ModelName))
{
- return true;
+ return ValueProviderDataAvailable;
}
}
}
+ if (hasGreedyBinders)
+ {
+ return GreedyPropertiesMayHaveData;
+ }
+
_logger.CannotBindToComplexType(bindingContext);
- return false;
+ return NoDataAvailable;
}
// Internal for tests
diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs
index 8774ba896f5906db1c0180b51d9bb372e2decfae..d8f282c337bdc98a23bdb520cc602ab1a4763a7e 100644
--- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs
+++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs
@@ -24,11 +24,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
private static readonly IModelMetadataProvider _metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
[Theory]
- [InlineData(true, true)]
- [InlineData(false, false)]
- public void CanCreateModel_ReturnsTrue_IfIsTopLevelObject(
- bool isTopLevelObject,
- bool expectedCanCreate)
+ [InlineData(true, ComplexTypeModelBinder.ValueProviderDataAvailable)]
+ [InlineData(false, ComplexTypeModelBinder.NoDataAvailable)]
+ public void CanCreateModel_ReturnsTrue_IfIsTopLevelObject(bool isTopLevelObject, int expectedCanCreate)
{
var bindingContext = CreateContext(GetMetadataForType(typeof(Person)));
bindingContext.IsTopLevelObject = isTopLevelObject;
@@ -56,7 +54,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.False(canCreate);
+ Assert.Equal(ComplexTypeModelBinder.NoDataAvailable, canCreate);
}
[Fact]
@@ -71,16 +69,16 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.True(canCreate);
+ Assert.Equal(ComplexTypeModelBinder.ValueProviderDataAvailable, canCreate);
}
[Theory]
- [InlineData(true)]
- [InlineData(false)]
- public void CanCreateModel_CreatesModel_WithAllGreedyProperties(bool isTopLevelObject)
+ [InlineData(ComplexTypeModelBinder.ValueProviderDataAvailable)]
+ [InlineData(ComplexTypeModelBinder.GreedyPropertiesMayHaveData)]
+ public void CanCreateModel_CreatesModel_WithAllGreedyProperties(int expectedCanCreate)
{
var bindingContext = CreateContext(GetMetadataForType(typeof(HasAllGreedyProperties)));
- bindingContext.IsTopLevelObject = isTopLevelObject;
+ bindingContext.IsTopLevelObject = expectedCanCreate == ComplexTypeModelBinder.ValueProviderDataAvailable;
var binder = CreateBinder(bindingContext.ModelMetadata);
@@ -88,20 +86,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.True(canCreate);
+ Assert.Equal(expectedCanCreate, canCreate);
}
[Theory]
- [InlineData(true)]
- [InlineData(false)]
- public void CanCreateModel_ReturnsTrue_IfNotIsTopLevelObject_BasedOnValueAvailability(
- bool valueAvailable)
+ [InlineData(ComplexTypeModelBinder.ValueProviderDataAvailable)]
+ [InlineData(ComplexTypeModelBinder.NoDataAvailable)]
+ public void CanCreateModel_ReturnsTrue_IfNotIsTopLevelObject_BasedOnValueAvailability(int valueAvailable)
{
// Arrange
var valueProvider = new Mock<IValueProvider>(MockBehavior.Strict);
valueProvider
.Setup(provider => provider.ContainsPrefix("SimpleContainer.Simple.Name"))
- .Returns(valueAvailable);
+ .Returns(valueAvailable == ComplexTypeModelBinder.ValueProviderDataAvailable);
var modelMetadata = GetMetadataForProperty(typeof(SimpleContainer), nameof(SimpleContainer.Simple));
var bindingContext = CreateContext(modelMetadata);
@@ -133,7 +130,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.False(canCreate);
+ Assert.Equal(ComplexTypeModelBinder.NoDataAvailable, canCreate);
}
[Fact]
@@ -149,20 +146,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.True(canCreate);
+ Assert.Equal(ComplexTypeModelBinder.ValueProviderDataAvailable, canCreate);
}
[Theory]
- [InlineData(typeof(TypeWithNoBinderMetadata), false)]
- [InlineData(typeof(TypeWithNoBinderMetadata), true)]
+ [InlineData(typeof(TypeWithNoBinderMetadata), ComplexTypeModelBinder.NoDataAvailable)]
+ [InlineData(typeof(TypeWithNoBinderMetadata), ComplexTypeModelBinder.ValueProviderDataAvailable)]
public void CanCreateModel_CreatesModelForValueProviderBasedBinderMetadatas_IfAValueProviderProvidesValue(
Type modelType,
- bool valueProviderProvidesValue)
+ int valueProviderProvidesValue)
{
var valueProvider = new Mock<IValueProvider>();
valueProvider
.Setup(o => o.ContainsPrefix(It.IsAny<string>()))
- .Returns(valueProviderProvidesValue);
+ .Returns(valueProviderProvidesValue == ComplexTypeModelBinder.ValueProviderDataAvailable);
var bindingContext = CreateContext(GetMetadataForType(modelType));
bindingContext.IsTopLevelObject = false;
@@ -179,18 +176,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
}
[Theory]
- [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), false)]
- [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), true)]
- [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false)]
- [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true)]
+ [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), ComplexTypeModelBinder.GreedyPropertiesMayHaveData)]
+ [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), ComplexTypeModelBinder.ValueProviderDataAvailable)]
+ [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), ComplexTypeModelBinder.GreedyPropertiesMayHaveData)]
+ [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), ComplexTypeModelBinder.ValueProviderDataAvailable)]
public void CanCreateModel_CreatesModelForValueProviderBasedBinderMetadatas_IfPropertyHasGreedyBindingSource(
Type modelType,
- bool valueProviderProvidesValue)
+ int expectedCanCreate)
{
var valueProvider = new Mock<IValueProvider>();
valueProvider
.Setup(o => o.ContainsPrefix(It.IsAny<string>()))
- .Returns(valueProviderProvidesValue);
+ .Returns(expectedCanCreate == ComplexTypeModelBinder.ValueProviderDataAvailable);
var bindingContext = CreateContext(GetMetadataForType(modelType));
bindingContext.IsTopLevelObject = false;
@@ -203,15 +200,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.True(canCreate);
+ Assert.Equal(expectedCanCreate, canCreate);
}
[Theory]
- [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), false)]
- [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), true)]
+ [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), ComplexTypeModelBinder.GreedyPropertiesMayHaveData)]
+ [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), ComplexTypeModelBinder.ValueProviderDataAvailable)]
public void CanCreateModel_ForExplicitValueProviderMetadata_UsesOriginalValueProvider(
Type modelType,
- bool originalValueProviderProvidesValue)
+ int expectedCanCreate)
{
var valueProvider = new Mock<IValueProvider>();
valueProvider
@@ -221,7 +218,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var originalValueProvider = new Mock<IBindingSourceValueProvider>();
originalValueProvider
.Setup(o => o.ContainsPrefix(It.IsAny<string>()))
- .Returns(originalValueProviderProvidesValue);
+ .Returns(expectedCanCreate == ComplexTypeModelBinder.ValueProviderDataAvailable);
originalValueProvider
.Setup(o => o.Filter(It.IsAny<BindingSource>()))
@@ -238,18 +235,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
var canCreate = binder.CanCreateModel(bindingContext);
// Assert
- Assert.True(canCreate);
+ Assert.Equal(expectedCanCreate, canCreate);
}
[Theory]
- [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false, true)]
- [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true, true)]
- [InlineData(typeof(TypeWithNoBinderMetadata), false, false)]
- [InlineData(typeof(TypeWithNoBinderMetadata), true, true)]
+ [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false, ComplexTypeModelBinder.GreedyPropertiesMayHaveData)]
+ [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true, ComplexTypeModelBinder.ValueProviderDataAvailable)]
+ [InlineData(typeof(TypeWithNoBinderMetadata), false, ComplexTypeModelBinder.NoDataAvailable)]
+ [InlineData(typeof(TypeWithNoBinderMetadata), true, ComplexTypeModelBinder.ValueProviderDataAvailable)]
public void CanCreateModel_UnmarkedProperties_UsesCurrentValueProvider(
Type modelType,
bool valueProviderProvidesValue,
- bool expectedCanCreate)
+ int expectedCanCreate)
{
var valueProvider = new Mock<IValueProvider>();
valueProvider
@@ -602,8 +599,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
// Arrange
var bindingContext = CreateContext(GetMetadataForType(typeof(Person)), new Person());
var originalModel = bindingContext.Model;
+ var binders = bindingContext.ModelMetadata.Properties.ToDictionary(
+ keySelector: item => item,
+ elementSelector: item => (IModelBinder)null);
- var binder = new Mock<TestableComplexTypeModelBinder>() { CallBase = true };
+ var binder = new Mock<TestableComplexTypeModelBinder>(binders) { CallBase = true };
binder
.Setup(b => b.CreateModelPublic(It.IsAny<ModelBindingContext>()))
.Verifiable();
@@ -621,8 +621,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
{
// Arrange
var bindingContext = CreateContext(GetMetadataForType(typeof(Person)), model: null);
+ var binders = bindingContext.ModelMetadata.Properties.ToDictionary(
+ keySelector: item => item,
+ elementSelector: item => (IModelBinder)null);
- var testableBinder = new Mock<TestableComplexTypeModelBinder> { CallBase = true };
+ var testableBinder = new Mock<TestableComplexTypeModelBinder>(binders) { CallBase = true };
testableBinder
.Setup(o => o.CreateModelPublic(bindingContext))
.Returns(new Person())
diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs
index beffb6d9a1ac47c841c2e3cc96562a19e8dcf996..72219b7ad8ce97932bc9cbcd09e51bf2975263d8 100644
--- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs
+++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs
@@ -3141,6 +3141,276 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
Assert.Equal("10,20", entry.RawValue);
}
+ private class Person5
+ {
+ public string Name { get; set; }
+ public IFormFile Photo { get; set; }
+ }
+
+ // Regression test for #4802.
+ [Fact]
+ public async Task ComplexTypeModelBinder_ReportsFailureToCollectionModelBinder()
+ {
+ // Arrange
+ var parameter = new ParameterDescriptor()
+ {
+ Name = "parameter",
+ ParameterType = typeof(IList<Person5>),
+ };
+
+ var testContext = ModelBindingTestHelper.GetTestContext(request =>
+ {
+ SetFormFileBodyContent(request, "Hello world!", "[0].Photo");
+
+ // CollectionModelBinder binds an empty collection when value providers are all empty.
+ request.QueryString = new QueryString("?a=b");
+ });
+
+ var modelState = testContext.ModelState;
+ var metadata = GetMetadata(testContext, parameter);
+ var modelBinder = GetModelBinder(testContext, parameter, metadata);
+ var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+ var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+ // Act
+ var modelBindingResult = await parameterBinder.BindModelAsync(
+ testContext,
+ modelBinder,
+ valueProvider,
+ parameter,
+ metadata,
+ value: null);
+
+ // Assert
+ Assert.True(modelBindingResult.IsModelSet);
+
+ var model = Assert.IsType<List<Person5>>(modelBindingResult.Model);
+ var person = Assert.Single(model);
+ Assert.Null(person.Name);
+ Assert.NotNull(person.Photo);
+ using (var reader = new StreamReader(person.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello world!", await reader.ReadToEndAsync());
+ }
+
+ Assert.True(modelState.IsValid);
+ var state = Assert.Single(modelState);
+ Assert.Equal("[0].Photo", state.Key);
+ Assert.Null(state.Value.AttemptedValue);
+ Assert.Empty(state.Value.Errors);
+ Assert.Null(state.Value.RawValue);
+ }
+
+ private class Person6
+ {
+ public string Name { get; set; }
+
+ public Person6 Mother { get; set; }
+
+ public IFormFile Photo { get; set; }
+ }
+
+ // Regression test for #6616.
+ [Fact]
+ public async Task ComplexTypeModelBinder_ReportsFailureToComplexTypeModelBinder_NearTopLevel()
+ {
+ // Arrange
+ var parameter = new ParameterDescriptor()
+ {
+ Name = "parameter",
+ ParameterType = typeof(Person6),
+ };
+
+ var testContext = ModelBindingTestHelper.GetTestContext(request =>
+ {
+ SetFormFileBodyContent(request, "Hello world!", "Photo");
+ });
+
+ var modelState = testContext.ModelState;
+ var metadata = GetMetadata(testContext, parameter);
+ var modelBinder = GetModelBinder(testContext, parameter, metadata);
+ var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+ var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+ // Act
+ var modelBindingResult = await parameterBinder.BindModelAsync(
+ testContext,
+ modelBinder,
+ valueProvider,
+ parameter,
+ metadata,
+ value: null);
+
+ // Assert
+ Assert.True(modelBindingResult.IsModelSet);
+
+ var model = Assert.IsType<Person6>(modelBindingResult.Model);
+ Assert.Null(model.Mother);
+ Assert.Null(model.Name);
+ Assert.NotNull(model.Photo);
+ using (var reader = new StreamReader(model.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello world!", await reader.ReadToEndAsync());
+ }
+
+ Assert.True(modelState.IsValid);
+ var state = Assert.Single(modelState);
+ Assert.Equal("Photo", state.Key);
+ Assert.Null(state.Value.AttemptedValue);
+ Assert.Empty(state.Value.Errors);
+ Assert.Null(state.Value.RawValue);
+ }
+
+ // Regression test for #6616.
+ [Fact]
+ public async Task ComplexTypeModelBinder_ReportsFailureToComplexTypeModelBinder()
+ {
+ // Arrange
+ var parameter = new ParameterDescriptor()
+ {
+ Name = "parameter",
+ ParameterType = typeof(Person6),
+ };
+
+ var testContext = ModelBindingTestHelper.GetTestContext(request =>
+ {
+ SetFormFileBodyContent(request, "Hello world!", "Photo");
+ SetFormFileBodyContent(request, "Hello Mom!", "Mother.Photo");
+ });
+
+ var modelState = testContext.ModelState;
+ var metadata = GetMetadata(testContext, parameter);
+ var modelBinder = GetModelBinder(testContext, parameter, metadata);
+ var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+ var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+ // Act
+ var modelBindingResult = await parameterBinder.BindModelAsync(
+ testContext,
+ modelBinder,
+ valueProvider,
+ parameter,
+ metadata,
+ value: null);
+
+ // Assert
+ Assert.True(modelBindingResult.IsModelSet);
+
+ var model = Assert.IsType<Person6>(modelBindingResult.Model);
+ Assert.NotNull(model.Mother);
+ Assert.Null(model.Mother.Mother);
+ Assert.NotNull(model.Mother.Photo);
+ using (var reader = new StreamReader(model.Mother.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello Mom!", await reader.ReadToEndAsync());
+ }
+
+ Assert.Null(model.Name);
+ Assert.NotNull(model.Photo);
+ using (var reader = new StreamReader(model.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello world!", await reader.ReadToEndAsync());
+ }
+
+ Assert.True(modelState.IsValid);
+ Assert.Collection(
+ modelState,
+ kvp =>
+ {
+ Assert.Equal("Photo", kvp.Key);
+ Assert.Null(kvp.Value.AttemptedValue);
+ Assert.Empty(kvp.Value.Errors);
+ Assert.Null(kvp.Value.RawValue);
+ },
+ kvp =>
+ {
+ Assert.Equal("Mother.Photo", kvp.Key);
+ Assert.Null(kvp.Value.AttemptedValue);
+ Assert.Empty(kvp.Value.Errors);
+ Assert.Null(kvp.Value.RawValue);
+ });
+ }
+
+ private class Person7
+ {
+ public string Name { get; set; }
+
+ public IList<Person7> Children { get; set; }
+
+ public IFormFile Photo { get; set; }
+ }
+
+ // Regression test for #6616.
+ [Fact]
+ public async Task ComplexTypeModelBinder_ReportsFailureToComplexTypeModelBinder_ViaCollection()
+ {
+ // Arrange
+ var parameter = new ParameterDescriptor()
+ {
+ Name = "parameter",
+ ParameterType = typeof(Person7),
+ };
+
+ var testContext = ModelBindingTestHelper.GetTestContext(request =>
+ {
+ SetFormFileBodyContent(request, "Hello world!", "Photo");
+ SetFormFileBodyContent(request, "Hello Fred!", "Children[0].Photo");
+ SetFormFileBodyContent(request, "Hello Ginger!", "Children[1].Photo");
+
+ request.QueryString = new QueryString("?Children[0].Name=Fred&Children[1].Name=Ginger");
+ });
+
+ var modelState = testContext.ModelState;
+ var metadata = GetMetadata(testContext, parameter);
+ var modelBinder = GetModelBinder(testContext, parameter, metadata);
+ var valueProvider = await CompositeValueProvider.CreateAsync(testContext);
+ var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext);
+
+ // Act
+ var modelBindingResult = await parameterBinder.BindModelAsync(
+ testContext,
+ modelBinder,
+ valueProvider,
+ parameter,
+ metadata,
+ value: null);
+
+ // Assert
+ Assert.True(modelBindingResult.IsModelSet);
+
+ var model = Assert.IsType<Person7>(modelBindingResult.Model);
+ Assert.NotNull(model.Children);
+ Assert.Collection(
+ model.Children,
+ item =>
+ {
+ Assert.Null(item.Children);
+ Assert.Equal("Fred", item.Name);
+ using (var reader = new StreamReader(item.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello Fred!", reader.ReadToEnd());
+ }
+ },
+ item =>
+ {
+ Assert.Null(item.Children);
+ Assert.Equal("Ginger", item.Name);
+ using (var reader = new StreamReader(item.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello Ginger!", reader.ReadToEnd());
+ }
+ });
+
+ Assert.Null(model.Name);
+ Assert.NotNull(model.Photo);
+ using (var reader = new StreamReader(model.Photo.OpenReadStream()))
+ {
+ Assert.Equal("Hello world!", await reader.ReadToEndAsync());
+ }
+
+ Assert.True(modelState.IsValid);
+ }
+
private static void SetJsonBodyContent(HttpRequest request, string content)
{
var stream = new MemoryStream(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes(content));
@@ -3151,18 +3421,32 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
private static void SetFormFileBodyContent(HttpRequest request, string content, string name)
{
const string fileName = "text.txt";
- var fileCollection = new FormFileCollection();
- var formCollection = new FormCollection(new Dictionary<string, StringValues>(), fileCollection);
- var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(content));
- request.Form = formCollection;
- request.ContentType = "multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq";
- request.Headers["Content-Disposition"] = $"form-data; name={name}; filename={fileName}";
+ FormFileCollection fileCollection;
+ if (request.HasFormContentType)
+ {
+ // Do less work and do not overwrite previous information if called a second time.
+ fileCollection = (FormFileCollection)request.Form.Files;
+ }
+ else
+ {
+ fileCollection = new FormFileCollection();
+ var formCollection = new FormCollection(new Dictionary<string, StringValues>(), fileCollection);
+
+ request.ContentType = "multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq";
+ request.Form = formCollection;
+ }
- fileCollection.Add(new FormFile(memoryStream, 0, memoryStream.Length, name, fileName)
+ var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(content));
+ var file = new FormFile(memoryStream, 0, memoryStream.Length, name, fileName)
{
- Headers = request.Headers
- });
+ Headers = new HeaderDictionary(),
+
+ // Do not move this up. Headers must be non-null before the ContentDisposition property is accessed.
+ ContentDisposition = $"form-data; name={name}; filename={fileName}",
+ };
+
+ fileCollection.Add(file);
}
private ModelMetadata GetMetadata(ModelBindingTestContext context, ParameterDescriptor parameter)
diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs
index fb89e0e2049e10b423edec515144c9147a7d1281..a5440e5de2d0751c76c1e8b4dd77d647c3e27271 100644
--- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs
+++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs
@@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.ComponentModel;
-using System.ComponentModel.DataAnnotations;
using System.Globalization;
using System.Linq;
using System.Threading.Tasks;
@@ -25,7 +24,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
private class Address
{
[FromHeader(Name = "Header")]
- [Required]
+ [BindRequired]
public string Street { get; set; }
}
@@ -33,6 +32,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
public async Task BindPropertyFromHeader_NoData_UsesFullPathAsKeyForModelStateErrors()
{
// Arrange
+ var expected = "A value for the 'Header' parameter or property was not provided.";
var parameter = new ParameterDescriptor()
{
Name = "Parameter1",
@@ -65,7 +65,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
var key = Assert.Single(modelState.Keys);
Assert.Equal("CustomParameter.Address.Header", key);
var error = Assert.Single(modelState[key].Errors);
- Assert.Equal(ValidationAttributeUtil.GetRequiredErrorMessage("Street"), error.ErrorMessage);
+ Assert.Equal(expected, error.ErrorMessage);
}
[Fact]
@@ -277,7 +277,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
ParameterType = modelType
};
- Action<HttpRequest> action = r => r.Headers.Add("CustomParameter", new[] { expectedAttemptedValue });
+ void action(HttpRequest r) => r.Headers.Add("CustomParameter", new[] { expectedAttemptedValue });
var testContext = GetModelBindingTestContext(action);
var parameterBinder = ModelBindingTestHelper.GetParameterBinder(testContext.HttpContext.RequestServices);