-
Notifications
You must be signed in to change notification settings - Fork 38.8k
Prevent reflective invocation of private methods in web dispatcher #35352
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: yongjunhong <yongjunh@apache.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this be checked in RequestMappingHandlerMapping
when request mappings are being initialized?
It would avoid repeating that on every call.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I requested a few changes.
Please also take Rossen's comment into account.
|
||
if (AopUtils.isCglibProxy(bean) && Modifier.isPrivate(method.getModifiers())) { | ||
throw new IllegalStateException( | ||
"Cannot invoke private method [" + method.getName() + "] on a CGLIB proxy. " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be good to include context about the bean
in the exception message as well.
if (AopUtils.isCglibProxy(bean) && Modifier.isPrivate(method.getModifiers())) { | ||
throw new IllegalStateException( | ||
"Cannot invoke private method [" + method.getName() + "] on a CGLIB proxy. " + | ||
"Handler methods on proxied components must be public or protected. " + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not completely true. It's possible for a package-private method to be proxied.
As stated in the reference documentation:
private
methods cannot be advised, because they cannot be overridden.- Methods that are not visible – for example, package-private methods in a parent class from a different package – cannot be advised because they are effectively private.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 72ff98a !
...ng-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java
Outdated
Show resolved
Hide resolved
...ng-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java
Outdated
Show resolved
Hide resolved
I'm currently traveling, so I should be able to work on it around Wednesday or Thursday this week. |
Signed-off-by: yongjunhong <yongjunh@apache.org>
Signed-off-by: yongjunhong <yongjunh@apache.org>
Signed-off-by: yongjunhong <yongjunh@apache.org>
Please take a look :) |
If you have some time, I’d really appreciate it if you could take a look at this PR as well. 🙇🏻♂️🙇🏻♂️ It’s similar to the this PR, focusing on the access modifiers and proxies!! |
fixes #30938