From 32d4420a020889e1c64c103bb8c50c7ee84449e3 Mon Sep 17 00:00:00 2001 From: Tumi Date: Thu, 17 Jul 2014 16:38:36 +0200 Subject: [PATCH] Fixes issue #217 in Kryo (https://github.com/EsotericSoftware/kryo/issues/217) --- .../reflectasm/AccessClassLoader.java | 44 ++++++++++++++-- .../reflectasm/ConstructorAccess.java | 52 ++++++++++++------- .../reflectasm/FieldAccess.java | 6 +-- .../reflectasm/MethodAccess.java | 10 ++-- .../reflectasm/PublicConstructorAccess.java | 5 ++ 5 files changed, 85 insertions(+), 32 deletions(-) create mode 100644 src/com/esotericsoftware/reflectasm/PublicConstructorAccess.java diff --git a/src/com/esotericsoftware/reflectasm/AccessClassLoader.java b/src/com/esotericsoftware/reflectasm/AccessClassLoader.java index 96ff571..0044ab8 100644 --- a/src/com/esotericsoftware/reflectasm/AccessClassLoader.java +++ b/src/com/esotericsoftware/reflectasm/AccessClassLoader.java @@ -16,6 +16,8 @@ class AccessClassLoader extends ClassLoader { // Fast-path for classes loaded in the same ClassLoader as this class. static private final ClassLoader selfContextParentClassLoader = getParentClassLoader(AccessClassLoader.class); static private volatile AccessClassLoader selfContextAccessClassLoader = new AccessClassLoader(selfContextParentClassLoader); + + static private volatile Method defineClassMethod; static AccessClassLoader get (Class type) { ClassLoader parent = getParentClassLoader(type); @@ -72,6 +74,7 @@ class AccessClassLoader extends ClassLoader { if (name.equals(FieldAccess.class.getName())) return FieldAccess.class; if (name.equals(MethodAccess.class.getName())) return MethodAccess.class; if (name.equals(ConstructorAccess.class.getName())) return ConstructorAccess.class; + if (name.equals(PublicConstructorAccess.class.getName())) return PublicConstructorAccess.class; // All other classes come from the classloader that loaded the type we are accessing. return super.loadClass(name, resolve); } @@ -79,19 +82,52 @@ class AccessClassLoader extends ClassLoader { Class defineClass (String name, byte[] bytes) throws ClassFormatError { try { // Attempt to load the access class in the same loader, which makes protected and default access members accessible. - Method method = ClassLoader.class.getDeclaredMethod("defineClass", new Class[] {String.class, byte[].class, int.class, - int.class, ProtectionDomain.class}); - if (!method.isAccessible()) method.setAccessible(true); - return (Class)method.invoke(getParent(), new Object[] {name, bytes, Integer.valueOf(0), Integer.valueOf(bytes.length), + return (Class)getDefineClassMethod().invoke(getParent(), new Object[] {name, bytes, Integer.valueOf(0), Integer.valueOf(bytes.length), getClass().getProtectionDomain()}); } catch (Exception ignored) { + // continue with the definition in the current loader (won't have access to protected and package-protected members) } return defineClass(name, bytes, 0, bytes.length, getClass().getProtectionDomain()); } + + // As per JLS, section 5.3, + // "The runtime package of a class or interface is determined by the package name and defining class loader of the class or interface." + static boolean areInSameRuntimeClassLoader(Class type1, Class type2) { + + if (type1.getPackage()!=type2.getPackage()) { + return false; + } + ClassLoader loader1 = type1.getClassLoader(); + ClassLoader loader2 = type2.getClassLoader(); + ClassLoader systemClassLoader = ClassLoader.getSystemClassLoader(); + if (loader1==null) { + return (loader2==null || loader2==systemClassLoader); + } + if (loader2==null) { + return loader1==systemClassLoader; + } + return loader1==loader2; + } private static ClassLoader getParentClassLoader (Class type) { ClassLoader parent = type.getClassLoader(); if (parent == null) parent = ClassLoader.getSystemClassLoader(); return parent; } + + private static Method getDefineClassMethod() throws Exception { + // DCL on volatile + if (defineClassMethod==null) { + synchronized(accessClassLoaders) { + defineClassMethod = ClassLoader.class.getDeclaredMethod("defineClass", new Class[] {String.class, byte[].class, int.class, + int.class, ProtectionDomain.class}); + try { + defineClassMethod.setAccessible(true); + } + catch (Exception ignored) { + } + } + } + return defineClassMethod; + } } diff --git a/src/com/esotericsoftware/reflectasm/ConstructorAccess.java b/src/com/esotericsoftware/reflectasm/ConstructorAccess.java index ed5638c..d0537e5 100644 --- a/src/com/esotericsoftware/reflectasm/ConstructorAccess.java +++ b/src/com/esotericsoftware/reflectasm/ConstructorAccess.java @@ -35,8 +35,8 @@ public abstract class ConstructorAccess { String className = type.getName(); String accessClassName = className + "ConstructorAccess"; if (accessClassName.startsWith("java.")) accessClassName = "reflectasm." + accessClassName; - Class accessClass = null; - + Class accessClass; + AccessClassLoader loader = AccessClassLoader.get(type); synchronized (loader) { try { @@ -45,39 +45,41 @@ public abstract class ConstructorAccess { String accessClassNameInternal = accessClassName.replace('.', '/'); String classNameInternal = className.replace('.', '/'); String enclosingClassNameInternal; - - boolean isPrivate = false; + Constructor constructor = null; + int modifiers = 0; if (!isNonStaticMemberClass) { enclosingClassNameInternal = null; try { - Constructor constructor = type.getDeclaredConstructor((Class[])null); - isPrivate = Modifier.isPrivate(constructor.getModifiers()); + constructor = type.getDeclaredConstructor((Class[])null); + modifiers = constructor.getModifiers(); } catch (Exception ex) { throw new RuntimeException("Class cannot be created (missing no-arg constructor): " + type.getName(), ex); } - if (isPrivate) { + if (Modifier.isPrivate(modifiers)) { throw new RuntimeException("Class cannot be created (the no-arg constructor is private): " + type.getName()); } } else { enclosingClassNameInternal = enclosingType.getName().replace('.', '/'); try { - Constructor constructor = type.getDeclaredConstructor(enclosingType); // Inner classes should have this. - isPrivate = Modifier.isPrivate(constructor.getModifiers()); + constructor = type.getDeclaredConstructor(enclosingType); // Inner classes should have this. + modifiers = constructor.getModifiers(); } catch (Exception ex) { throw new RuntimeException("Non-static member class cannot be created (missing enclosing class constructor): " + type.getName(), ex); } - if (isPrivate) { + if (Modifier.isPrivate(modifiers)) { throw new RuntimeException( "Non-static member class cannot be created (the enclosing class constructor is private): " + type.getName()); } } + String superclassNameInternal = Modifier.isPublic(modifiers) ? + "com/esotericsoftware/reflectasm/PublicConstructorAccess" : + "com/esotericsoftware/reflectasm/ConstructorAccess"; ClassWriter cw = new ClassWriter(0); - cw.visit(V1_1, ACC_PUBLIC + ACC_SUPER, accessClassNameInternal, null, - "com/esotericsoftware/reflectasm/ConstructorAccess", null); + cw.visit(V1_1, ACC_PUBLIC + ACC_SUPER, accessClassNameInternal, null, superclassNameInternal, null); - insertConstructor(cw); + insertConstructor(cw, superclassNameInternal); insertNewInstance(cw, classNameInternal); insertNewInstanceInner(cw, classNameInternal, enclosingClassNameInternal); @@ -85,20 +87,30 @@ public abstract class ConstructorAccess { accessClass = loader.defineClass(accessClassName, cw.toByteArray()); } } + ConstructorAccess access; try { - ConstructorAccess access = (ConstructorAccess)accessClass.newInstance(); - access.isNonStaticMemberClass = isNonStaticMemberClass; - return access; - } catch (Exception ex) { - throw new RuntimeException("Error constructing constructor access class: " + accessClassName, ex); + access = (ConstructorAccess)accessClass.newInstance(); + } catch (Throwable t) { + throw new RuntimeException("Exception constructing constructor access class: " + accessClassName, t); } + if (!(access instanceof PublicConstructorAccess) && !AccessClassLoader.areInSameRuntimeClassLoader(type, accessClass)) { + // Must test this after the try-catch block, whether the class has been loaded as if has been defined. + // Throw a Runtime exception here instead of an IllegalAccessError when invoking newInstance() + throw new RuntimeException( + (!isNonStaticMemberClass ? + "Class cannot be created (the no-arg constructor is protected or package-protected, and its ConstructorAccess could not be defined in the same class loader): " : + "Non-static member class cannot be created (the enclosing class constructor is protected or package-protected, and its ConstructorAccess could not be defined in the same class loader): ") + + type.getName()); + } + access.isNonStaticMemberClass = isNonStaticMemberClass; + return access; } - static private void insertConstructor (ClassWriter cw) { + static private void insertConstructor (ClassWriter cw, String superclassNameInternal) { MethodVisitor mv = cw.visitMethod(ACC_PUBLIC, "", "()V", null, null); mv.visitCode(); mv.visitVarInsn(ALOAD, 0); - mv.visitMethodInsn(INVOKESPECIAL, "com/esotericsoftware/reflectasm/ConstructorAccess", "", "()V"); + mv.visitMethodInsn(INVOKESPECIAL, superclassNameInternal, "", "()V"); mv.visitInsn(RETURN); mv.visitMaxs(1, 1); mv.visitEnd(); diff --git a/src/com/esotericsoftware/reflectasm/FieldAccess.java b/src/com/esotericsoftware/reflectasm/FieldAccess.java index 0952bac..e8d2b97 100644 --- a/src/com/esotericsoftware/reflectasm/FieldAccess.java +++ b/src/com/esotericsoftware/reflectasm/FieldAccess.java @@ -19,7 +19,7 @@ public abstract class FieldAccess { public int getIndex (String fieldName) { for (int i = 0, n = fieldNames.length; i < n; i++) if (fieldNames[i].equals(fieldName)) return i; - throw new IllegalArgumentException("Unable to find public field: " + fieldName); + throw new IllegalArgumentException("Unable to find non-private field: " + fieldName); } public void set (Object instance, String fieldName, Object value) { @@ -147,8 +147,8 @@ public abstract class FieldAccess { access.fieldNames = fieldNames; access.fieldTypes = fieldTypes; return access; - } catch (Exception ex) { - throw new RuntimeException("Error constructing field access class: " + accessClassName, ex); + } catch (Throwable t) { + throw new RuntimeException("Error constructing field access class: " + accessClassName, t); } } diff --git a/src/com/esotericsoftware/reflectasm/MethodAccess.java b/src/com/esotericsoftware/reflectasm/MethodAccess.java index 9145812..45e0e13 100644 --- a/src/com/esotericsoftware/reflectasm/MethodAccess.java +++ b/src/com/esotericsoftware/reflectasm/MethodAccess.java @@ -35,21 +35,21 @@ public abstract class MethodAccess { public int getIndex (String methodName) { for (int i = 0, n = methodNames.length; i < n; i++) if (methodNames[i].equals(methodName)) return i; - throw new IllegalArgumentException("Unable to find public method: " + methodName); + throw new IllegalArgumentException("Unable to find non-private method: " + methodName); } /** Returns the index of the first method with the specified name and param types. */ public int getIndex (String methodName, Class... paramTypes) { for (int i = 0, n = methodNames.length; i < n; i++) if (methodNames[i].equals(methodName) && Arrays.equals(paramTypes, parameterTypes[i])) return i; - throw new IllegalArgumentException("Unable to find public method: " + methodName + " " + Arrays.toString(paramTypes)); + throw new IllegalArgumentException("Unable to find non-private method: " + methodName + " " + Arrays.toString(paramTypes)); } /** Returns the index of the first method with the specified name and the specified number of arguments. */ public int getIndex (String methodName, int paramsCount) { for (int i = 0, n = methodNames.length; i < n; i++) if (methodNames[i].equals(methodName) && parameterTypes[i].length == paramsCount) return i; - throw new IllegalArgumentException("Unable to find public method: " + methodName + " with " + paramsCount + " params."); + throw new IllegalArgumentException("Unable to find non-private method: " + methodName + " with " + paramsCount + " params."); } public String[] getMethodNames () { @@ -260,8 +260,8 @@ public abstract class MethodAccess { access.parameterTypes = parameterTypes; access.returnTypes = returnTypes; return access; - } catch (Exception ex) { - throw new RuntimeException("Error constructing method access class: " + accessClassName, ex); + } catch (Throwable t) { + throw new RuntimeException("Error constructing method access class: " + accessClassName, t); } } diff --git a/src/com/esotericsoftware/reflectasm/PublicConstructorAccess.java b/src/com/esotericsoftware/reflectasm/PublicConstructorAccess.java new file mode 100644 index 0000000..2d38845 --- /dev/null +++ b/src/com/esotericsoftware/reflectasm/PublicConstructorAccess.java @@ -0,0 +1,5 @@ +package com.esotericsoftware.reflectasm; + +public abstract class PublicConstructorAccess extends ConstructorAccess { + +} \ No newline at end of file