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.

Sign up for GitHub

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

Jump to bottom

Fix argument validation in RuntimeType.InvokeMember #74998

Merged
merged 1 commit into from
Sep 2, 2022
Merged

Fix argument validation in RuntimeType.InvokeMember #74998

merged 1 commit into from
Sep 2, 2022

Conversation

stephentoub
Copy link
Member

The rollout of !! erroneously moved an ArgumentNullException to be thrown earlier in the method, preventing a null name from being used (which is valid with BindingFlags.CreateInstance).

(Separately, we should consider fixing the nullable reference type annotation on string name, since null is allowed in some circumstances.)

Fixes #74977 (reply in thread)
cc: @AaronRobinsonMSFT, @steveharter

@stephentoub
Fix argument validation in RuntimeType.InvokeMember
57624c1 
The rollout of `!!` erroneously moved an ArgumentNullException to be thrown earlier in the method, preventing a null name from being used (which is valid with BindingFlags.CreateInstance).

(Separately, we should consider fixing the nullable reference type annotation on `string name`, since null is allowed in some circumstances.)
@ghost ghost assigned stephentoub Sep 2, 2022
@ghost
Copy link

ghost commented Sep 2, 2022

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

The rollout of !! erroneously moved an ArgumentNullException to be thrown earlier in the method, preventing a null name from being used (which is valid with BindingFlags.CreateInstance).

(Separately, we should consider fixing the nullable reference type annotation on string name, since null is allowed in some circumstances.)

Fixes #74977 (reply in thread)
cc: @AaronRobinsonMSFT, @steveharter

Author: stephentoub
Assignees: stephentoub
Labels:

area-System.Reflection
Milestone: -

@AaronRobinsonMSFT
Copy link
Member

Thanks @stephentoub. I assume we can also get this into 7.0 once it is in 8.0?

@stephentoub
Copy link
Member Author

That's my plan.

@stephentoub
Copy link
Member Author

/backport to release/7.0

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/2980567108

buyaa-n
buyaa-n approved these changes Sep 2, 2022
View reviewed changes
Copy link
Contributor

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions github-actions bot mentioned this pull request Sep 2, 2022
Merged
@stephentoub stephentoub merged commit e6e4d22 into dotnet:main Sep 2, 2022
@stephentoub stephentoub deleted the fixargnullinvoke branch September 2, 2022 20:53
@ghost ghost locked as resolved and limited conversation to collaborators Oct 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@buyaa-n buyaa-n buyaa-n approved these changes

@AaronRobinsonMSFT AaronRobinsonMSFT AaronRobinsonMSFT approved these changes

Assignees

@stephentoub stephentoub

Labels
area-System.Reflection
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stephentoub @AaronRobinsonMSFT @buyaa-n