huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Throw if multiple Body bound parameters are discovered 

Fixes #6963
This commit is contained in:
Pranav K 2018-04-05 15:43:14 -07:00
parent feed0fea2c
commit ec31ff0c28
No known key found for this signature in database
GPG Key ID: 1963DA6D96C3057A
8 changed files with 177 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ParameterModel.cs
View File

@ -5,6 +5,7 @@ using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
using Microsoft.Extensions.Internal;
namespace Microsoft.AspNetCore.Mvc.ApplicationModels
{
@ -46,5 +47,14 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
get => Name;
set => Name = value;
}
public string DisplayName
{
get
{
var parameterTypeName = TypeNameHelper.GetTypeDisplayName(ParameterInfo.ParameterType, fullName: false);
return $"{parameterTypeName} {ParameterName}";
}
}
}
}

42
src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs
View File

@ -147,44 +147,34 @@ namespace Microsoft.AspNetCore.Mvc.Internal
return;
}
var inferredBindingSources = new BindingSource[actionModel.Parameters.Count];
var foundFromBodyParameter = false;
for (var i = 0; i < inferredBindingSources.Length; i++)
for (var i = 0; i < actionModel.Parameters.Count; i++)
{
var parameter = actionModel.Parameters[i];
var bindingSource = parameter.BindingInfo?.BindingSource;
if (bindingSource == null)
{
bindingSource = InferBindingSourceForParameter(parameter);
}
if (bindingSource == BindingSource.Body)
{
if (foundFromBodyParameter)
{
// More than one parameter is inferred as FromBody. Log a warning and skip this action.
_logger.UnableToInferBindingSource(actionModel);
}
else
{
foundFromBodyParameter = true;
}
}
inferredBindingSources[i] = bindingSource;
}
for (var i = 0; i < inferredBindingSources.Length; i++)
{
var bindingSource = inferredBindingSources[i];
if (bindingSource != null)
{
actionModel.Parameters[i].BindingInfo = new BindingInfo
parameter.BindingInfo = new BindingInfo
{
BindingSource = bindingSource,
};
}
}
var fromBodyParameters = actionModel.Parameters.Where(p => p.BindingInfo.BindingSource == BindingSource.Body).ToList();
if (fromBodyParameters.Count > 1)
{
var parameters = string.Join(Environment.NewLine, fromBodyParameters.Select(p => p.DisplayName));
var message = Resources.FormatApiController_MultipleBodyParametersFound(
actionModel.DisplayName,
nameof(FromQueryAttribute),
nameof(FromRouteAttribute),
nameof(FromBodyAttribute));
message += Environment.NewLine + parameters;
throw new InvalidOperationException(message);
}
}
// For any complex types that are bound from value providers, set the prefix

16
src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs
View File

@ -102,7 +102,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
private static readonly Action<ILogger, Exception> _modelStateInvalidFilterExecuting;
private static readonly Action<ILogger, MethodInfo, string, string, Exception> _inferredParameterSource;
private static readonly Action<ILogger, MethodInfo, Exception> _unableToInferParameterSources;
private static readonly Action<ILogger, IModelBinderProvider[], Exception> _registeredModelBinderProviders;
private static readonly Action<ILogger, string, Type, string, Type, Exception> _foundNoValueForPropertyInRequest;
private static readonly Action<ILogger, string, string, Type, Exception> _foundNoValueForParameterInRequest;
@ -394,11 +393,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
1,
"Inferred binding source for '{ParameterName}` on `{ActionName}` as {BindingSource}.");
_unableToInferParameterSources = LoggerMessage.Define<MethodInfo>(
LogLevel.Warning,
2,
"Unable to unambiguously infer binding sources for parameters on '{ActionName}'. More than one parameter may be inferred to bound from body.");
_unsupportedFormatFilterContentType = LoggerMessage.Define<string>(
LogLevel.Debug,
1,
@ -1124,16 +1118,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
}
}
public static void UnableToInferBindingSource(
this ILogger logger,
ActionModel actionModel)
{
if (logger.IsEnabled(LogLevel.Warning))
{
_unableToInferParameterSources(logger, actionModel.ActionMethod, null);
}
}
public static void IfMatchPreconditionFailed(this ILogger logger, EntityTagHeaderValue etag)
{
_ifMatchPreconditionFailed(logger, etag, null);

14
src/Microsoft.AspNetCore.Mvc.Core/Properties/Resources.Designer.cs generated
View File

@ -1452,6 +1452,20 @@ namespace Microsoft.AspNetCore.Mvc.Core
internal static string FormatComplexTypeModelBinder_NoParameterlessConstructor_ForParameter(object p0, object p1)
=> string.Format(CultureInfo.CurrentCulture, GetString("ComplexTypeModelBinder_NoParameterlessConstructor_ForParameter"), p0, p1);
/// <summary>
/// Action '{0}' has more than one parameter that were specified or inferred as bound from request body. Only one parameter per action may be bound from body. Inspect the following parameters, and use '{1}' to specify query string bound, '{2}' to specify route bound, and '{3}' for parameters to be bound from body:
/// </summary>
internal static string ApiController_MultipleBodyParametersFound
{
get => GetString("ApiController_MultipleBodyParametersFound");
}
/// <summary>
/// Action '{0}' has more than one parameter that were specified or inferred as bound from request body. Only one parameter per action may be bound from body. Inspect the following parameters, and use '{1}' to specify query string bound, '{2}' to specify route bound, and '{3}' for parameters to be bound from body:
/// </summary>
internal static string FormatApiController_MultipleBodyParametersFound(object p0, object p1, object p2, object p3)
=> string.Format(CultureInfo.CurrentCulture, GetString("ApiController_MultipleBodyParametersFound"), p0, p1, p2, p3);
private static string GetString(string name, params string[] formatterNames)
{
var value = _resourceManager.GetString(name);

3
src/Microsoft.AspNetCore.Mvc.Core/Resources.resx
View File

@ -439,4 +439,7 @@
<data name="ComplexTypeModelBinder_NoParameterlessConstructor_ForParameter" xml:space="preserve">
<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, give the '{1}' parameter a non-null default value.</value>
</data>
<data name="ApiController_MultipleBodyParametersFound" xml:space="preserve">
<value>Action '{0}' has more than one parameter that was specified or inferred as bound from request body. Only one parameter per action may be bound from body. Inspect the following parameters, and use '{1}' to specify bound from query, '{2}' to specify bound from route, and '{3}' for parameters to be bound from body:</value>
</data>
</root>

5
test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/ParameterModelTest.cs
View File

@ -64,6 +64,11 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels
// Ensure non-default value
Assert.NotEqual(value1, Activator.CreateInstance(property.PropertyType));
}
else if (property.Name.Equals(nameof(ActionModel.DisplayName)))
{
// DisplayName is re-calculated, hence reference equality wouldn't work.
Assert.Equal(value1, value2);
}
else
{
Assert.Same(value1, value2);

128
test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs
View File

@ -384,6 +384,101 @@ namespace Microsoft.AspNetCore.Mvc.Internal
Assert.Same(BindingSource.Body, result);
}
[Fact]
public void OnProvidersExecuting_DoesNotInferBindingSourceForParametersWithBindingInfo()
{
// Arrange
var actionName = nameof(ParameterWithBindingInfo.Action);
var provider = GetProvider();
var context = GetContext(typeof(ParameterWithBindingInfo));
// Act
provider.OnProvidersExecuting(context);
// Assert
var controllerModel = Assert.Single(context.Result.Controllers);
var actionModel = Assert.Single(controllerModel.Actions, a => a.ActionName == actionName);
var parameterModel = Assert.Single(actionModel.Parameters);
Assert.NotNull(parameterModel.BindingInfo);
Assert.Same(BindingSource.Custom, parameterModel.BindingInfo.BindingSource);
}
[Fact]
public void OnProvidersExecuting_Throws_IfMultipleParametersAreInferredAsBodyBound()
{
// Arrange
var expected =
$@"Action '{typeof(ControllerWithMultipleInferredFromBodyParameters).FullName}.{nameof(ControllerWithMultipleInferredFromBodyParameters.Action)} ({typeof(ControllerWithMultipleInferredFromBodyParameters).Assembly.GetName().Name})' " +
"has more than one parameter that was specified or inferred as bound from request body. Only one parameter per action may be bound from body. Inspect the following parameters, and use 'FromQueryAttribute' to specify bound from query, 'FromRouteAttribute' to specify bound from route, and 'FromBodyAttribute' for parameters to be bound from body:" +
Environment.NewLine + "TestModel a" +
Environment.NewLine + "Car b";
var context = GetContext(typeof(ControllerWithMultipleInferredFromBodyParameters));
var provider = GetProvider();
// Act & Assert
var ex = Assert.Throws<InvalidOperationException>(() => provider.OnProvidersExecuting(context));
Assert.Equal(expected, ex.Message);
}
[Fact]
public void OnProvidersExecuting_Throws_IfMultipleParametersAreInferredOrSpecifiedAsBodyBound()
{
// Arrange
var expected =
$@"Action '{typeof(ControllerWithMultipleInferredOrSpecifiedFromBodyParameters).FullName}.{nameof(ControllerWithMultipleInferredOrSpecifiedFromBodyParameters.Action)} ({typeof(ControllerWithMultipleInferredOrSpecifiedFromBodyParameters).Assembly.GetName().Name})' " +
"has more than one parameter that was specified or inferred as bound from request body. Only one parameter per action may be bound from body. Inspect the following parameters, and use 'FromQueryAttribute' to specify bound from query, 'FromRouteAttribute' to specify bound from route, and 'FromBodyAttribute' for parameters to be bound from body:" +
Environment.NewLine + "TestModel a" +
Environment.NewLine + "int b";
var context = GetContext(typeof(ControllerWithMultipleInferredOrSpecifiedFromBodyParameters));
var provider = GetProvider();
// Act & Assert
var ex = Assert.Throws<InvalidOperationException>(() => provider.OnProvidersExecuting(context));
Assert.Equal(expected, ex.Message);
}
[Fact]
public void OnProvidersExecuting_Throws_IfMultipleParametersAreFromBody()
{
// Arrange
var expected =
$@"Action '{typeof(ControllerWithMultipleFromBodyParameters).FullName}.{nameof(ControllerWithMultipleFromBodyParameters.Action)} ({typeof(ControllerWithMultipleFromBodyParameters).Assembly.GetName().Name})' " +
"has more than one parameter that was specified or inferred as bound from request body. Only one parameter per action may be bound from body. Inspect the following parameters, and use 'FromQueryAttribute' to specify bound from query, 'FromRouteAttribute' to specify bound from route, and 'FromBodyAttribute' for parameters to be bound from body:" +
Environment.NewLine + "decimal a" +
Environment.NewLine + "int b";
var context = GetContext(typeof(ControllerWithMultipleFromBodyParameters));
var provider = GetProvider();
// Act & Assert
var ex = Assert.Throws<InvalidOperationException>(() => provider.OnProvidersExecuting(context));
Assert.Equal(expected, ex.Message);
}
[Fact(Skip = "https://github.com/aspnet/Mvc/issues/7609")]
public void OnProvidersExecuting_PreservesBindingInfo_WhenInferringBindingSource()
{
// Arrange
var actionName = nameof(ParameterBindingController.ModelBinderAttribute);
var context = GetContext(typeof(ParameterBindingController));
var provider = GetProvider();
// Act
provider.OnProvidersExecuting(context);
// Assert
var controller = Assert.Single(context.Result.Controllers);
var action = Assert.Single(controller.Actions, a => a.ActionName == actionName);
var parameter = Assert.Single(action.Parameters);
var bindingInfo = parameter.BindingInfo;
Assert.NotNull(bindingInfo);
Assert.Same(BindingSource.Query, bindingInfo.BindingSource);
Assert.Equal("top", bindingInfo.BinderModelName);
}
[Fact]
public void InferParameterBindingSources_SetsCorrectBindingSourceForComplexTypesWithCancellationToken()
{
@ -651,6 +746,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal
[HttpPut("cancellation")]
public IActionResult ComplexTypeModelWithCancellationToken(TestModel model, CancellationToken cancellationToken) => null;
[HttpGet("parameter-with-model-binder-attribute")]
public IActionResult ModelBinderAttribute([ModelBinder(Name = "top")] int value) => null;
}
[ApiController]
@ -709,5 +807,35 @@ namespace Microsoft.AspNetCore.Mvc.Internal
public IActionResult SomeAction([FromQuery] TestModel test) => null;
}
private class Car { }
[ApiController]
private class ControllerWithMultipleInferredFromBodyParameters
{
[HttpGet("test")]
public IActionResult Action(TestModel a, Car b) => null;
}
[ApiController]
private class ControllerWithMultipleInferredOrSpecifiedFromBodyParameters
{
[HttpGet("test")]
public IActionResult Action(TestModel a, [FromBody] int b) => null;
}
[ApiController]
private class ControllerWithMultipleFromBodyParameters
{
[HttpGet("test")]
public IActionResult Action([FromBody] decimal a, [FromBody] int b) => null;
}
[ApiController]
private class ParameterWithBindingInfo
{
[HttpGet("test")]
public IActionResult Action([ModelBinder(typeof(object))] Car car) => null;
}
}
}

1
test/WebSites/BasicWebSite/Startup.cs
View File

@ -28,6 +28,7 @@ namespace BasicWebSite
// Filter that records a value in HttpContext.Items
options.Filters.Add(new TraceResourceFilter());
})
.SetCompatibilityVersion(CompatibilityVersion.Latest)
.AddXmlDataContractSerializerFormatters();
services.Configure<ApiBehaviorOptions>(options =>