[Fixes #3874] Null passed as arguments to controller method parameters when no InputFormatter matches

* Add an UnsupportedContentType to the ModelState dictionary when no formatter can read the body.
* Add a filter to the pipeline that searches for that specific exception and transforms the response into 415.
This commit is contained in:
jacalvar 2016-01-19 16:11:20 -08:00
parent 8753c60b43
commit 7cbb263edb
15 changed files with 332 additions and 114 deletions

View File

@ -48,6 +48,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal
options.ModelBinders.Add(new GenericModelBinder());
options.ModelBinders.Add(new MutableObjectModelBinder());
// Set up filters
options.Filters.Add(new UnsupportedContentTypeFilter());
// Set up default output formatters.
options.OutputFormatters.Add(new HttpNoContentOutputFormatter());
options.OutputFormatters.Add(new StringOutputFormatter());

View File

@ -84,9 +84,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
if (formatter == null)
{
var unsupportedContentType = Resources.FormatUnsupportedContentType(
var message = Resources.FormatUnsupportedContentType(
bindingContext.OperationBindingContext.HttpContext.Request.ContentType);
bindingContext.ModelState.AddModelError(modelBindingKey, unsupportedContentType);
var exception = new UnsupportedContentTypeException(message);
bindingContext.ModelState.AddModelError(modelBindingKey, exception, bindingContext.ModelMetadata);
// This model binder is the only handler for the Body binding source and it cannot run twice. Always
// tell the model binding system to skip other model binders and never to fall back i.e. indicate a

View File

@ -0,0 +1,29 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using Microsoft.AspNetCore.Mvc.Formatters;
namespace Microsoft.AspNetCore.Mvc.ModelBinding
{
/// <summary>
/// The <see cref="Exception"/> that is added to model state when a model binder for the body of the request is
/// unable to understand the request content type header.
/// </summary>
public class UnsupportedContentTypeException : Exception
{
/// <summary>
/// Creates a new instance of <see cref="UnsupportedContentTypeException"/> with the specified
/// exception <paramref name="message"/>.
/// </summary>
/// <param name="message">The message that describes the error.</param>
public UnsupportedContentTypeException(string message)
: base(message)
{
if (message == null)
{
throw new ArgumentNullException(nameof(message));
}
}
}
}

View File

@ -0,0 +1,55 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.Net.Http.Headers;
namespace Microsoft.AspNetCore.Mvc.ModelBinding
{
/// <summary>
/// A filter that scans for <see cref="UnsupportedContentTypeException"/> in the
/// <see cref="ActionContext.ModelState"/> and shortcircuits the pipeline
/// with an Unsupported Media Type (415) response.
/// </summary>
public class UnsupportedContentTypeFilter : IActionFilter
{
/// <inheritdoc />
public void OnActionExecuting(ActionExecutingContext context)
{
if (HasUnsupportedContentTypeError(context))
{
context.Result = new UnsupportedMediaTypeResult();
}
}
private bool HasUnsupportedContentTypeError(ActionExecutingContext context)
{
var modelState = context.ModelState;
if (modelState.IsValid)
{
return false;
}
foreach (var kvp in modelState)
{
var errors = kvp.Value.Errors;
for (int i = 0; i < errors.Count; i++)
{
var error = errors[i];
if (error.Exception is UnsupportedContentTypeException)
{
return true;
}
}
}
return false;
}
/// <inheritdoc />
public void OnActionExecuted(ActionExecutedContext context)
{
}
}
}

View File

@ -0,0 +1,132 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
using System.Collections.Generic;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Internal;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Filters;
using Microsoft.AspNetCore.Routing;
using Xunit;
namespace Microsoft.AspNetCore.Mvc.ModelBinding
{
public class UnsupportedContentTypeFilterTest
{
[Fact]
public void OnActionExecuting_ChangesActionResult_IfUnsupportedContentTypeExceptionIsFoundOnModelState()
{
// Arrange
var context = new ActionExecutingContext(
new ActionContext
{
HttpContext = new DefaultHttpContext(),
RouteData = new RouteData(),
ActionDescriptor = new ActionDescriptor()
},
new List<IFilterMetadata>(),
new Dictionary<string, object>(),
new object());
var modelMetadata = new EmptyModelMetadataProvider()
.GetMetadataForType(typeof(int));
context.ModelState.AddModelError(
"person.body",
new UnsupportedContentTypeException("error"),
modelMetadata);
var filter = new UnsupportedContentTypeFilter();
// Act
filter.OnActionExecuting(context);
// Assert
Assert.NotNull(context.Result);
var status = Assert.IsType<UnsupportedMediaTypeResult>(context.Result);
}
[Fact]
public void OnActionExecuting_DoesNotChangeActionResult_IfOtherErrorsAreFoundOnModelState()
{
// Arrange
var context = new ActionExecutingContext(
new ActionContext
{
HttpContext = new DefaultHttpContext(),
RouteData = new RouteData(),
ActionDescriptor = new ActionDescriptor()
},
new List<IFilterMetadata>(),
new Dictionary<string, object>(),
new object());
context.ModelState.AddModelError("person.body", "Some error");
var filter = new UnsupportedContentTypeFilter();
// Act
filter.OnActionExecuting(context);
// Assert
Assert.Null(context.Result);
}
[Fact]
public void OnActionExecuting_DoesNotChangeActionResult_IfModelStateIsValid()
{
// Arrange
var context = new ActionExecutingContext(
new ActionContext
{
HttpContext = new DefaultHttpContext(),
RouteData = new RouteData(),
ActionDescriptor = new ActionDescriptor()
},
new List<IFilterMetadata>(),
new Dictionary<string, object>(),
new object());
var filter = new UnsupportedContentTypeFilter();
// Act
filter.OnActionExecuting(context);
// Assert
Assert.Null(context.Result);
}
[Fact]
public void OnActionExecuting_DoesNotChangeActionResult_IfOtherExceptionsAreFoundOnModelState()
{
// Arrange
var context = new ActionExecutingContext(
new ActionContext
{
HttpContext = new DefaultHttpContext(),
RouteData = new RouteData(),
ActionDescriptor = new ActionDescriptor()
},
new List<IFilterMetadata>(),
new Dictionary<string, object>(),
new object());
var modelMetadata = new EmptyModelMetadataProvider()
.GetMetadataForType(typeof(int));
context.ModelState.AddModelError(
"person.body",
new Exception("error"),
modelMetadata);
var filter = new UnsupportedContentTypeFilter();
// Act
filter.OnActionExecuting(context);
// Assert
Assert.Null(context.Result);
}
}
}

View File

@ -203,7 +203,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
// Key is empty because this was a top-level binding.
var entry = Assert.Single(bindingContext.ModelState);
Assert.Equal(string.Empty, entry.Key);
var errorMessage = Assert.Single(entry.Value.Errors).ErrorMessage;
var errorMessage = Assert.Single(entry.Value.Errors).Exception.Message;
Assert.Equal("Unsupported content type 'text/xyz'.", errorMessage);
}

View File

@ -32,11 +32,11 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
// Act
var response = await Client.SendAsync(request);
var product = JsonConvert.DeserializeObject<Product>(await response.Content.ReadAsStringAsync());
var body = await response.Content.ReadAsStringAsync();
// Assert
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
Assert.Null(product);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("CreateProduct_Product_Text", body);
}
[Fact]
@ -49,11 +49,11 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
// Act
var response = await Client.SendAsync(request);
var product = JsonConvert.DeserializeObject<Product>(await response.Content.ReadAsStringAsync());
var body = await response.Content.ReadAsStringAsync();
// Assert
Assert.Equal(HttpStatusCode.NoContent, response.StatusCode);
Assert.Null(product);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("ConsumesAttribute_PassThrough_Product_Json", body);
}
[Theory]

View File

@ -552,7 +552,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
"<SampleInt>10</SampleInt>" +
"</DummyClass>";
// There's nothing that can deserialize the body, so the result contains the default value.
// There's nothing that can deserialize the body, so the result is UnsupportedMediaType.
var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/Json");
request.Content = new StringContent(input, Encoding.UTF8, "application/xml");
@ -560,8 +560,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
var response = await Client.SendAsync(request);
// Assert
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("\"0\"", await response.Content.ReadAsStringAsync());
Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode);
}
}
}

View File

@ -59,47 +59,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(sampleInputInt.ToString(), await response.Content.ReadAsStringAsync());
}
[Theory]
[InlineData("", true)]
[InlineData(null, true)]
[InlineData("invalid", true)]
[InlineData("application/custom", true)]
[InlineData("image/jpg", true)]
[InlineData("", false)]
[InlineData(null, false)]
[InlineData("invalid", false)]
[InlineData("application/custom", false)]
[InlineData("image/jpg", false)]
public async Task ModelStateErrorValidation_NoInputFormatterFound_ForGivenContentType(
string requestContentType,
bool filterHandlesModelStateError)
{
// Arrange
var actionName = filterHandlesModelStateError ? "ActionFilterHandlesError" : "ActionHandlesError";
var expectedSource = filterHandlesModelStateError ? "filter" : "action";
var input = "{\"SampleInt\":10}";
var content = new StringContent(input);
content.Headers.Clear();
content.Headers.TryAddWithoutValidation("Content-Type", requestContentType);
// Act
var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/InputFormatter/" + actionName);
request.Headers.Accept.Add(MediaTypeWithQualityHeaderValue.Parse("application/json"));
request.Content = content;
var response = await Client.SendAsync(request);
var responseBody = await response.Content.ReadAsStringAsync();
var result = JsonConvert.DeserializeObject<FormatterWebSite.ErrorInfo>(responseBody);
// Assert
Assert.Equal(1, result.Errors.Count);
Assert.Equal("Unsupported content type '" + requestContentType + "'.",
result.Errors[0]);
Assert.Equal(actionName, result.ActionName);
Assert.Equal("dummy", result.ParameterName);
Assert.Equal(expectedSource, result.Source);
}
[Theory]
[InlineData("application/json", "{\"SampleInt\":10}", 10)]
[InlineData("application/json", "{}", 0)]
@ -154,13 +113,11 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(expected, responseBody);
}
[Theory]
[InlineData("{\"SampleInt\":10}")]
[InlineData("{}")]
[InlineData("")]
public async Task JsonInputFormatter_IsModelStateInvalid_ForEmptyContentType(string jsonInput)
[Fact]
public async Task JsonInputFormatter_Returns415UnsupportedMediaType_ForEmptyContentType()
{
// Arrange
var jsonInput = "{\"SampleInt\":10}";
var content = new StringContent(jsonInput, Encoding.UTF8, "application/json");
content.Headers.Clear();
@ -169,7 +126,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
var responseBody = await response.Content.ReadAsStringAsync();
// Assert
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode);
}
[Theory]
@ -225,7 +182,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
var responseBody = await response.Content.ReadAsStringAsync();
// Assert
Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode);
Assert.Equal(HttpStatusCode.UnsupportedMediaType, response.StatusCode);
}
}
}

