Add additional types to SuppressChildValidationMetadataProvider (#37150) #37188
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prevent 'System.Delegate' and some reflection types from being validated. Fixes #36919
ghost
added
the
area-mvc
pranavkm
changed the title
davidfowl
approved these changes
Oct 2, 2021
bradygaster
approved these changes
Oct 4, 2021
Contributor
Author
|
@dotnet/aspnet-build this was approved. Could you merge it? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prevent 'System.Delegate' and some reflection types from being validated.
Fixes #36919
Description
MVC has where it will attempt to ensure that properties of a model being bound, that are marked as non-nullable, and indeed not null during validation. Validation will also recurse over the entire object graph. In the user reported issue, the user had a
System.Delegatetype appear in their model. Since the BCL is annotated for nullability, MVC's validation ends up atAssembly.ExportedTypesviaDelegate.MethodInfoand ends up visiting everyTypeinstance on it.While we think we need a better fix for this in 7.0, we'd like to consider a much more limited fix for 6.0 by preventing properties on
Delegateand some reflection types from being validated. MVC already excludes some types includingSystem.Typefrom being validated, so this is an expansion of the list to includeDelegateand a few reflection types such asMethodInfo,ParameterInfo, that would not warrant validation.Regression
No
Testing
[x] Manual
[x] Automated
Risk
It's possible that someone's actually trying to model bind a custom instance of
MethodInfothat does need to be validated for nullability. This is an incredibly contrived scenario and we don't have any evidence anyone is actually doing so. Users should be able to workaround this by configuringMvcOptionsto allow these types to be validated.