Configure the page handler / controller instance as the container when validating top-level properties (#22164)

Validation attributes such as a CompareAttribute require a container to be configured in ValidationContext. This change
configures uses the controller or page handler instance that the property is being bound on when binding top-level properties.

Fixes https://github.com/dotnet/aspnetcore/issues/4895
This commit is contained in:
Pranav K 2020-06-02 17:48:56 -07:00 committed by GitHub
parent 646dfc63e4
commit 902e735b27
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 227 additions and 45 deletions

View File

@ -2685,13 +2685,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
public abstract Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationVisitor GetValidationVisitor(Microsoft.AspNetCore.Mvc.ActionContext actionContext, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.IModelValidatorProvider validatorProvider, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidatorCache validatorCache, Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider metadataProvider, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationStateDictionary validationState);
public virtual void Validate(Microsoft.AspNetCore.Mvc.ActionContext actionContext, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationStateDictionary validationState, string prefix, object model) { }
public virtual void Validate(Microsoft.AspNetCore.Mvc.ActionContext actionContext, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationStateDictionary validationState, string prefix, object model, Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata) { }
public virtual void Validate(Microsoft.AspNetCore.Mvc.ActionContext actionContext, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.ValidationStateDictionary validationState, string prefix, object model, Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, object container) { }
}
public partial class ParameterBinder
{
public ParameterBinder(Microsoft.AspNetCore.Mvc.ModelBinding.IModelMetadataProvider modelMetadataProvider, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinderFactory modelBinderFactory, Microsoft.AspNetCore.Mvc.ModelBinding.Validation.IObjectModelValidator validator, Microsoft.Extensions.Options.IOptions<Microsoft.AspNetCore.Mvc.MvcOptions> mvcOptions, Microsoft.Extensions.Logging.ILoggerFactory loggerFactory) { }
protected Microsoft.Extensions.Logging.ILogger Logger { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } }
[System.Diagnostics.DebuggerStepThroughAttribute]
public virtual System.Threading.Tasks.Task<Microsoft.AspNetCore.Mvc.ModelBinding.ModelBindingResult> BindModelAsync(Microsoft.AspNetCore.Mvc.ActionContext actionContext, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder modelBinder, Microsoft.AspNetCore.Mvc.ModelBinding.IValueProvider valueProvider, Microsoft.AspNetCore.Mvc.Abstractions.ParameterDescriptor parameter, Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, object value) { throw null; }
[System.Diagnostics.DebuggerStepThroughAttribute]
public virtual System.Threading.Tasks.ValueTask<Microsoft.AspNetCore.Mvc.ModelBinding.ModelBindingResult> BindModelAsync(Microsoft.AspNetCore.Mvc.ActionContext actionContext, Microsoft.AspNetCore.Mvc.ModelBinding.IModelBinder modelBinder, Microsoft.AspNetCore.Mvc.ModelBinding.IValueProvider valueProvider, Microsoft.AspNetCore.Mvc.Abstractions.ParameterDescriptor parameter, Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, object value, object container) { throw null; }
}
public partial class PrefixContainer
{
@ -3246,6 +3248,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
protected virtual void SuppressValidation(string key) { }
public bool Validate(Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, string key, object model) { throw null; }
public virtual bool Validate(Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, string key, object model, bool alwaysValidateAtTopLevel) { throw null; }
public virtual bool Validate(Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, string key, object model, bool alwaysValidateAtTopLevel, object container) { throw null; }
protected virtual bool ValidateNode() { throw null; }
protected virtual bool Visit(Microsoft.AspNetCore.Mvc.ModelBinding.ModelMetadata metadata, string key, object model) { throw null; }
protected virtual bool VisitChildren(Microsoft.AspNetCore.Mvc.ModelBinding.Validation.IValidationStrategy strategy) { throw null; }

View File

@ -84,7 +84,8 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
valueProvider,
parameter,
modelMetadata,
value: null);
value: null,
container: null); // Parameters do not have containers.
if (result.IsModelSet)
{
@ -110,7 +111,8 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
valueProvider,
property,
modelMetadata,
value: null);
value: null,
container: controller);
if (result.IsModelSet)
{

View File

@ -77,6 +77,28 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
string prefix,
object model,
ModelMetadata metadata)
=> Validate(actionContext, validationState, prefix, model, metadata, container: null);
/// <summary>
/// Validates the provided object model.
/// If <paramref name="model"/> is <see langword="null"/> and the <paramref name="metadata"/>'s
/// <see cref="ModelMetadata.IsRequired"/> is <see langword="true"/>, will add one or more
/// model state errors that <see cref="Validate(ActionContext, ValidationStateDictionary, string, object)"/>
/// would not.
/// </summary>
/// <param name="actionContext">The <see cref="ActionContext"/>.</param>
/// <param name="validationState">The <see cref="ValidationStateDictionary"/>.</param>
/// <param name="prefix">The model prefix key.</param>
/// <param name="model">The model object.</param>
/// <param name="metadata">The <see cref="ModelMetadata"/>.</param>
/// <param name="container">The model container</param>
public virtual void Validate(
ActionContext actionContext,
ValidationStateDictionary validationState,
string prefix,
object model,
ModelMetadata metadata,
object container)
{
var visitor = GetValidationVisitor(
actionContext,
@ -85,7 +107,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
_modelMetadataProvider,
validationState);
visitor.Validate(metadata, prefix, model, alwaysValidateAtTopLevel: metadata.IsRequired);
visitor.Validate(metadata, prefix, model, alwaysValidateAtTopLevel: metadata.IsRequired, container);
}
/// <summary>

