Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValidationAttribute.SetResourceAccessorByPropertyLookup should throw better exception #44292

Closed
krwq opened this issue Nov 5, 2020 · 3 comments

Comments

@krwq
Copy link
Member

krwq commented Nov 5, 2020

Converting TODO-NULLABLE into the issue here since I'm not familiar with the area I'll leave it for area owner to decide if they want to close it or leave it. I'll remove the original comment in case it's decided this should be closed.

In ValidationAttribute.SetResourceAccessorByPropertyLookup (last line):

_errorMessageResourceAccessor = () => (string)property.GetValue(null, null)!;

Is commented as If the user-provided resource returns null, an ArgumentNullException is thrown - should probably throw a better exception

@ghost
Copy link

ghost commented Nov 5, 2020

Tagging subscribers to this area: @ajcvickers
See info in area-owners.md if you want to be subscribed.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 5, 2020
@ajcvickers
Copy link
Contributor

@roji Can you take a quick look at this along with the other nullable cleanup?

@ajcvickers ajcvickers added this to the 6.0.0 milestone Jul 13, 2021
@ajcvickers ajcvickers removed the untriaged New issue has not been triaged by the area owner label Jul 13, 2021
@roji
Copy link
Member

roji commented Jul 14, 2021

This basically means that if the user sets up the validation attribute to point at a resource accessor, and the property returns null, attempts to format an error message will fail with ArgumentNullException. This is incorrect/unsupported usage by the user, so barring changing the exception to be slightly more informative I don't think there's anything to do.

@roji roji closed this as completed Jul 14, 2021
@roji roji removed this from the 6.0.0 milestone Jul 14, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants