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

Avoid illegal reflective access when possible #144

Merged

Conversation

harawata
Copy link
Contributor

@harawata harawata commented Jan 8, 2022

As reported in #143 , making a method accessible is problematic in recent versions of Java ('illegal reflective access' warning on 9+ and InaccessibleObjectException on 16+).
This PR avoids some illegal reflective accesses that I found.
Please see the log message of each commit for the details.

The test passes on Java 8, but yields warning on 9+ and exception on 16+.
It's essentially the same as orphan-oss#104 .
OGNL should call `StringBuilder#codePointAt()` (which is a bridge method) rather than the method defined in its non-public parent class `AbstractStringBuilder`.
The test passes on Java 8, but yields warning on 9+ and exception on 16+.
The test passes on Java 8, but yields warning on 9+ and exception on 16+.
The test passes, but there is a message in System.err: Two methods with same method signature but not providing classes assignable?
This was originally reported as orphan-oss#35 and temporarily resolved by orphan-oss#40 , but as you can see, this is not an error case in the first place.
The test passes, but there is a message in System.err.
It might have been a legitimate check in non-generic era, but not anymore.
The test passes, but there is a message in System.err.
Slightly different version of the previous commit.
Copy link
Collaborator

@lukaszlenart lukaszlenart left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lukaszlenart lukaszlenart merged commit 8c616c7 into orphan-oss:master Jan 9, 2022
@lukaszlenart
Copy link
Collaborator

OGNL 3.3.2 is out! Thanks a lot!

@harawata
Copy link
Contributor Author

harawata commented Jan 9, 2022

Thank you for the swift response, @lukaszlenart !

@harawata harawata deleted the improve-best-method-detection branch January 9, 2022 13:52
@harawata
Copy link
Contributor Author

@lukaszlenart ,

I'm sorry to bother you, but I forgot to remove the unused local variable retsAreEqual. 😵
https://github.com/jkuhnert/ognl/blob/8c616c71d8b01ff02df11d8d0f679c9ab4e0ea2b/src/main/java/ognl/OgnlRuntime.java#L1787
Although it is not a big problem, it may be better to remove it before the next release.
If it's easier for you, I'll submit a new PR for that. Please just let me know!

@lukaszlenart
Copy link
Collaborator

Please issue a PR, thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants