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

Check permission to AccessibleObject#setAccessible(boolean) a better way #4389

Merged
merged 2 commits into from Dec 15, 2016

Conversation

@jmiettinen
Copy link
Contributor

@jmiettinen jmiettinen commented Dec 15, 2016

In #4266, we noticed a difference what methods JRuby finds in Java-classes between JRuby-jar being in boot classpath and in the normal classpath. This change removes that difference.

The reason for the difference is that all classes in boot classpath are privileged and AccessController#checkPermission(Permission) will always return true to them, whereas for others the result depends on security policy in effect.

In normal (at least Oracle) JVM installations, the policy doesn't allow suppressAccessChecks, but call to AccessibleObject#setAccessible(boolean) will still succeed as there's no SecurityManager set.

I posit that trying to call setAccessible is a better way to see if we might later on succeed in calling setAccessible than checking the permission.

…ccessible(boolean) instead of seeing if we have permission 'suppressAccessChecks'. We may not have that permission if we're not using a privileged classloader but can still setAccessible if (when) there's no security manager
@headius
Copy link
Member

@headius headius commented Dec 15, 2016

I like the idea, but perhaps we should test it against a class in JRuby, so we're not modifying permissions for a globally-visible class. Perhaps make an Integer look-alike static inner class and use that?

@jmiettinen
Copy link
Contributor Author

@jmiettinen jmiettinen commented Dec 15, 2016

Calling Field.setAccessible(boolean) only overrides the accessibility checks for that Field-object.

It is, however, better to do checks for code that's under our control. I don't know what would be the best place to insert such class so I dumped it into same package for now.

@headius
Copy link
Member

@headius headius commented Dec 15, 2016

This looks fine to me...I'll merge. Thanks!

@headius headius merged commit 755fdb7 into jruby:master Dec 15, 2016
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
@headius headius added this to the JRuby 9.1.7.0 milestone Dec 15, 2016
headius added a commit to headius/jruby that referenced this pull request May 4, 2017
Check permission to AccessibleObject#setAccessible(boolean) a better way
@headius headius mentioned this pull request May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.