View File

@ -3,6 +3,7 @@
using System.Net;
using System.Net.Http;
using System.Text;
using System.Threading.Tasks;
using Newtonsoft.Json;
using Xunit;
@ -290,9 +291,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
[InlineData("GET", "api/Admin/Test?name=mario&ssn=123456", "GetUserByNameAndSsn")]
[InlineData("GET", "api/Admin/Test?name=mario&ssn=123456&age=3", "GetUserByNameAgeAndSsn")]
[InlineData("GET", "api/Admin/Test/5?random=9", "GetUser")]
[InlineData("POST", "api/Admin/Test", "PostUser")]
[InlineData("POST", "api/Admin/Test?name=mario&age=10", "PostUserByNameAndAge")]
// Note: Normally the following would not match DeleteUserByIdAndOptName because it has 'id' and 'age' as parameters while the DeleteUserByIdAndOptName action has 'id' and 'name'.
// However, because the default value is provided on action parameter 'name', having the 'id' in the request was enough to match the action.
[InlineData("DELETE", "api/Admin/Test/6?age=10", "DeleteUserByIdAndOptName")]
@ -328,6 +326,25 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(expectedActionName, result.ActionName);
}
[Theory]
[InlineData("api/Admin/Test", "PostUser")]
[InlineData("api/Admin/Test?name=mario&age=10", "PostUserByNameAndAge")]
public async Task LegacyActionSelection_OverloadedAction_WithUnnamedAction(string requestUrl, string expectedActionName)
{
// Arrange
var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/" + requestUrl);
request.Content = new StringContent("{}", Encoding.UTF8, "application/json");
// Act
var response = await Client.SendAsync(request);
// Assert
var data = Assert.Single(response.Headers.GetValues("ActionSelection"));
var result = JsonConvert.DeserializeObject<ActionSelectionResult>(data);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal(expectedActionName, result.ActionName);
}
[Theory]
[InlineData("GET", "api/Store/Test", "GetUsers")]
[InlineData("GET", "api/Store/Test/2", "GetUsersByName")]
@ -378,7 +395,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
[InlineData("GET", "api/Blog/Test/GetUser/4?id=3", "GetUser")]
[InlineData("GET", "api/Blog/Test/GetUserByNameAgeAndSsn?name=user&age=90&ssn=123456789", "GetUserByNameAgeAndSsn")]
[InlineData("GET", "api/Blog/Test/GetUserByNameAndSsn?name=user&ssn=123456789", "GetUserByNameAndSsn")]
[InlineData("POST", "api/Blog/Test/PostUserByNameAndAddress?name=user", "PostUserByNameAndAddress")]
public async Task LegacyActionSelection_RouteWithActionName(string httpMethod, string requestUrl, string expectedActionName)
{
// Arrange
@ -394,6 +410,25 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(expectedActionName, result.ActionName);
}
[Fact]
public async Task LegacyActionSelection_RouteWithActionName()
{
// Arrange
var request = new HttpRequestMessage(
HttpMethod.Post,
"http://localhost/api/Blog/Test/PostUserByNameAndAddress?name=user");
request.Content = new StringContent("{}", Encoding.UTF8, "application/json");
// Act
var response = await Client.SendAsync(request);
// Assert
var data = Assert.Single(response.Headers.GetValues("ActionSelection"));
var result = JsonConvert.DeserializeObject<ActionSelectionResult>(data);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("PostUserByNameAndAddress", result.ActionName);
}
[Theory]
[InlineData("GET", "api/Blog/Test/getusers", "GetUsers")]
[InlineData("GET", "api/Blog/Test/getuseR/1", "GetUser")]
@ -401,7 +436,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
[InlineData("GET", "api/Blog/Test/GetUser/4?Id=3", "GetUser")]
[InlineData("GET", "api/Blog/Test/GetUserByNameAgeandSsn?name=user&age=90&ssn=123456789", "GetUserByNameAgeAndSsn")]
[InlineData("GET", "api/Blog/Test/getUserByNameAndSsn?name=user&ssn=123456789", "GetUserByNameAndSsn")]
[InlineData("POST", "api/Blog/Test/PostUserByNameAndAddress?name=user", "PostUserByNameAndAddress")]
public async Task LegacyActionSelection_RouteWithActionName_Casing(string httpMethod, string requestUrl, string expectedActionName)
{
// Arrange
@ -417,6 +451,25 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(expectedActionName, result.ActionName);
}
[Fact]
public async Task LegacyActionSelection_RouteWithActionName_Casing()
{
// Arrange
var request = new HttpRequestMessage(
HttpMethod.Post,
"http://localhost/api/Blog/Test/PostUserByNameAndAddress?name=user");
request.Content = new StringContent("{}", Encoding.UTF8, "application/json");
// Act
var response = await Client.SendAsync(request);
// Assert
var data = Assert.Single(response.Headers.GetValues("ActionSelection"));
var result = JsonConvert.DeserializeObject<ActionSelectionResult>(data);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal("PostUserByNameAndAddress", result.ActionName);
}
[Theory]
[InlineData("GET", "api/Admin/Test", "GetUsers")]
[InlineData("GET", "api/Admin/Test/?name=peach", "GetUsersByName")]
@ -438,19 +491,36 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal(expectedActionName, result.ActionName);
}
[Theory]
[InlineData("GET", "api/Admin/ParameterAttribute/2", "GetUser")]
[InlineData("GET", "api/Admin/ParameterAttribute?id=2", "GetUser")]
[InlineData("GET", "api/Admin/ParameterAttribute?myId=2", "GetUserByMyId")]
[InlineData("POST", "api/Admin/ParameterAttribute/3?name=user", "PostUserNameFromUri")]
[InlineData("POST", "api/Admin/ParameterAttribute/3", "PostUserNameFromBody")]
[InlineData("DELETE", "api/Admin/ParameterAttribute/3?name=user", "DeleteUserWithNullableIdAndName")]
[InlineData("DELETE", "api/Admin/ParameterAttribute?address=userStreet", "DeleteUser")]
public async Task LegacyActionSelection_ModelBindingParameterAttribute_AreAppliedWhenSelectingActions(string httpMethod, string requestUrl, string expectedActionName)
{
// Arrange
var request = new HttpRequestMessage(new HttpMethod(httpMethod), "http://localhost/" + requestUrl);
request.Content = new StringContent("{}", Encoding.UTF8, "application/json");
// Act
var response = await Client.SendAsync(request);
// Assert
var data = Assert.Single(response.Headers.GetValues("ActionSelection"));
var result = JsonConvert.DeserializeObject<ActionSelectionResult>(data);
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Equal(expectedActionName, result.ActionName);
}
[Theory]
[InlineData("api/Admin/ParameterAttribute/3?name=user", "PostUserNameFromUri")]
[InlineData("api/Admin/ParameterAttribute/3", "PostUserNameFromBody")]
public async Task LegacyActionSelection_Post_ModelBindingParameterAttribute_AreAppliedWhenSelectingActions(string requestUrl, string expectedActionName)
{
// Arrange
var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/" + requestUrl);
request.Content = new StringContent("{}", Encoding.UTF8, "application/json");
// Act
var response = await Client.SendAsync(request);

View File

@ -10,9 +10,9 @@ namespace BasicWebSite.Controllers.ActionConstraints
public class ConsumesAttribute_PassThroughController : Controller
{
[Consumes("application/json")]
public Product CreateProduct([FromBody] Product_Json jsonInput)
public IActionResult CreateProduct(Product_Json jsonInput)
{
return jsonInput;
return Content("ConsumesAttribute_PassThrough_Product_Json");
}
}
}

View File

@ -10,20 +10,22 @@ namespace BasicWebSite.Controllers.ActionConstraints
public class ConsumesAttribute_WithFallbackActionController : Controller
{
[Consumes("application/json")]
public Product CreateProduct([FromBody] Product_Json jsonInput)
[ActionName("CreateProduct")]
public IActionResult CreateProductJson()
{
return jsonInput;
return Content("CreateProduct_Product_Json");
}
[Consumes("application/xml")]
public Product CreateProduct([FromBody] Product_Xml xmlInput)
[ActionName("CreateProduct")]
public IActionResult CreateProductXml()
{
return xmlInput;
return Content("CreateProduct_Product_Xml");
}
public Product CreateProduct([FromBody] Product_Text defaultInput)
public IActionResult CreateProduct()
{
return defaultInput;
return Content("CreateProduct_Product_Text");
}
}
}

View File

@ -44,12 +44,14 @@ namespace FiltersWebSite.Controllers
public void OnResultExecuting(ResultExecutingContext context)
{
var options = context.HttpContext.RequestServices.GetRequiredService<IOptions<MvcOptions>>();
var jsonFormatter = options.Value.OutputFormatters.OfType<JsonOutputFormatter>().Single();
// Update the output formatter collection to only return JSON.
var result = (ObjectResult)context.Result;
result.Formatters.Add(jsonFormatter);
if (context.Result is ObjectResult)
{
var options = context.HttpContext.RequestServices.GetRequiredService<IOptions<MvcOptions>>();
var jsonFormatter = options.Value.OutputFormatters.OfType<JsonOutputFormatter>().Single();
var result = (ObjectResult)context.Result;
result.Formatters.Add(jsonFormatter);
}
}
}
}

