Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Commit 842d661

Browse files
kichallakichalla
committed
[Fixes #5698] Regression in 1.1 model binding for model types without default constructor
- Also reverts "Check for default constructor in ComplexTypeModelBinderProvider" commit d09e921.
1 parent bd9e431 commit 842d661

8 files changed

Lines changed: 163 additions & 156 deletions

File tree

‎src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq.Expressions;
77
using System.Reflection;
88
using System.Threading.Tasks;
9+
using Microsoft.AspNetCore.Mvc.Core;
910
using Microsoft.AspNetCore.Mvc.Internal;
1011

1112
namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
@@ -47,6 +48,28 @@ public Task BindModelAsync(ModelBindingContext bindingContext)
4748
return TaskCache.CompletedTask;
4849
}
4950

51+
// The following check causes the ComplexTypeModelBinder to NOT participate in binding structs as
52+
// reflection does not provide information about the implicit parameterless constructor for a struct.
53+
// This binder would eventually fail to construct an instance of the struct as the Linq's NewExpression
54+
// compile fails to construct it.
55+
var modelTypeInfo = bindingContext.ModelType.GetTypeInfo();
56+
if (bindingContext.Model == null &&
57+
(modelTypeInfo.IsAbstract ||
58+
modelTypeInfo.GetConstructor(Type.EmptyTypes) == null))
59+
{
60+
if (bindingContext.IsTopLevelObject)
61+
{
62+
throw new InvalidOperationException(
63+
Resources.FormatComplexTypeModelBinder_NoParameterlessConstructor_TopLevelObject(modelTypeInfo.FullName));
64+
}
65+
66+
throw new InvalidOperationException(
67+
Resources.FormatComplexTypeModelBinder_NoParameterlessConstructor_ForProperty(
68+
modelTypeInfo.FullName,
69+
bindingContext.ModelName,
70+
bindingContext.ModelMetadata.ContainerType.FullName));
71+
}
72+
5073
// Perf: separated to avoid allocating a state machine when we don't
5174
// need to go async.
5275
return BindModelCoreAsync(bindingContext);

‎src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinderProvider.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Reflection;
65
using System.Collections.Generic;
76

87
namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
@@ -20,9 +19,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
2019
throw new ArgumentNullException(nameof(context));
2120
}
2221

23-
if (context.Metadata.IsComplexType &&
24-
!context.Metadata.IsCollectionType &&
25-
HasDefaultConstructor(context.Metadata.ModelType.GetTypeInfo()))
22+
if (context.Metadata.IsComplexType && !context.Metadata.IsCollectionType)
2623
{
2724
var propertyBinders = new Dictionary<ModelMetadata, IModelBinder>();
2825
foreach (var property in context.Metadata.Properties)
@@ -35,14 +32,5 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
3532

3633
return null;
3734
}
38-
39-
private bool HasDefaultConstructor(TypeInfo modelTypeInfo)
40-
{
41-
// The following check causes the ComplexTypeModelBinder to NOT participate in binding structs.
42-
// - Reflection does not provide information about the implicit parameterless constructor for a struct.
43-
// - Also this binder would eventually fail to construct an instance of the struct as the Linq's
44-
// NewExpression compile fails to construct it.
45-
return !modelTypeInfo.IsAbstract && modelTypeInfo.GetConstructor(Type.EmptyTypes) != null;
46-
}
4735
}
4836
}

‎src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs‎

Lines changed: 50 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,4 +387,10 @@
387387
<value>Unable to create an instance of type '{0}'. The type specified in {1} must not be abstract and must have a parameterless constructor.</value>
388388
<comment>0 is the type to configure. 1 is the name of the parameter, configurationType.</comment>
389389
</data>
390+
<data name="ComplexTypeModelBinder_NoParameterlessConstructor_TopLevelObject" xml:space="preserve">
391+
<value>Could not create an instance of type '{0}'. Model bound complex types must not be abstract or value types and must have a parameterless constructor.</value>
392+
</data>
393+
<data name="ComplexTypeModelBinder_NoParameterlessConstructor_ForProperty" xml:space="preserve">
394+
<value>Could not create an instance of type '{0}'. Model bound complex types must not be abstract or value types and must have a parameterless constructor. Alternatively, set the '{1}' property to a non-null value in the '{2}' constructor.</value>
395+
</data>
390396
</root>