View File

@ -82,13 +82,34 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
/// <param name="metadata">The <see cref="ModelMetadata"/>.</param>
/// <param name="value">The initial model value.</param>
/// <returns>The result of model binding.</returns>
public virtual async Task<ModelBindingResult> BindModelAsync(
public virtual Task<ModelBindingResult> BindModelAsync(
ActionContext actionContext,
IModelBinder modelBinder,
IValueProvider valueProvider,
ParameterDescriptor parameter,
ModelMetadata metadata,
object value)
=> BindModelAsync(actionContext, modelBinder, valueProvider, parameter, metadata, value, container: null).AsTask();
/// <summary>
/// Binds a model specified by <paramref name="parameter"/> using <paramref name="value"/> as the initial value.
/// </summary>
/// <param name="actionContext">The <see cref="ActionContext"/>.</param>
/// <param name="modelBinder">The <see cref="IModelBinder"/>.</param>
/// <param name="valueProvider">The <see cref="IValueProvider"/>.</param>
/// <param name="parameter">The <see cref="ParameterDescriptor"/></param>
/// <param name="metadata">The <see cref="ModelMetadata"/>.</param>
/// <param name="value">The initial model value.</param>
/// <param name="container">The container for the model.</param>
/// <returns>The result of model binding.</returns>
public virtual async ValueTask<ModelBindingResult> BindModelAsync(
ActionContext actionContext,
IModelBinder modelBinder,
IValueProvider valueProvider,
ParameterDescriptor parameter,
ModelMetadata metadata,
object value,
object container)
{
if (actionContext == null)
{
@ -164,7 +185,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
parameter,
metadata,
modelBindingContext,
modelBindingResult);
modelBindingResult,
container);
Logger.DoneAttemptingToValidateParameterOrProperty(parameter, metadata);
}
@ -192,7 +214,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
ParameterDescriptor parameter,
ModelMetadata metadata,
ModelBindingContext modelBindingContext,
ModelBindingResult modelBindingResult)
ModelBindingResult modelBindingResult,
object container)
{
RecalculateModelMetadata(parameter, modelBindingResult, ref metadata);
@ -211,7 +234,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
modelBindingContext.ValidationState,
modelBindingContext.ModelName,
modelBindingResult.Model,
metadata);
metadata,
container);
}
else if (metadata.IsRequired)
{
@ -240,7 +264,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
modelBindingContext.ValidationState,
modelName,
modelBindingResult.Model,
metadata);
metadata,
container);
}
}

