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

Return null on illegal field acces #948

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mdesharnais
Copy link
Contributor

This seems to fix issue #940 without breaking any test.

In a nutshell, an IllegalAccessException is now expected when trying to access fields not exported by a module through reflexion.

@agentgt
Copy link
Contributor

agentgt commented Apr 6, 2022

If we pull your change in we break templates for folks upgrading in possibly very bizarre and not easy to understand ways (see #951 and the plethora of other recent bugs albeit that is caused by the default resolvers changing for java version). I can explain how this happens if you like when I have more time.

The current solution is not ideal but it is better to have it fail fast.

The field value resolver is inherently flawed and really needs a dedicated Java 9+ solution of using tryAccessible (and various other friends that are only in 9) instead of just catching exceptions and returning null.

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