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

Add a bundle 'jdk-reflection' #86

Merged
merged 3 commits into from
Nov 14, 2015

Conversation

centic9
Copy link
Contributor

@centic9 centic9 commented Nov 8, 2015

This allows to forbid certain reflection calls which are usually blocked by SecurityManagers, more might be useful, but this is the bare minimum set that often indicates invalid access.

…on calls which are usually blocked by SecurityManagers
@uschindler
Copy link
Member

Cool thanks! I will look into merging it. We also collected some bad stuff which might be added as bundles.

@uschindler uschindler self-assigned this Nov 8, 2015
@uschindler uschindler added this to the 2.1 milestone Nov 8, 2015
@uschindler
Copy link
Member

I general, I would remove the Method#invoke stuff. The bad stuff is only setAccessible(). Reflection is not bad per se, but working around security is a problem.


java.lang.reflect.AccessibleObject#setAccessible(java.lang.reflect.AccessibleObject[], boolean) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9
java.lang.reflect.AccessibleObject#setAccessible(boolean) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9
java.lang.reflect.Method#invoke(java.lang.Object, java.lang.Object[]) @ Reflection usage fails with SecurityManagers and likely will not work any more in Java 9
Copy link
Member

Choose a reason for hiding this comment

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

As said on issue, I would remove this. It is not bad and won't fail with security managers, unless very restructive. But setAccessible() is a no-go.

@centic9
Copy link
Contributor Author

centic9 commented Nov 11, 2015

Sure, I have updated the pull request accordingly now.

@uschindler
Copy link
Member

Thanks. Currently I am thinking about the fact, if the reflection bundle should be made dependent on Java version like unsafe or deprecated.

One small thing. The test class should be named Java6* instead of Oracle*. The reason is that the antunit/Oracle* classes are compiled only on JVMs that are Oracle. The test is basically a test class that should compile with any JVM. I can change that before regenerating the class file.

uschindler added a commit that referenced this pull request Nov 14, 2015
@uschindler uschindler merged commit b9ea33e into policeman-tools:master Nov 14, 2015
@uschindler
Copy link
Member

I merged this. Will do some cleanups and adding docs in a minute.

uschindler added a commit that referenced this pull request Nov 14, 2015
@centic9 centic9 deleted the add_bundle_reflection branch November 15, 2015 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants