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

Fix inaccessible object exception #1198

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stefansimon76
Copy link

"field.setAccessible(true);" throws an exception on java-17 because java.lang is not open for reflection. The problem cannot be worked around on the client side because "setAccessible(true)" is executed before evaluating the passed parameter "excludeFields". Both should be fixed with the change.
The existing tests also cover the change.

@garydgregory
Copy link
Member

garydgregory commented Apr 10, 2024

Hello @stefansimon76

  • There is too much copy-pasta in this PR, the same code is duplicated at least 3 times.
  • The whole point of the feature is to use reflection as the API name makes obvious, so you must enable the app to run with reflection, ignoring reflection exceptions will only hide the fact that the app is misconfigured. So this will likely break existing app behavior.
  • The reason we are calling AccessibleObject.setAccessible() is to perform a single security check instead of one per field, this PR loses this feature.
  • The valid remaining question IMO is: Is there a benefit to pruning the array of fields passed to AccessibleObject.setAccessible()? Semantically it seems "yes" but practically, the added complexity might not be worth it based on a quick look at the implementation of AccessibleObject.setAccessible().

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