View File

@ -133,7 +133,24 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
/// <param name="alwaysValidateAtTopLevel">If <c>true</c>, applies validation rules even if the top-level value is <c>null</c>.</param>
/// <returns><c>true</c> if the object is valid, otherwise <c>false</c>.</returns>
public virtual bool Validate(ModelMetadata metadata, string key, object model, bool alwaysValidateAtTopLevel)
=> Validate(metadata, key, model, alwaysValidateAtTopLevel, container: null);
/// <summary>
/// Validates a object.
/// </summary>
/// <param name="metadata">The <see cref="ModelMetadata"/> associated with the model.</param>
/// <param name="key">The model prefix key.</param>
/// <param name="model">The model object.</param>
/// <param name="alwaysValidateAtTopLevel">If <c>true</c>, applies validation rules even if the top-level value is <c>null</c>.</param>
/// <param name="container">The model container.</param>
/// <returns><c>true</c> if the object is valid, otherwise <c>false</c>.</returns>
public virtual bool Validate(ModelMetadata metadata, string key, object model, bool alwaysValidateAtTopLevel, object container)
{
if (container != null && metadata.MetadataKind != ModelMetadataKind.Property)
{
throw new ArgumentException(Resources.FormatValidationVisitor_ContainerCannotBeSpecified(metadata.MetadataKind));
}
if (model == null && key != null && !alwaysValidateAtTopLevel)
{
var entry = ModelState[key];
@ -148,6 +165,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
return true;
}
// Container is non-null only when validation top-level properties. Start off by treating "container" as the "Model" instance.
// Invoking StateManager.Recurse later in this invocation will result in it being correctly used as the container instance during the
// validation of "model".
Model = container;
return Visit(metadata, key, model);
}

View File

@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema
Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.
Example:
... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.
mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.
mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
@ -516,4 +516,7 @@
<data name="FailedToReadRequestForm" xml:space="preserve">
<value>Failed to read the request form. {0}</value>
</data>
<data name="ValidationVisitor_ContainerCannotBeSpecified" xml:space="preserve">
<value>A container cannot be specified when the ModelMetada is of kind '{0}'.</value>
</data>
</root>

View File

@ -1151,8 +1151,9 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
It.IsAny<IValueProvider>(),
It.IsAny<ParameterDescriptor>(),
It.IsAny<ModelMetadata>(),
null,
null))
.Returns((ActionContext context, IModelBinder modelBinder, IValueProvider valueProvider, ParameterDescriptor descriptor, ModelMetadata metadata, object v) =>
.Returns((ActionContext context, IModelBinder modelBinder, IValueProvider valueProvider, ParameterDescriptor descriptor, ModelMetadata metadata, object v, object c) =>
{
ModelBindingResult result;
if (descriptor.Name == "accountId")
@ -1172,7 +1173,7 @@ namespace Microsoft.AspNetCore.Mvc.Controllers
result = ModelBindingResult.Failed();
}
return Task.FromResult(result);
return new ValueTask<ModelBindingResult>(result);
});
var controllerContext = GetControllerContext(actionDescriptor);

View File

@ -78,7 +78,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
valueProvider,
property,
modelMetadata,
value: null);
value: null,
container: instance);
if (result.IsModelSet)
{
@ -159,7 +160,8 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
valueProvider,
parameter,
modelMetadata,
value: null);
value: null,
container: null); // Parameters do not have containers.
if (result.IsModelSet)
{

View File

@ -827,22 +827,23 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
public IList<ParameterDescriptor> Descriptors { get; } = new List<ParameterDescriptor>();
public override Task<ModelBindingResult> BindModelAsync(
public override ValueTask<ModelBindingResult> BindModelAsync(
ActionContext actionContext,
IModelBinder modelBinder,
IValueProvider valueProvider,
ParameterDescriptor parameter,
ModelMetadata metadata,
object value)
object value,
object container)
{
Descriptors.Add(parameter);
if (_args.TryGetValue(parameter.Name, out var result))
{
return Task.FromResult(ModelBindingResult.Success(result));
return new ValueTask<ModelBindingResult>(ModelBindingResult.Success(result));
}
return Task.FromResult(ModelBindingResult.Failed());
return new ValueTask<ModelBindingResult>(ModelBindingResult.Failed());
}
}

View File

@ -3,13 +3,15 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Net.Http.Json;
using System.Reflection;
using System.Text.Json;
using System.Threading.Tasks;
using BasicWebSite.Models;
using System.Text.Json;
using Xunit;
namespace Microsoft.AspNetCore.Mvc.FunctionalTests
@ -627,6 +629,30 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests
Assert.Equal("OnGetTestName", content);
}
[Fact]
public async Task BindPropertiesAppliesValidation()
{
// Act
var response = await Client.GetAsync("BindPropertiesWithValidation/Action?Password=Test&ConfirmPassword=different");
// Assert
await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest);
var problem = await response.Content.ReadFromJsonAsync<ValidationProblemDetails>();
Assert.Collection(
problem.Errors.OrderBy(e => e.Key),
kvp =>
{
Assert.Equal("ConfirmPassword", kvp.Key);
Assert.Equal("Password and confirm password do not match.", Assert.Single(kvp.Value));
},
kvp =>
{
Assert.Equal("UserName", kvp.Key);
Assert.Equal("User name is required.", Assert.Single(kvp.Value));
});
}
[Fact]
public async Task InvalidForm_ResultsInModelError()
{

View File

@ -510,6 +510,17 @@ Hello from /Pages/Shared/";
Assert.Contains("18 &#x2264; Age &#x2264; 60", response);
}
[Fact]
public async Task CompareValidationAttributes_OnTopLevelProperties()
{
// Act
var response = await Client.GetStringAsync("/Validation/PageWithCompareValidation?password=test&comparePassword=different");
// Assert
Assert.Contains("User name is required", response);
Assert.Contains("Password and confirm password do not match.", response);
}
[Fact]
public async Task ValidationAttributes_OnHandlerParameters()
{

View File

@ -0,0 +1,35 @@
// 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.ComponentModel.DataAnnotations;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ModelBinding;
namespace BasicWebSite
{
[BindProperties(SupportsGet = true)]
public class BindPropertiesWithValidationController : Controller
{
[Required(ErrorMessage = "User name is required.")]
public string UserName { get; set; }
[Required]
public string Password { get; set; }
[Compare(nameof(Password), ErrorMessage = "Password and confirm password do not match.")]
public string ConfirmPassword { get; set; }
[BindNever]
public string BindNeverProperty { get; set; }
public IActionResult Action()
{
if (!ModelState.IsValid)
{
return ValidationProblem();
}
return Ok();
}
}
}

View File

@ -0,0 +1,27 @@
// 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.ComponentModel.DataAnnotations;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.RazorPages;
namespace RazorPagesWebSite
{
public class PageWithCompareValidation : PageModel
{
[BindProperty(SupportsGet = true)]
[Required(ErrorMessage = "User name is required.")]
public string UserName { get; set; }
[BindProperty(SupportsGet = true)]
[Required(ErrorMessage = "Password is required.")]
public string Password { get; set; }
[BindProperty(SupportsGet = true)]
[Compare(nameof(Password), ErrorMessage = "Password and confirm password do not match.")]
public int ConfirmPassword { get; set; }
[Required] // Here to make sure we do not validate an unbound property.
public string NotBoundProperty { get; set; }
}
}

View File

@ -0,0 +1,3 @@
@page
@model PageWithCompareValidation
<div asp-validation-summary="All"></div>