javassist
javassist copied to clipboard
jboss-javassist
FactoryHelper needs toClass(ClassFile, Lookup)
FactoryHelper is missing an API to define a class to the same class loader and runtime package as a Lookup's lookup class. This will be needed going forwarded as Unsafe.defineClass is deprecated and hacking into the non-public ClassLoader.defineClass methods can't be guaranteed to work in the long term.
Adding a toClass(ClassFile, Lookup) method would allow consumers to specify a Lookup with PACKAGE access that authenticates that the lookup object was created by a caller in the runtime package or derived from a lookup on a target class in the runtime package by suitably privileged code.
In addition, I note that DefineClassHelper.toPublicClass does use Lookup.defineClass but it uses a Lookup on class DefineClassHelper and so can only be used to inject into package javassist.util.proxy.
If I follow you correctly this is not currently a problem but something we should look into if and when there is no access to defineClass any longer.
It does seem like the way forward would be using method handles to create a class from byte code I will agree. At the same time it appears Oracle is hard set on not allowing method handles to invoke any kind of privileges, it will only ever be able to access that which is accessible. This is all fine and well in an ideal world but whether this will work in practice remains to be seen. If they won't expose privileges through method handles there would likely be some other work around instead, which javassist of course would prefer to use over the contrary.
Either way and unless you are privy to some insider information of what will come, please do share. While we speculate, this remains an obstacle we will cross another day when more certainty prevails.
@AlanBateman Unless you disagree: Suggest close issue Reason: Works as implemented
The FactoryHelper.toClass methods involve Javaassist breaking into the non-public defineClass method. As things stand in JDK 9, this triggers an "Illegal reflective access". A future JDK release will dial this up further and eventually the hack will fail with InaccessibleObjectException. Users of Javaassist can of course create a full power Lookup to any class in their own module and so can use Lookup defineClass to inject classes where the class files are generated by Javaassist. However, Javaassist will not be able to inject classes itself, it would need the user to provide a Lookup (as a capability) to a target class in the appropriate runtime package. This issue was created to investigating adding an API for that, assuming FactoryHelper is widely used.
Fact: Warnings and assumptions aside, currently things are working as implemented whilst retaining the existing source=1.6 compatibility.
I fail to see how having an issue open with speculations serves any purpose, but I may be wrong.
If you do want to investigate this then I suggest roll a PR with test cases proving hard facts. I challenge you to prove:
- You are free to create in your own module any type of access modifiers by passing the lookup to the API and in so doing negate the use of
defineClasscompletely. - How this impacts compatibility, since
defineClassmethod handle is only since Java 9 is there a way to retain compatibility?
We already have a PR to squash the warnings, see #158, if that is your only concern.
My suggestion to close this issue remains... unless you can motivate otherwise. This issue prompts for no immediate action which needs to be taken at this time.
@nickl - I created this suggestion in your issue tracker to make the maintainers of Javaassist aware that Java SE now has supported way to inject classes into an existing runtime package. Once you start to test with --illegal-access=deny then you will start to see why it may make sense to consider.
Ahh now it becomes clear...
Please note: I am nickl- and not nickl and I am not a maintainer, there is actually only 1 maintainer for this project and as you may imagine he has his hands full enough.
Don't worry we are all aware of the illegal access mess Oracle has created, and how they are stirring users up to rally to get projects to fix the mess they created. Don't be one of the sheep we do not need open issues to point out the obvious. Especially where javassist is concerned, if java gave you what you needed then you didn't need javassist.
Again I challenge you to come up with solutions by submitting PRs and to refrain from creating work which you deem to be someone else's problem. Show some gratuity for the work these project maintainers are doing without compensation.
@AlanBateman You say there is a "supported way to inject classes into existing runtime packages", please prove it.
We added Lookup.defineClass in Java SE 9 specifically for this type of use-case. The javadoc for the method is here: https://docs.oracle.com/javase/9/docs/api/java/lang/invoke/MethodHandles.Lookup.html#defineClass-byte:A-
Javassist aims to retain backward compatibility, currently set at source=1.6, which is currently still possible as implemented.
It has not been proven that Lookup.defineClass will be an adequate replacement for Unsafe.defineClass but more importantly since the latter is still available to us there is no immediate need to replace it yet.
Suggest close issue Reason: Works as implemented.
If Unsafe.defineClass is completely out of the picture perhaps sun.reflect.ClassDefiner would still be available see #163
@AlanBateman, I've worked on this. Now the API supports MethodHandles.Lookup. The version of Javassist including this work will be released soon.
I could fix the problem without changing the original API for javassist.util.proxy. I used the following code snippet:
Class<?> neighbor = ... // a class belonging to the same package that bcode belongs to.
Lookup lookup = MethodHandles.lookup();
Lookup prvlookup = MethodHandles.privateLookupIn(neighbor, lookup);
return prvlookup.defineClass(bcode);
Since javassist.util.proxy always generates a subclass of the given class, I could use that super class as the "neighbor" class. On the other hand, Javassist also provides CtClass#toClass(), which loads the given bytecode without any hint to obtain the "neighbor" class. So I added a new method CtClass#toClass(Class<?> neighbor) as well as CtClass#toClass(Lookup).
This would be an OK solution but I'm wondering whether there is some way to obtain an appropriate Lookup from, for example, a package name. My current approach is problematic when making a new class in a new package, which has not existed yet. I read API documentations and googled lots but could not find a good solution.
Any suggestions or hints are highly appreciated.
If code in Javaassist is calling privateLookupIn to get a full-power lookup to a class in the user's module then Javaassist needs to read that module and the user's module needs to open the package to Javaassist. That's all okay when everything is on the class path as unnamed modules read all other modules and all packages are open. If Javasssist and/or the user's code is deployed as an explicit module in the future then you'll need to think about these things. In the code snippet, Javaassist will need to call Module addReads to read the user's module and the user code should need to statically or dynamically open the package to Javaassist. The nice thing about using a Lookup object in the API is that the don't need the establish the readability or require the user to open any packages.
There isn't any notion of a Lookup to a run-time package or any support for dynamically adding a package to a name module at this time. There have been a few brief discussions about this on the jigsaw-dev but nothing more.
Thank you for the feedback. So, is the following code a right snippet?
Class<?> neighbor = ... // a class belonging to the same package that bcode belongs to.
Lookup lookup = MethodHandles.lookup();
this.getClass().getModule().addReads(neighbor.getModule()); // call addReads() here
Lookup prvlookup = MethodHandles.privateLookupIn(neighbor, lookup);
return prvlookup.defineClass(bcode);
I'll also add a method taking Lookup to javassist.util.proxy. However, I suppose that the users want to run their programs without modification until they get used to Java's module system. Providing a smooth transition path would be desirable. So it's sad that I must ask the users to replace all the calls to CtClass#toClass(), which invokes defineClass(), with calls to CtClass#toClass(Class<?>) or CtClass#toClass(Lookup) since toClass() knows only the name of the loaded class but no other hints such as a "neighbor" class.
If the runtime package of the lookup class is opened to Javaassist then the code fragment will work and the new class will be defined in the same runtime package as the lookup class.
The issue with CtClass.toClass(ClassLoader) is that is assumes it can inject into any run-time package without any restrictions and there has never been a supported way to do this in the JDK. I assume in time that Javassist users will come to understand this. Without API changes then I assume Javassist users can use CtClass::toBytecode today to get the class bytes and use Lookup.defineClass themselves.
OK. I modified javassist.util.proxy to take Lookup. I also added Automatic-Module-Name to manifest.mf for smooth transition.
From which version would Automatic-Module-Name be available? The latest on Maven Central is 3.24.1-GA, and it doesn't have the attribute.
Is there any movement on this issue? Those reflection issues still occur up to 3.27.0-GA with javassist.util.proxy.SecurityActions - anything to move this forward?
I understand that the new module system of Java basically killed the reflection API and thus the only option is to change your code to use Lookup when accessing Javassist.
I would not exactly call it my code - it is part of Hibernate and Spring boot. So you suggest they need code changes to improve on this topic? Could you elaborate a little bit and what would need to be changed - i try to follow up in those projects then. Thanks!
A module M (e.g. application code) can prevent other modules (e.g. Javassist) from accessing M's classes and methods. So if the module system is on, the module M has to expose their classes and methods to Javassist. Honestly speaking, I don't have clear instructions to do that. The instructions would depend on the hierarchy of the class loaders and how the application container uses the module systems.
Sounds very similar on how OSXGi is handled, thank you for elaborating on this a little more. I think this is a little to far off my knowledge to try to bootstrap effort on the other projects without actually knowing what route to go down. Thank you for the effort to bring this forward nevertheless!
Another option suggested by the Java development team was to obtain a Lookup object in a way depending on the application code (I don't know a simple or general way to get it) and use it to access the application code. Javassist provides the API for doing that but no simple way to obtain the Lookup object.
Yes, the Java's module system provides similar functionality to OSGi. Many developers wondered why we needed another one as well as OSGi, which works more cooperatively with existing frameworks like Javassist.
Java's module system has one advantage integrating with Java compile. E.g. if you have same class x.y from both M and N jar which have module info, then Java compile will report one error: have same class name from both module M and N. I am not sure OSGi could achieve it, as my understanding, OSGi usually handle such during Runtime. (Maybe I am wrong)
