From 1d4ba687ae76d663d817f319721b3f8a10b6baf6 Mon Sep 17 00:00:00 2001 From: Su5eD Date: Sat, 25 Nov 2023 18:41:31 +0100 Subject: [PATCH 1/3] Fix loading package-info classes --- .../java/cpw/mods/cl/ModuleClassLoader.java | 4 ++- .../cpw/mods/cl/ProtectionDomainHelper.java | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/main/java/cpw/mods/cl/ModuleClassLoader.java b/src/main/java/cpw/mods/cl/ModuleClassLoader.java index 04110d0..0f4ff24 100644 --- a/src/main/java/cpw/mods/cl/ModuleClassLoader.java +++ b/src/main/java/cpw/mods/cl/ModuleClassLoader.java @@ -172,7 +172,9 @@ private Class readerToClass(final ModuleReader reader, final ModuleReference var modroot = this.resolvedRoots.get(ref.descriptor().name()); ProtectionDomainHelper.tryDefinePackage(this, name, modroot.jar().getManifest(), t->modroot.jar().getManifest().getAttributes(t), this::definePackage); // Packages are dirctories, and can't be signed, so use raw attributes instead of signed. var cs = ProtectionDomainHelper.createCodeSource(toURL(ref.location()), modroot.jar().verifyAndGetSigners(cname, bytes)); - return defineClass(name, bytes, 0, bytes.length, ProtectionDomainHelper.createProtectionDomain(cs, this)); + var cls = defineClass(name, bytes, 0, bytes.length, ProtectionDomainHelper.createProtectionDomain(cs, this)); + ProtectionDomainHelper.trySetPackageModule(cls.getPackage(), cls.getModule()); + return cls; } protected byte[] maybeTransformClassBytes(final byte[] bytes, final String name, final String context) { diff --git a/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java b/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java index b9bb025..ab60cc2 100644 --- a/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java +++ b/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java @@ -1,5 +1,7 @@ package cpw.mods.cl; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.VarHandle; import java.net.URL; import java.security.*; import java.util.HashMap; @@ -27,6 +29,31 @@ public static ProtectionDomain createProtectionDomain(CodeSource codeSource, Cla } } + private static final VarHandle PKG_MODULE_HANDLE; + static { + try { + var trustedLookupField = MethodHandles.Lookup.class.getDeclaredField("IMPL_LOOKUP"); + trustedLookupField.setAccessible(true); + MethodHandles.Lookup trustedLookup = (MethodHandles.Lookup) trustedLookupField.get(null); + + Class namedPackage = Class.forName("java.lang.NamedPackage"); + PKG_MODULE_HANDLE = trustedLookup.findVarHandle(namedPackage, "module", Module.class); + } catch (Throwable t) { + throw new RuntimeException(t); + } + } + + static void trySetPackageModule(Package pkg, Module module) { + Module value = (Module) PKG_MODULE_HANDLE.get(pkg); + if (value == null || !value.isNamed()) { + try { + PKG_MODULE_HANDLE.set(pkg, module); + } catch (Throwable t) { + throw new RuntimeException(t); + } + } + } + static Package tryDefinePackage(final ClassLoader classLoader, String name, Manifest man, Function trustedEntries, Function definePackage) throws IllegalArgumentException { final var pname = name.substring(0, name.lastIndexOf('.')); From 9a052fbc912a527452d7c9c809a307c62f57398d Mon Sep 17 00:00:00 2001 From: Su5eD Date: Mon, 27 Nov 2023 21:00:03 +0100 Subject: [PATCH 2/3] Add regression test --- .../cpw/mods/cl/test/TestPackageInfo.java | 22 +++++++++++++++++++ .../java/cpw/mods/cl/testjar1/SomeClass.java | 7 ++++++ .../cpw/mods/cl/testjar1/TestAnnotation.java | 11 ++++++++++ .../cpw/mods/cl/testjar1/package-info.java | 3 +++ 4 files changed, 43 insertions(+) create mode 100644 src/test/java/cpw/mods/cl/test/TestPackageInfo.java create mode 100644 src/testjar1/java/cpw/mods/cl/testjar1/SomeClass.java create mode 100644 src/testjar1/java/cpw/mods/cl/testjar1/TestAnnotation.java create mode 100644 src/testjar1/java/cpw/mods/cl/testjar1/package-info.java diff --git a/src/test/java/cpw/mods/cl/test/TestPackageInfo.java b/src/test/java/cpw/mods/cl/test/TestPackageInfo.java new file mode 100644 index 0000000..7454a8c --- /dev/null +++ b/src/test/java/cpw/mods/cl/test/TestPackageInfo.java @@ -0,0 +1,22 @@ +package cpw.mods.cl.test; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.lang.annotation.Annotation; +import java.util.Arrays; + +public class TestPackageInfo { + + @Test + public void testPackageInfoAvailability() throws Exception { + // package-info classes can be loaded + TestjarUtil.withTestjar1Setup(cl -> { + String annotationType = "cpw.mods.cl.testjar1.TestAnnotation"; + // Reference package through a class to ensure correct behavior of ModuleClassLoader#findClass(String,String) + Class cls = Class.forName("cpw.mods.cl.testjar1.SomeClass", true, cl); + Annotation[] annotations = cls.getPackage().getDeclaredAnnotations(); + Assertions.assertTrue(Arrays.stream(annotations).anyMatch(ann -> ann.annotationType().getName().equals(annotationType)), "Expected package to be annotated with " + annotationType); + }); + } +} diff --git a/src/testjar1/java/cpw/mods/cl/testjar1/SomeClass.java b/src/testjar1/java/cpw/mods/cl/testjar1/SomeClass.java new file mode 100644 index 0000000..a8bbe82 --- /dev/null +++ b/src/testjar1/java/cpw/mods/cl/testjar1/SomeClass.java @@ -0,0 +1,7 @@ +package cpw.mods.cl.testjar1; + +/** + * Referenced by {@code TestClassLoader}. + */ +public class SomeClass { +} diff --git a/src/testjar1/java/cpw/mods/cl/testjar1/TestAnnotation.java b/src/testjar1/java/cpw/mods/cl/testjar1/TestAnnotation.java new file mode 100644 index 0000000..d1488ea --- /dev/null +++ b/src/testjar1/java/cpw/mods/cl/testjar1/TestAnnotation.java @@ -0,0 +1,11 @@ +package cpw.mods.cl.testjar1; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +@Target(ElementType.PACKAGE) +@Retention(RetentionPolicy.RUNTIME) +public @interface TestAnnotation { +} diff --git a/src/testjar1/java/cpw/mods/cl/testjar1/package-info.java b/src/testjar1/java/cpw/mods/cl/testjar1/package-info.java new file mode 100644 index 0000000..82cf822 --- /dev/null +++ b/src/testjar1/java/cpw/mods/cl/testjar1/package-info.java @@ -0,0 +1,3 @@ +@TestAnnotation +package cpw.mods.cl.testjar1; + From 7e4705a9099669fa3c05b2660b921cfce38ae979 Mon Sep 17 00:00:00 2001 From: Su5eD Date: Fri, 12 Jan 2024 14:11:51 +0100 Subject: [PATCH 3/3] Add comments --- src/main/java/cpw/mods/cl/ProtectionDomainHelper.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java b/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java index ab60cc2..22a92db 100644 --- a/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java +++ b/src/main/java/cpw/mods/cl/ProtectionDomainHelper.java @@ -32,6 +32,7 @@ public static ProtectionDomain createProtectionDomain(CodeSource codeSource, Cla private static final VarHandle PKG_MODULE_HANDLE; static { try { + // Obtain VarHandle for NamedPackage#module via trusted lookup var trustedLookupField = MethodHandles.Lookup.class.getDeclaredField("IMPL_LOOKUP"); trustedLookupField.setAccessible(true); MethodHandles.Lookup trustedLookup = (MethodHandles.Lookup) trustedLookupField.get(null); @@ -39,17 +40,19 @@ public static ProtectionDomain createProtectionDomain(CodeSource codeSource, Cla Class namedPackage = Class.forName("java.lang.NamedPackage"); PKG_MODULE_HANDLE = trustedLookup.findVarHandle(namedPackage, "module", Module.class); } catch (Throwable t) { - throw new RuntimeException(t); + throw new RuntimeException("Error finding package module handle", t); } } static void trySetPackageModule(Package pkg, Module module) { + // Ensure named packages are bound to their module of origin + // Necessary for loading package-info classes Module value = (Module) PKG_MODULE_HANDLE.get(pkg); if (value == null || !value.isNamed()) { try { PKG_MODULE_HANDLE.set(pkg, module); } catch (Throwable t) { - throw new RuntimeException(t); + throw new RuntimeException("Error setting package module", t); } } }