View File

@ -9,35 +9,6 @@ namespace FormatterWebSite.Controllers
{
public class InputFormatterController : Controller
{
[HttpPost]
public object ActionHandlesError([FromBody] DummyClass dummy)
{
if (!ModelState.IsValid)
{
// Body model binder normally reports errors for parameters using the empty name.
var parameterBindingErrors = ModelState["dummy"]?.Errors ?? ModelState[string.Empty]?.Errors;
if (parameterBindingErrors != null && parameterBindingErrors.Count != 0)
{
return new ErrorInfo
{
ActionName = "ActionHandlesError",
ParameterName = "dummy",
Errors = parameterBindingErrors.Select(x => x.ErrorMessage).ToList(),
Source = "action"
};
}
}
return dummy;
}
[HttpPost]
[ValidateBodyParameter]
public object ActionFilterHandlesError([FromBody] DummyClass dummy)
{
return dummy;
}
public IActionResult ReturnInput([FromBody] string test)
{
if (!ModelState.IsValid)

View File

@ -33,10 +33,6 @@ namespace FormatterWebSite.Controllers
[HttpPost]
public IActionResult ReturnInput([FromBody]DummyClass dummyObject)
{
if (!ModelState.IsValid)
{
return new HttpStatusCodeResult(StatusCodes.Status400BadRequest);
}
return Content(dummyObject.SampleInt.ToString());
}