‎src/Microsoft.AspNetCore.Mvc.TagHelpers/Properties/Resources.Designer.cs‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderProviderTest.cs‎

Lines changed: 0 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -55,107 +55,11 @@ public void Create_ForSupportedTypes_ReturnsBinder()
5555
Assert.IsType<ComplexTypeModelBinder>(result);
5656
}
5757

58-
[Theory]
59-
[InlineData(typeof(PointStructWithExplicitConstructor))]
60-
[InlineData(typeof(PointStructWithNoExplicitConstructor))]
61-
public void Create_ForStructModel_ReturnsNull(Type modelType)
62-
{
63-
// Arrange
64-
var provider = new ComplexTypeModelBinderProvider();
65-
var context = new TestModelBinderProviderContext(modelType);
66-
67-
// Act
68-
var result = provider.GetBinder(context);
69-
70-
// Assert
71-
Assert.Null(result);
72-
}
73-
74-
[Theory]
75-
[InlineData(typeof(ClassWithNoDefaultConstructor))]
76-
[InlineData(typeof(ClassWithStaticConstructor))]
77-
[InlineData(typeof(ClassWithInternalDefaultConstructor))]
78-
public void Create_ForModelTypeWithNoDefaultPublicConstructor_ReturnsNull(Type modelType)
79-
{
80-
// Arrange
81-
var provider = new ComplexTypeModelBinderProvider();
82-
var context = new TestModelBinderProviderContext(modelType);
83-
84-
// Act
85-
var result = provider.GetBinder(context);
86-
87-
// Assert
88-
Assert.Null(result);
89-
}
90-
91-
[Fact]
92-
public void Create_ForAbstractModelTypeWithDefaultPublicConstructor_ReturnsNull()
93-
{
94-
// Arrange
95-
var provider = new ComplexTypeModelBinderProvider();
96-
var context = new TestModelBinderProviderContext(typeof(AbstractClassWithDefaultConstructor));
97-
98-
// Act
99-
var result = provider.GetBinder(context);
100-
101-
// Assert
102-
Assert.Null(result);
103-
}
104-
105-
private struct PointStructWithNoExplicitConstructor
106-
{
107-
public double X { get; set; }
108-
public double Y { get; set; }
109-
}
110-
111-
private struct PointStructWithExplicitConstructor
112-
{
113-
public PointStructWithExplicitConstructor(double x, double y)
114-
{
115-
X = x;
116-
Y = y;
117-
}
118-
public double X { get; }
119-
public double Y { get; }
120-
}
121-
12258
private class Person
12359
{
12460
public string Name { get; set; }
12561

12662
public int Age { get; set; }
12763
}
128-
129-
private class ClassWithNoDefaultConstructor
130-
{
131-
public ClassWithNoDefaultConstructor(int id) { }
132-
}
133-
134-
private class ClassWithInternalDefaultConstructor
135-
{
136-
internal ClassWithInternalDefaultConstructor() { }
137-
}
138-
139-
private class ClassWithStaticConstructor
140-
{
141-
static ClassWithStaticConstructor() { }
142-
143-
public ClassWithStaticConstructor(int id) { }
144-
}
145-
146-
private abstract class AbstractClassWithDefaultConstructor
147-
{
148-
private readonly string _name;
149-
150-
public AbstractClassWithDefaultConstructor()
151-
: this("James")
152-
{
153-
}
154-
155-
public AbstractClassWithDefaultConstructor(string name)
156-
{
157-
_name = name;
158-
}
159-
}
16064
}
16165
}

0 commit comments

Comments
 (0)