Skip to content

Commit 0751c82

Browse files
committed
Optimize prefixed parameters when assembling model name.
Added an option to optimize model names where the model name is the same as one of its properties when binding the model name. This should only be used when this is the desired behavior. Tests have been added to assert this behavior when passing parameters whose model name is the same as the property to be bound. This was introduced to allow circumventing the bug where the parameter name is the same as the name of one of its properties, causing the model to trick the PrefixContainer into considering that parameter as prefixed (model.property), even when it is not. ```C# class SomeClass { public string Parameter { get; set; } = ""; } ``` and later we define the model name as parameter (e.g in a ActionMethod for example) ```C# public IActionResult SomeEndpoint([FromQuery] SomeClass parameter) { /* elided */ } ``` internally, when we pass the query "?parameter=somecoolparametervalue", it will classify it as prefixed (parameter.Parameter instead of only parameter), because of the internal logic of PrefixContainer. However, it is not prefixed, so later on when it try to bind the key (the keys from the query, in this case parameter) with the internal keys (wrong classified as parameter.Parameter) the ModelBinder would not find an valid candidate for matching with the key. Now the optimization option allows to treat the modelName and the propertyName with only one if both are equal.
1 parent f2353d2 commit 0751c82

File tree

5 files changed

+49
-3
lines changed

5 files changed

+49
-3
lines changed

src/Mvc/Mvc.Core/src/ModelBinding/Binders/ComplexObjectModelBinder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ internal void CreateModel(ModelBindingContext bindingContext)
364364
}
365365

366366
var fieldName = property.BinderModelName ?? property.PropertyName!;
367-
var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName);
367+
var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName, skipOptimization: false);
368368
var result = await BindPropertyAsync(bindingContext, property, propertyBinder, fieldName, modelName);
369369

370370
if (result.IsModelSet)

src/Mvc/Mvc.Core/src/ModelBinding/ModelNames.cs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,9 @@ public static string CreateIndexModelName(string parentName, string index)
3939
/// </summary>
4040
/// <param name="prefix">The prefix to use.</param>
4141
/// <param name="propertyName">The property name.</param>
42+
/// <param name="skipOptimization">Skip the optimization of property name and prefix.</param>
4243
/// <returns>The property model name.</returns>
43-
public static string CreatePropertyModelName(string? prefix, string? propertyName)
44+
public static string CreatePropertyModelName(string? prefix, string? propertyName, bool skipOptimization = true)
4445
{
4546
if (string.IsNullOrEmpty(prefix))
4647
{
@@ -59,6 +60,17 @@ public static string CreatePropertyModelName(string? prefix, string? propertyNam
5960
return prefix + propertyName;
6061
}
6162

63+
if (!skipOptimization)
64+
{
65+
if (string.Equals(prefix, propertyName, StringComparison.OrdinalIgnoreCase))
66+
{
67+
// if we are dealing with with something like:
68+
// prefix = parameter and propertyName = parameter
69+
// it should fallback to the property name.
70+
return propertyName;
71+
}
72+
}
73+
6274
return prefix + "." + propertyName;
6375
}
6476
}

src/Mvc/Mvc.Core/src/PublicAPI.Shipped.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2190,7 +2190,6 @@ static Microsoft.AspNetCore.Mvc.ModelBinding.ModelBinderProviderExtensions.Remov
21902190
static Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadataProviderExtensions.GetMetadataForProperty(this Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider! provider, System.Type! containerType, string! propertyName) -> Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata!
21912191
static Microsoft.AspNetCore.Mvc.ModelBinding.ModelNames.CreateIndexModelName(string! parentName, int index) -> string!
21922192
static Microsoft.AspNetCore.Mvc.ModelBinding.ModelNames.CreateIndexModelName(string! parentName, string! index) -> string!
2193-
static Microsoft.AspNetCore.Mvc.ModelBinding.ModelNames.CreatePropertyModelName(string? prefix, string? propertyName) -> string!
21942193
static Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ModelValidatorProviderExtensions.RemoveType(this System.Collections.Generic.IList<Microsoft.AspNetCore.Mvc.ModelBinding.Validation.IModelValidatorProvider!>! list, System.Type! type) -> void
21952194
static Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ModelValidatorProviderExtensions.RemoveType<TModelValidatorProvider>(this System.Collections.Generic.IList<Microsoft.AspNetCore.Mvc.ModelBinding.Validation.IModelValidatorProvider!>! list) -> void
21962195
static Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.StateManager.Recurse(Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor! visitor, string! key, Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata! metadata, object? model, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.IValidationStrategy! strategy) -> Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor.StateManager

src/Mvc/Mvc.Core/src/PublicAPI.Unshipped.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ Microsoft.AspNetCore.Mvc.ProducesDefaultResponseTypeAttribute.Description.get ->
66
Microsoft.AspNetCore.Mvc.ProducesDefaultResponseTypeAttribute.Description.set -> void
77
Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute.Description.get -> string?
88
Microsoft.AspNetCore.Mvc.ProducesResponseTypeAttribute.Description.set -> void
9+
static Microsoft.AspNetCore.Mvc.ModelBinding.ModelNames.CreatePropertyModelName(string? prefix, string? propertyName, bool skipOptimization = true) -> string!

src/Mvc/test/Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,40 @@ public async Task CollectionModelBinder_BindsListOfSimpleType_WithExplicitPrefix
9696
Assert.True(modelState.IsValid);
9797
}
9898

99+
class TestClass
100+
{
101+
public string Parameter { get; set; } = "";
102+
}
103+
104+
[Fact]
105+
public async Task CollectionModelBinder_CanBindProperties_WithSameNameOfParameter()
106+
{
107+
// Arrange
108+
var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
109+
var parameter = new ParameterDescriptor()
110+
{
111+
Name = "parameter",
112+
ParameterType = typeof(TestClass)
113+
};
114+
115+
var testContext = ModelBindingTestHelper.GetTestContext(request =>
116+
{
117+
request.QueryString = new QueryString("?parameter=testing");
118+
});
119+
120+
var modelState = testContext.ModelState;
121+
122+
// Act
123+
var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext);
124+
125+
// Assert
126+
Assert.True(modelBindingResult.IsModelSet);
127+
128+
var model = Assert.IsType<TestClass>(modelBindingResult.Model);
129+
Assert.True(model.Parameter == "testing");
130+
Assert.True(modelState.IsValid);
131+
}
132+
99133
[Theory]
100134
[InlineData("?[0]=10&[1]=11")]
101135
[InlineData("?index=low&index=high&[high]=11&[low]=10")]

0 commit comments

Comments
 (0)