ReflectUtils breaks with Java 9 build 148+ about cglib HOT 29 CLOSED

It's not that easy. Creating a new class loader causes the generated code to be loaded by another class loader. Even if the class's package name is the same, the loaded class will exist in a different runtime package what limits the scope of classes that can be subclassed. There is no backwards-compatible way for us to resolve this.

This issue is not easy to workaround and I still hoping that this will not make the final release of Java 9, this change would break thousands of projects. For now, I suggest to wait and see as creating a new class loader every time is a very breaking change.

This is also a major performance issue as class loaders are quite expensive to create.

As for Mockito: It does no longer use cglib in more recent versions but the problem prevails for any other code generation library.

from cglib.

I stumbled upon the same problem, would have been too good to be true.

from cglib.

See https://issues.apache.org/jira/browse/SOLR-9893

from cglib.

This is the problematic code:

from cglib.

As said before, I have a solution that works without reflection to achieve the same and requires no AccessController checks. Only limitation: Requires Java 7, but otherwise very easy! Maybe I post my idea here, let me just finish something else.

from cglib.

I think I know what you mean by that. You want to create a handle to defineMethod within a subclass of class loader where you inherit the privileges of the surrounding class. This way, you can access the method from outside if you expose the reference to this handle.

Quite clever actually, we might pursue this.

from cglib.

Exactly! :-)

from cglib.

Nevertheless, we have a Lucene expressions module that subclasses ClassLoader base class all the time (whenever a user creates a query using the Lucene Expressions syntax). We have performance and garbage collection tests in Lucene and Elasticsearch that never ever caused any additional load. To me those "oh man thats slow" arguments are a no-go unless proven by benchmarks under normal workloads.

from cglib.

This is no guess work, I do a lot of code generation (creator of Byte Buddy, do most of the Mockito-related code generation work and do code generation in Hibernate) and I ran a lot of benchmarks in the field of code generation. Creating dedicated class loaders has a very significant impact, I did benchmark this.

from cglib.

With Java 7 / 8?

from cglib.

Yes, I remember testing this with a recent Java 8 version. For generating a class with a dedicated class loader, it takes usually twice the time compared to injection.

from cglib.

Ok, for us this is not a problem at all. Securitywise, CGLIB should allow both variants, e.g. use the one with new ClassLoader if SecurityManager does not allow it (e.g., Elasticsearch forbids any setAccessible anywhere in its code or dependend libraries).

from cglib.

Hi,
I just tried the MethodHandle hack: It does not work, because the Lookup object is "too intelligent". If you try to get a MethodHandle for a virtual method that is protected, it adds an explicit cast of the receiver to our lookup class. So the MH looks like this afterwards: MethodHandle(ClassLoaderAccessor,String,byte[],int,int,ProtectionDomain)Class
The first arg should be ClassLoader but was downcasted to our subclass ClassLoaderAccessor:

import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
import java.security.ProtectionDomain;

public class Test {
  
  private static abstract class ClassLoaderAccessor extends ClassLoader {
    static final MethodHandle MH_DEFINE_CLASS;
    static {
      try {
        MH_DEFINE_CLASS = MethodHandles.lookup().findVirtual(ClassLoader.class, "defineClass",
            MethodType.methodType(Class.class, String.class, byte[].class, int.class, int.class, ProtectionDomain.class));
        System.out.println(MH_DEFINE_CLASS);
      } catch (NoSuchMethodException | IllegalAccessException e) {
        throw new AssertionError("Cannot find ClassLoader#defineClass(String, byte[], int, int, ProtectionDomain)", e);
      }
    }
  }
  
  public static void main(String[] args) throws Throwable {
    ClassLoaderAccessor.MH_DEFINE_CLASS.invoke(ClassLoader.getSystemClassLoader(), "foo.bar", new byte[10], 0, 10, null);
  }

}

See MethodHandles#restrictProtectedReceiver in JDK source code.

Javadocs in https://docs.oracle.com/javase/7/docs/api/java/lang/invoke/MethodHandles.Lookup.html#findVirtual(java.lang.Class,%20java.lang.String,%20java.lang.invoke.MethodType) says: "The first argument will be of type refc if the lookup class has full privileges to access the member. Otherwise the member must be protected and the first argument will be restricted in type to the lookup class."

from cglib.

Just FYI, Byte Buddy added a fallback to using Unsafe for defining classes. To make use of this, use the newest version of Mockito 2 where this issue is worked around.

from cglib.

Hi,
the Unsafe trick is good, because Unsafe is (at least in Java 9) reachable, if you have the right privileges. Cglib, should do the same.

About Byte-Buddy: Looks fine, although it has one big problem: Please use AccessController.doPrivileged when initializing and calling the Unsafe and ClassLoader hacks. Cglib does it, but Byte-Buddy does not: No-go for Elasticsearch or Lucene/Solr, which runs under SecurityManager (also its tests run with SM to ensure that nothing bad happens and we run with lowest possible privileges).

from cglib.

The library is called Byte Buddy.

About the security manager issue: It is up to the users of the library to provide correct privileges. Otherwise, giving privileges to Byte Buddy would allow using the class injector to randomly inject classes to any user of the library. If one wants to use a security manager, the consumer of the library should assure that the injector is called with the correct privileges, anything else would offer an attack vector where getting hold of Byte Buddy classes would allow corrupting any privilege check by instrumentation. This is also implied by the documentation.

from cglib.

The problem is that the security check is done inside a static initializer and this caused much headache with Elasticsearch. You cannot really control when and by whom this static init block is called! That's why I am complaining. So correct would be the following: Get Unsafe/ClassLoader privates with own security, so one can grant ByteBuddy the correct privilege to access sun.misc.Unsafe in general.

On every call that causes injection of a class, just ask security manager if this is fine (using a RuntimePermission) and run the code without doPrivileged.

from cglib.

This is not true for Byte Buddy but it is for cglib. Byte Buddy attempts to set accessibility when attempting to inject a class. This causes the security manager to be asked.

from cglib.

To get declared fields, you also need privileges. Making them accessible is another thing.

from cglib.

Added a fallback to Unsafe.

from cglib.

@raphw Can you tell me which version of Mockito this issue is (or will be) fixed in? It's blocking us (see https://issues.apache.org/jira/browse/CALCITE-1568) and we don't use Cglib directly, only via Mockito.

from cglib.

Its fixed in Mockito for a while. Mockito is using Byte Buddy where this issue is resolved since 2.0.

from cglib.

@raphw I had no idea we were on an old version of Mockito. We'll upgrade. Thanks!

from cglib.

Warning is stile present in JDK 10.

from cglib.

cglib is no longer under active development and with the module system, it does no longer make much sense to support the library as it is impossible to properly get the callbacks to work with modules if proxies are injected into foreign modules. I recommend you to migrate.

For more information, see this blog entry: https://mydailyjava.blogspot.no/2018/04/jdk-11-and-proxies-in-world-past.html

from cglib.

from cglib.

Byte Buddy or Javassist.

from cglib.

wow.
So that's the final call. Use something else.

well fark.

from cglib.

Please close the project then on github - to stop the wasteful researching

from cglib.