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

Do not suppress `ModelValidationState.Invalid` entries 

- #7992, #7963
This commit is contained in:
Doug Bunting 2018-07-12 11:21:21 -07:00
parent dfbdb37979
commit f2608c2ff4
No known key found for this signature in database
GPG Key ID: 888B4EB7822B32E9
3 changed files with 193 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs
View File

@ -296,7 +296,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
var entries = ModelState.FindKeysWithPrefix(key);
foreach (var entry in entries)
{
entry.Value.ValidationState = ModelValidationState.Skipped;
if (entry.Value.ValidationState != ModelValidationState.Invalid)
{
entry.Value.ValidationState = ModelValidationState.Skipped;
}
}
}

63
test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs
View File

@ -133,6 +133,27 @@ namespace Microsoft.AspNetCore.Mvc.Internal
Assert.Empty(entry.Errors);
}
// More like how product code does suppressions than Validate_SimpleType_SuppressValidation()
[Fact]
public void Validate_SimpleType_SuppressValidationWithNullKey()
{
// Arrange
var actionContext = new ActionContext();
var modelState = actionContext.ModelState;
var validator = CreateValidator();
var model = "test";
var validationState = new ValidationStateDictionary
{
{ model, new ValidationStateEntry { SuppressValidation = true } }
};
// Act
validator.Validate(actionContext, validationState, "parameter", model);
// Assert
Assert.True(modelState.IsValid);
Assert.Empty(modelState);
}
[Fact]
public void Validate_ComplexValueType_Valid()
@ -1195,6 +1216,48 @@ namespace Microsoft.AspNetCore.Mvc.Internal
Assert.Empty(entry.Value.Errors);
}
// Regression test for aspnet/Mvc#7992
[Fact]
public void Validate_SuppressValidation_AfterHasReachedMaxErrors_Invalid()
{
// Arrange
var actionContext = new ActionContext();
var modelState = actionContext.ModelState;
modelState.MaxAllowedErrors = 2;
modelState.AddModelError(key: "one", errorMessage: "1");
modelState.AddModelError(key: "two", errorMessage: "2");
var validator = CreateValidator();
var model = (object)23; // Box ASAP
var validationState = new ValidationStateDictionary
{
{ model, new ValidationStateEntry { SuppressValidation = true } }
};
// Act
validator.Validate(actionContext, validationState, prefix: string.Empty, model);
// Assert
Assert.False(modelState.IsValid);
Assert.True(modelState.HasReachedMaxErrors);
Assert.Collection(
modelState,
kvp =>
{
Assert.Empty(kvp.Key);
Assert.Equal(ModelValidationState.Invalid, kvp.Value.ValidationState);
var error = Assert.Single(kvp.Value.Errors);
Assert.IsType<TooManyModelErrorsException>(error.Exception);
},
kvp =>
{
Assert.Equal("one", kvp.Key);
Assert.Equal(ModelValidationState.Invalid, kvp.Value.ValidationState);
var error = Assert.Single(kvp.Value.Errors);
Assert.Equal("1", error.ErrorMessage);
});
}
private static DefaultObjectValidator CreateValidator(Type excludedType)
{
var excludeFilters = new List<SuppressChildValidationMetadataProvider>();

148
test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs
View File

@ -8,6 +8,7 @@ using System.Linq;
using System.Reflection;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.JsonPatch;
using Microsoft.AspNetCore.Mvc.Abstractions;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Mvc.DataAnnotations;
@ -706,6 +707,131 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
});
}
// Regression test 1 for aspnet/Mvc#7963. ModelState should never be valid.
[Fact]
public async Task BindModelAsync_ForOverlappingParametersWithSuppressions_InValid_WithValidSecondParameter()
{
// Arrange
var parameterDescriptor = new ParameterDescriptor
{
Name = "patchDocument",
ParameterType = typeof(IJsonPatchDocument),
};
var actionContext = GetControllerContext();
var modelState = actionContext.ModelState;
// First ModelState key is not empty to match SimpleTypeModelBinder.
modelState.SetModelValue("id", "notAGuid", "notAGuid");
modelState.AddModelError("id", "This is not valid.");
var modelMetadataProvider = new TestModelMetadataProvider();
modelMetadataProvider.ForType<IJsonPatchDocument>().ValidationDetails(v => v.ValidateChildren = false);
var modelMetadata = modelMetadataProvider.GetMetadataForType(typeof(IJsonPatchDocument));
var parameterBinder = new ParameterBinder(
modelMetadataProvider,
Mock.Of<IModelBinderFactory>(),
new DefaultObjectValidator(
modelMetadataProvider,
new[] { TestModelValidatorProvider.CreateDefaultProvider() }),
_optionsAccessor,
NullLoggerFactory.Instance);
// BodyModelBinder does not update ModelState in success case.
var modelBindingResult = ModelBindingResult.Success(new JsonPatchDocument());
var modelBinder = CreateMockModelBinder(modelBindingResult);
// Act
var result = await parameterBinder.BindModelAsync(
actionContext,
modelBinder,
new SimpleValueProvider(),
parameterDescriptor,
modelMetadata,
value: null);
// Assert
Assert.True(result.IsModelSet);
Assert.False(modelState.IsValid);
Assert.Collection(
modelState,
kvp =>
{
Assert.Equal("id", kvp.Key);
Assert.Equal(ModelValidationState.Invalid, kvp.Value.ValidationState);
var error = Assert.Single(kvp.Value.Errors);
Assert.Equal("This is not valid.", error.ErrorMessage);
});
}
// Regression test 2 for aspnet/Mvc#7963. ModelState should never be valid.
[Fact]
public async Task BindModelAsync_ForOverlappingParametersWithSuppressions_InValid_WithInValidSecondParameter()
{
// Arrange
var parameterDescriptor = new ParameterDescriptor
{
Name = "patchDocument",
ParameterType = typeof(IJsonPatchDocument),
};
var actionContext = GetControllerContext();
var modelState = actionContext.ModelState;
// First ModelState key is not empty to match SimpleTypeModelBinder.
modelState.SetModelValue("id", "notAGuid", "notAGuid");
modelState.AddModelError("id", "This is not valid.");
// Second ModelState key is empty to match BodyModelBinder.
modelState.AddModelError(string.Empty, "This is also not valid.");
var modelMetadataProvider = new TestModelMetadataProvider();
modelMetadataProvider.ForType<IJsonPatchDocument>().ValidationDetails(v => v.ValidateChildren = false);
var modelMetadata = modelMetadataProvider.GetMetadataForType(typeof(IJsonPatchDocument));
var parameterBinder = new ParameterBinder(
modelMetadataProvider,
Mock.Of<IModelBinderFactory>(),
new DefaultObjectValidator(
modelMetadataProvider,
new[] { TestModelValidatorProvider.CreateDefaultProvider() }),
_optionsAccessor,
NullLoggerFactory.Instance);
var modelBindingResult = ModelBindingResult.Failed();
var modelBinder = CreateMockModelBinder(modelBindingResult);
// Act
var result = await parameterBinder.BindModelAsync(
actionContext,
modelBinder,
new SimpleValueProvider(),
parameterDescriptor,
modelMetadata,
value: null);
// Assert
Assert.False(result.IsModelSet);
Assert.False(modelState.IsValid);
Assert.Collection(
modelState,
kvp =>
{
Assert.Empty(kvp.Key);
Assert.Equal(ModelValidationState.Invalid, kvp.Value.ValidationState);
var error = Assert.Single(kvp.Value.Errors);
Assert.Equal("This is also not valid.", error.ErrorMessage);
},
kvp =>
{
Assert.Equal("id", kvp.Key);
Assert.Equal(ModelValidationState.Invalid, kvp.Value.ValidationState);
var error = Assert.Single(kvp.Value.Errors);
Assert.Equal("This is not valid.", error.ErrorMessage);
});
}
private static ControllerContext GetControllerContext()
{
var services = new ServiceCollection();
@ -813,25 +939,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
return mockValueProvider.Object;
}
private static IModelValidatorProvider CreateMockValidatorProvider(IModelValidator validator = null)
{
var mockValidator = new Mock<IModelValidatorProvider>();
mockValidator
.Setup(o => o.CreateValidators(
It.IsAny<ModelValidatorProviderContext>()))
.Callback<ModelValidatorProviderContext>(context =>
{
if (validator != null)
{
foreach (var result in context.Results)
{
result.Validator = validator;
}
}
});
return mockValidator.Object;
}
private class Person : IEquatable<Person>, IEquatable<object>
{
public string Name { get; set; }
@ -862,9 +969,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
public string DerivedProperty { get; set; }
}
[Required]
private Person PersonProperty { get; set; }
public abstract class FakeModelMetadata : ModelMetadata
{
public FakeModelMetadata()