You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2026/08/07 16:35:22 UTC
(groovy) branch GROOVY-12223 updated: GROOVY-12223: Honour module boundaries for hidden nestmates
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch GROOVY-12223
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY-12223 by this push:
new dad120c49a GROOVY-12223: Honour module boundaries for hidden nestmates
dad120c49a is described below
commit dad120c49a648d5755ebcea8bb855603e34409a9
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sat Aug 8 01:28:58 2026 +0900
GROOVY-12223: Honour module boundaries for hidden nestmates
---
.../org/apache/groovy/util/HiddenClassDefiner.java | 182 ++++++++++-----------
.../reflection/ClassLoaderForClassArtifacts.java | 20 +--
.../codehaus/groovy/runtime/ProxyClassDefiner.java | 37 +++--
.../groovy/runtime/ProxyGeneratorAdapter.java | 5 +-
.../groovy/runtime/metaclass/ReflectorLoader.java | 92 +++--------
.../groovy/util/HiddenClassDefinerTest.groovy | 56 +++++--
.../ClassLoaderForClassArtifactsTest.groovy | 35 ++++
.../groovy/runtime/ProxyClassDefinerTest.groovy | 145 +++++++++++++++-
.../runtime/metaclass/ReflectorLoaderTest.groovy | 29 ++--
9 files changed, 378 insertions(+), 223 deletions(-)
diff --git a/src/main/java/org/apache/groovy/util/HiddenClassDefiner.java b/src/main/java/org/apache/groovy/util/HiddenClassDefiner.java
index 8880b2b426..4da39dd314 100644
--- a/src/main/java/org/apache/groovy/util/HiddenClassDefiner.java
+++ b/src/main/java/org/apache/groovy/util/HiddenClassDefiner.java
@@ -18,6 +18,7 @@
*/
package org.apache.groovy.util;
+import groovy.transform.Internal;
import org.codehaus.groovy.control.CompilerConfiguration;
import org.objectweb.asm.ClassReader;
import org.objectweb.asm.ClassWriter;
@@ -32,58 +33,59 @@ import java.util.Collections;
* Central facility for defining <em>hidden classes</em>
* (<a href="https://openjdk.org/jeps/371">JEP 371</a>).
*
- * <h2>Lookup ownership (read this first)</h2>
- * <p>{@link MethodHandles#lookup()} is <em>caller-sensitive</em>: it returns a
+ * <h2>Lookup ownership</h2>
+ * <p>{@link MethodHandles#lookup()} is caller-sensitive: it returns a
* full-privilege lookup only for the class that literally contains the call.
- * Production call sites therefore capture a lookup on the intended nest host
- * itself, for example:
+ * Production call sites capture a lookup on the intended nest host:
* <pre>{@code
- * // ReflectorLoader / ProxyGeneratorAdapter (or any nest host):
* private static final MethodHandles.Lookup LOOKUP = MethodHandles.lookup();
- *
* Class<?> hidden = HiddenClassDefiner.tryDefineNestmate(LOOKUP, bytecode, false);
* }</pre>
- * The {@link Lookup#lookupClass() lookup class} determines the hidden class's
- * defining loader, run-time package, protection domain, and nest host. A lookup
- * from {@code ReflectorLoader} is therefore <em>not</em> interchangeable with
- * one from {@code ProxyGeneratorAdapter} or from this utility: each hosts
- * nestmates in a different package / loader / nest. What is shared is only the
- * define policy (package alignment, {@code NESTMATE}+weak options, soft-fail).
+ * {@link Lookup#lookupClass()} fixes the hidden class's defining loader,
+ * package, protection domain, and nest host. Lookups from different runtime
+ * classes are not interchangeable as nest hosts even though they share the same
+ * module rights (see below).
+ *
+ * <p>The lookup captured inside this class is used only as the <em>caller</em>
+ * argument to {@link MethodHandles#privateLookupIn(Class, Lookup)} for the
+ * foreign-host overload — never as a production nest host.
*
- * <p>A lookup captured inside {@code HiddenClassDefiner} only has full privilege
- * for {@code HiddenClassDefiner} itself. It is used solely as the <em>caller</em>
- * argument to {@link MethodHandles#privateLookupIn(Class, Lookup)} in the
- * foreign-host overload — never as the nest host of production generators.
+ * <h2>Module rights vs nest host</h2>
+ * <p>Every production call site lives in the Groovy runtime, so {@code lookup()}
+ * always grants the same module-level access. Capturing it in different runtime
+ * classes does not open a third-party module that never opened itself to the
+ * runtime. What differs is the nest host (package / loader / nest membership),
+ * which still matters for unloadability and linkage.
*
- * <p>The overload {@link #tryDefineNestmate(Class, byte[], boolean)} is a
- * <em>best-effort</em> helper for foreign hosts (user classes Groovy does not
- * control). It succeeds only when the host's package is accessible to Groovy's
- * module (typical for unnamed-module application classes; not for sealed /
- * unopened packages such as those in {@code java.base}). Callers must always
- * handle a {@code null} result and fall back to {@link ClassLoader#defineClass}.
+ * <h2>What nestmates cover (modules A / B / C)</h2>
+ * <p>With modules A (Java library), B (Groovy program), C (Groovy runtime):
+ * <ol>
+ * <li><strong>Caller-owned lookup</strong>
+ * ({@link #tryDefineNestmate(Lookup, byte[], boolean)}): nestmate of a
+ * runtime class. Works when every type named by the bytecode is resolvable
+ * from the runtime loader. No private access into foreign modules.</li>
+ * <li><strong>Foreign host</strong>
+ * ({@link #tryDefineNestmate(Class, byte[], boolean)}): best-effort
+ * {@code privateLookupIn}. Succeeds when the host package is open to the
+ * runtime (typical for unnamed-module application classes); not for
+ * strongly encapsulated packages such as {@code java.lang}
+ * ({@link String} is the counter-example). Callers must handle
+ * {@code null} and fall back to {@link ClassLoader#defineClass}.</li>
+ * </ol>
+ * Nestmates cover C and often open/unnamed B. They do not tunnel private access
+ * into A. Visible {@code defineClass} is the intentional safety net.
*
* <h2>Soft-fail contract</h2>
- * <p>{@code try*} methods return {@code null} on expected failure modes so call
- * sites can fall back with one null check:
- * <ul>
- * <li>{@link IllegalAccessException}, {@link SecurityException}</li>
- * <li>{@link LinkageError}</li>
- * <li>{@link IllegalArgumentException}, {@link IndexOutOfBoundsException}
- * (invalid class-file bytes / ASM)</li>
- * <li>GraalVM native-image "feature unsupported" errors, detected by class
- * name ({@code com.oracle.svm.core.jdk.UnsupportedFeatureError}) without
- * a build dependency on GraalVM</li>
- * </ul>
- * Other {@link Error}s and unexpected failures are rethrown.
+ * <p>{@code try*} methods return {@code null} on expected failures
+ * ({@link IllegalAccessException}, {@link SecurityException}, {@link LinkageError},
+ * invalid class-file / ASM exceptions, GraalVM {@code UnsupportedFeatureError}
+ * matched by class name). Other {@link Error}s are rethrown.
*
- * <h2>Enablement (kill switch and native image)</h2>
- * <p>{@link #isEnabled()} is evaluated on each call (class definition is not a
- * hot path). That keeps {@code -Dgroovy.hidden.classes.disable=true} effective
- * under GraalVM native image, where a {@code static final} snapshot taken at
- * <em>build-time</em> class-init would freeze the wrong answer. When running
- * inside a native image ({@code org.graalvm.nativeimage.imagecode=runtime}),
- * enablement is {@code false}: runtime class definition is unsupported there,
- * so the soft path never attempts it.
+ * <h2>Enablement</h2>
+ * <p>{@link #isEnabled()} is evaluated per call so
+ * {@code -Dgroovy.hidden.classes.disable=true} works under native-image
+ * build-time init, and is always {@code false} when
+ * {@code org.graalvm.nativeimage.imagecode=runtime}.
*
* @since 6.0.0
* @see Lookup#defineHiddenClass(byte[], boolean, Lookup.ClassOption...)
@@ -95,9 +97,8 @@ public final class HiddenClassDefiner {
/**
* GraalVM property set in native images. Value {@code "runtime"} means the
- * current process is executing an already-built native image (as opposed to
- * {@code "buildtime"} during image generation). Package-private: not a
- * public configuration surface — use {@link #isEnabled()}.
+ * current process is executing an already-built native image. Not a public
+ * configuration surface — use {@link #isEnabled()}.
*/
static final String PROPERTY_NATIVE_IMAGE_CODE = "org.graalvm.nativeimage.imagecode";
@@ -112,14 +113,16 @@ public final class HiddenClassDefiner {
"com.oracle.svm.core.jdk.UnsupportedFeatureError";
/**
- * Lookup for <em>this</em> class only — used exclusively as the caller
- * argument to {@link MethodHandles#privateLookupIn(Class, Lookup)} in the
- * foreign-host overload. It is never used as a nest host for production
- * generators (those pass their own {@link MethodHandles#lookup()}).
+ * Lookup for <em>this</em> class only — caller argument to
+ * {@link MethodHandles#privateLookupIn(Class, Lookup)}, never a production
+ * nest host.
*/
private static final Lookup LOOKUP = MethodHandles.lookup();
- /** Nestmate + weak (eager unloading) — the only option set production uses. */
+ /**
+ * Nestmate with the default (weak) lifecycle. Default hidden classes are
+ * weakly held; {@link Lookup.ClassOption#STRONG} is not requested.
+ */
private static final Lookup.ClassOption[] NESTMATE_WEAK =
new Lookup.ClassOption[]{Lookup.ClassOption.NESTMATE};
@@ -129,16 +132,8 @@ public final class HiddenClassDefiner {
/**
* Whether hidden-class definition may be attempted in this process.
*
- * <p>Returns {@code false} when:
- * <ul>
- * <li>{@code -Dgroovy.hidden.classes.disable=true}, or</li>
- * <li>the process is a GraalVM native image at run time
- * ({@code org.graalvm.nativeimage.imagecode=runtime}).</li>
- * </ul>
- * Evaluated on each call so the kill switch remains effective when this
- * class is initialized at native-image <em>build</em> time.
- *
- * @return {@code true} when a {@code tryDefineNestmate} attempt is allowed
+ * @return {@code false} when the kill switch is set or when running inside
+ * a GraalVM native image at run time
*/
public static boolean isEnabled() {
if (isNativeImageRuntime()) {
@@ -149,13 +144,8 @@ public final class HiddenClassDefiner {
/**
* Defines {@code bytes} as a hidden nestmate of {@code lookup.lookupClass()}
- * with a weak lifecycle.
- *
- * <p>The lookup must have been obtained via {@link MethodHandles#lookup()}
- * <em>inside</em> the intended nest-host class (or otherwise carry full
- * privilege for that class). Which class called {@code lookup()} matters:
- * it fixes the hidden class's loader, package, and nest. The class-file
- * package is rewritten to match the lookup class before definition.
+ * with a weak lifecycle. The class-file package is rewritten to match the
+ * lookup class before definition.
*
* @param lookup full-privilege lookup for the nest host
* @param bytes class-file bytes
@@ -183,18 +173,12 @@ public final class HiddenClassDefiner {
}
/**
- * Best-effort definition of a hidden nestmate of a <em>foreign</em> host
- * class (one whose source Groovy does not control).
+ * Best-effort definition of a hidden nestmate of a <em>foreign</em> host.
+ * Uses {@code privateLookupIn} from this class; gated by
+ * {@link #canAttemptPrivateLookup(Class)}. Not a substitute for a host-owned
+ * lookup.
*
- * <p>Obtains a host lookup via
- * {@code MethodHandles.privateLookupIn(host, }lookup captured in this class)
- * and then delegates to {@link #tryDefineNestmate(Lookup, byte[], boolean)}.
- * This succeeds only when the host's package is accessible to Groovy's
- * module; it is <strong>not</strong> a substitute for a lookup created
- * inside the host.
- *
- * @param host nest host and class-loader / package donor; must be a
- * normal (non-hidden) reference type
+ * @param host nest host and class-loader / package donor
* @param bytes class-file bytes
* @param initialize {@code true} to run {@code <clinit>} immediately
* @return the hidden class, or {@code null} if private lookup or definition fails
@@ -203,7 +187,7 @@ public final class HiddenClassDefiner {
final Class<?> host,
final byte[] bytes,
final boolean initialize) {
- if (!isEnabled() || !isUsableHost(host) || bytes == null) {
+ if (!isEnabled() || !canAttemptPrivateLookup(host) || bytes == null) {
return null;
}
try {
@@ -216,33 +200,47 @@ public final class HiddenClassDefiner {
}
}
+ /**
+ * Internal policy: whether {@code privateLookupIn} from this utility into
+ * {@code host} is worth attempting.
+ *
+ * <p>Returns {@code false} for unusable host shapes and for named-module
+ * packages that are not open to the Groovy runtime (e.g. {@code String} in
+ * {@code java.base}). A {@code true} result does not guarantee success.
+ *
+ * <p>Not a stable user API — for runtime define policy and tests only.
+ *
+ * @param host candidate foreign nest host
+ * @return {@code true} when a private-lookup attempt is not known to be futile
+ * @since 6.0.0
+ */
+ @Internal
+ public static boolean canAttemptPrivateLookup(final Class<?> host) {
+ if (!isUsableHost(host)) {
+ return false;
+ }
+ final Module hostModule = host.getModule();
+ final Module callerModule = LOOKUP.lookupClass().getModule();
+ return hostModule.isOpen(host.getPackageName(), callerModule);
+ }
+
// -------------------------------------------------------------------------
// Internals (package-visible where tests need them)
// -------------------------------------------------------------------------
- /**
- * {@code true} when executing inside a GraalVM native image (not during
- * image build). Package-private for tests.
- */
+ /** {@code true} when executing inside a GraalVM native image (not during image build). */
static boolean isNativeImageRuntime() {
return NATIVE_IMAGE_CODE_RUNTIME.equals(System.getProperty(PROPERTY_NATIVE_IMAGE_CODE));
}
- /**
- * {@code true} when {@code e} is GraalVM's unsupported-feature error.
- * Package-private for tests.
- */
+ /** {@code true} when {@code e} is GraalVM's unsupported-feature error (name match). */
static boolean isUnsupportedFeatureError(final Error e) {
return e != null && UNSUPPORTED_FEATURE_ERROR.equals(e.getClass().getName());
}
/**
* Soft-fails GraalVM unsupported-feature errors as {@code null}; rethrows
- * every other {@link Error}. Package-private so unit tests cover the same
- * branch used by both {@code tryDefineNestmate} overloads.
- *
- * @param e error caught around define / privateLookupIn
- * @return {@code null} if soft-failed
+ * every other {@link Error}.
*/
static Class<?> softFailOrRethrow(final Error e) {
if (isUnsupportedFeatureError(e)) {
diff --git a/src/main/java/org/codehaus/groovy/reflection/ClassLoaderForClassArtifacts.java b/src/main/java/org/codehaus/groovy/reflection/ClassLoaderForClassArtifacts.java
index 6e1e17f112..2c61125207 100644
--- a/src/main/java/org/codehaus/groovy/reflection/ClassLoaderForClassArtifacts.java
+++ b/src/main/java/org/codehaus/groovy/reflection/ClassLoaderForClassArtifacts.java
@@ -31,20 +31,14 @@ import java.util.concurrent.atomic.AtomicInteger;
* classes such as generated meta-method dispatchers.
*
* <p>Since Groovy 6.0 this loader first attempts to define each artifact as a
- * <em>hidden nestmate</em> of the target class via
- * {@link HiddenClassDefiner#tryDefineNestmate(Class, byte[], boolean)} (best-effort
- * {@code privateLookupIn} into the target). Benefits when that succeeds:
- * <ul>
- * <li>non-discoverable by name;</li>
- * <li>same defining loader / package / protection domain as the target;</li>
- * <li>nestmate of the target;</li>
- * <li>weak lifecycle — eligible for eager unloading once the {@link Class}
- * object is unreachable.</li>
- * </ul>
+ * hidden nestmate of the target via
+ * {@link HiddenClassDefiner#tryDefineNestmate(Class, byte[], boolean)}. When
+ * that soft-fails (unopened package, unsuitable host, …) it falls back to
+ * {@link ClassLoader#defineClass} with the protection domain captured at
+ * construction. Module / nest-host policy is documented on
+ * {@link HiddenClassDefiner}.
*
- * <p>If hidden-class definition is refused (module not open, target unsuitable,
- * …) the loader falls back to {@link ClassLoader#defineClass} with the
- * protection domain captured at construction time.
+ * @see HiddenClassDefiner
*/
public class ClassLoaderForClassArtifacts extends ClassLoader {
diff --git a/src/main/java/org/codehaus/groovy/runtime/ProxyClassDefiner.java b/src/main/java/org/codehaus/groovy/runtime/ProxyClassDefiner.java
index 2d1c18d0cf..447a183d8c 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ProxyClassDefiner.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ProxyClassDefiner.java
@@ -31,30 +31,31 @@ import java.util.function.BiFunction;
* hidden nestmate when safe and falling back to a visible {@link ClassLoader}
* definition otherwise.
*
- * <p>Extracted from {@link ProxyGeneratorAdapter} so that class keeps focused on
- * bytecode generation while nest-host policy and define/fallback live here.
+ * <p>Extracted so {@link ProxyGeneratorAdapter} stays focused on bytecode
+ * generation. Nest-host and module policy live in
+ * {@link HiddenClassDefiner}; this class only applies proxy-specific rules.
*
* <h2>When a proxy stays visible</h2>
* <ul>
* <li>hidden classes are disabled;</li>
- * <li>any named dependency (super / delegate / implemented type) is itself
- * hidden — hidden types cannot appear as nominal field/super types;</li>
+ * <li>any named dependency is itself hidden;</li>
* <li>interface-style aggregates ({@code Object} super, no typed delegate,
- * at least one user interface) — these are routinely re-wrapped (e.g. by
- * MockFor) and must remain loadable by binary name.</li>
+ * at least one user interface) — must stay nameable for MockFor re-wrap.</li>
* </ul>
*
- * <h2>Nest host selection (at most one foreign try, then own Lookup)</h2>
+ * <h2>Nest host selection</h2>
* <ol>
- * <li>{@code delegateClass} if usable and its loader can see every dependency;</li>
+ * <li>{@code delegateClass} if a usable foreign host (open package + loader
+ * can see every dependency);</li>
* <li>else concrete {@code superClass} (not {@code Object}) under the same rule;</li>
- * <li>else the caller-owned {@link Lookup} from {@link ProxyGeneratorAdapter}
+ * <li>else caller-owned {@link Lookup} from {@link ProxyGeneratorAdapter}
* when that loader can see every dependency.</li>
* </ol>
- * Interfaces are not used as nest hosts: they add trials without helping the
- * common cases and complicate class-loader reasoning.
+ * Interfaces are not used as nest hosts. Module A/B/C coverage is documented on
+ * {@link HiddenClassDefiner}.
*
* @since 6.0.0
+ * @see HiddenClassDefiner
*/
final class ProxyClassDefiner {
@@ -115,7 +116,6 @@ final class ProxyClassDefiner {
}
}
- // Visible fallback — never mark as hidden.
type = visibleDefine.apply(binaryName, bytecode);
return new Result(type, false, resolvePublicConstructor(type, ctorArgs));
}
@@ -144,12 +144,13 @@ final class ProxyClassDefiner {
}
}
}
- // Interface aggregates must stay nameable for re-wrapping (MockFor, etc.).
return delegateClass != null || superClass != Object.class || !hasUserInterface(implClasses);
}
/**
* Single preferred foreign nest host, or {@code null} to skip to own Lookup.
+ * Unopened platform packages are rejected via
+ * {@link HiddenClassDefiner#canAttemptPrivateLookup(Class)}.
*/
static Class<?> preferredForeignHost(
final Class<?> superClass,
@@ -217,7 +218,6 @@ final class ProxyClassDefiner {
if (candidate == null) {
return null;
}
- // Ensure deferred linkage succeeds before we commit to the hidden path.
if (resolvePublicConstructor(candidate, ctorArgs) == null) {
return null;
}
@@ -236,12 +236,17 @@ final class ProxyClassDefiner {
}
}
+ /**
+ * Proxy-specific exclusions plus {@link HiddenClassDefiner#canAttemptPrivateLookup}
+ * (shape + module-open). {@code Object} / this adapter / sealed types are
+ * never foreign hosts.
+ */
private static boolean isCandidateHost(final Class<?> type) {
return type != null
&& type != Object.class
&& type != ProxyGeneratorAdapter.class
- && !isUnusableNamedType(type)
- && !type.isSealed();
+ && !type.isSealed()
+ && HiddenClassDefiner.canAttemptPrivateLookup(type);
}
private static boolean isUnusableNamedType(final Class<?> type) {
diff --git a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
index 09c2d1a0c4..0593062177 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
@@ -128,9 +128,8 @@ public class ProxyGeneratorAdapter extends ClassVisitor {
private static final AtomicLong PROXY_COUNTER = new AtomicLong();
/**
- * Full-privilege lookup for this class (caller-sensitive, captured here).
- * Used as the last-resort nest host when no foreign host is suitable —
- * never replaced by {@code privateLookupIn} on a utility class.
+ * Full-privilege lookup for this class — last-resort nest host when no
+ * foreign host is suitable. See {@link org.apache.groovy.util.HiddenClassDefiner}.
*/
private static final Lookup LOOKUP = MethodHandles.lookup();
diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/ReflectorLoader.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/ReflectorLoader.java
index db526d6201..144fbeff1d 100644
--- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ReflectorLoader.java
+++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ReflectorLoader.java
@@ -18,48 +18,30 @@
*/
package org.codehaus.groovy.runtime.metaclass;
-import org.apache.groovy.util.HiddenClassDefiner;
import org.codehaus.groovy.runtime.Reflector;
-import java.lang.invoke.MethodHandles;
-import java.lang.invoke.MethodHandles.Lookup;
import java.security.ProtectionDomain;
import java.util.HashMap;
import java.util.Map;
/**
- * Reflector creation helper. This class is used to define the {@link Reflector} classes.
+ * Reflector creation helper historically used to define {@link Reflector}
+ * subclasses.
*
- * <p>For each {@link ClassLoader} such a loader will be created by {@code MetaClass}.
- * Special about this loader is that it knows the classes from the Groovy runtime.
- * The {@link Reflector} class is resolved in different ways: during the definition
- * of a class {@link Reflector} will resolve to the {@link Reflector} class of the
- * runtime, even if there is another {@link Reflector} class in the parent loader.
- * After the new class is defined {@link Reflector} will resolve like other Groovy
- * classes. This loader is able to resolve all Groovy classes even if the parent
- * does not know them, but the parent serves first (Reflector during a class
- * definition is different).
+ * <p><strong>Deprecated since 6.0.0.</strong> Production MetaClass / call-site
+ * paths no longer construct or use this loader; the only remaining references
+ * are tests. Prefer modern reflective / method-handle dispatch. Retained for
+ * binary compatibility only; will be removed in a future major release.
*
- * <p>Since Groovy 6.0 this loader preferentially defines each generated
- * Reflector class as a <em>hidden nestmate</em> of {@code ReflectorLoader}
- * itself (via a {@link Lookup} captured in this class — the correct
- * caller-sensitive way to obtain full privilege). Hidden classes are
- * non-discoverable and eligible for eager unloading. When that fails the
- * classic {@link ClassLoader#defineClass} path is used transparently.
+ * <p>Special about this loader is that it knows the classes from the Groovy
+ * runtime. During class definition {@link Reflector} resolves to the runtime's
+ * {@link Reflector} even if a different one exists in the parent loader.
*
- * <p>{@code name} passed to {@link #defineClass(String, byte[], ProtectionDomain)}
- * is the cache / fallback binary name only: a successful hidden definition
- * yields a JVM-assigned name (with a {@code /} suffix) that is not loadable
- * by {@code name}.
+ * @deprecated No production callers remain; kept only for binary compatibility.
*/
+@Deprecated(since = "6.0.0", forRemoval = true)
public class ReflectorLoader extends ClassLoader {
- /**
- * Full-privilege lookup for this class, captured at class-init time.
- * Used as the nest host for hidden Reflector subclasses.
- */
- private static final Lookup LOOKUP = MethodHandles.lookup();
-
private boolean inDefine = false;
private final Map<String, Class<?>> loadedClasses = new HashMap<>();
private final ClassLoader delegatationLoader;
@@ -100,22 +82,15 @@ public class ReflectorLoader extends ClassLoader {
}
/**
- * Helper method to define Reflector classes.
- *
- * <p>Prefers a hidden nestmate of {@code ReflectorLoader} and falls back to
- * the classic {@link ClassLoader#defineClass} path when that is not possible.
+ * Defines a Reflector class via {@link ClassLoader#defineClass} and caches
+ * it under {@code name}.
*
- * <p>{@code name} is stored in the per-loader cache and used for the visible
- * fallback definition; it is <em>not</em> the runtime name of a successfully
- * defined hidden class.
+ * <p>Sets {@code inDefine} during definition so {@link Reflector} resolves
+ * to the runtime class.
*
- * <p>This method sets the {@code inDefine} flag to {@code true} during
- * class definition to ensure {@link Reflector} is resolved correctly.
- *
- * @param name cache key / fallback binary name of the Reflector class
+ * @param name cache key and binary name of the Reflector class
* @param bytecode the bytecode of the Reflector class
- * @param domain the protection domain for the fallback visible-class
- * definition; not used when the hidden-class path succeeds
+ * @param domain the protection domain
* @return the newly defined class
*/
public synchronized Class<?> defineClass(
@@ -124,12 +99,9 @@ public class ReflectorLoader extends ClassLoader {
final ProtectionDomain domain) {
inDefine = true;
try {
- final Class<?> cls = defineReflectorClass(name, bytecode, domain);
+ final Class<?> cls = defineClass(name, bytecode, 0, bytecode.length, domain);
+ resolveClass(cls);
loadedClasses.put(name, cls);
- if (!cls.isHidden()) {
- // Hidden classes do not need resolveClass(); visible classes do.
- resolveClass(cls);
- }
return cls;
} finally {
inDefine = false;
@@ -138,8 +110,6 @@ public class ReflectorLoader extends ClassLoader {
/**
* Creates a new ReflectorLoader with the specified parent class loader.
- * This loader is responsible for defining Reflector classes that can resolve
- * the Reflector class from the Groovy runtime correctly.
*
* @param parent the parent class loader (should never be {@code null})
*/
@@ -151,10 +121,6 @@ public class ReflectorLoader extends ClassLoader {
/**
* Retrieves a previously defined Reflector class by name from the cache.
*
- * <p>The key is the logical binary name passed to
- * {@link #defineClass(String, byte[], ProtectionDomain)}, not the
- * JVM-assigned hidden-class name.
- *
* @param name the fully qualified name of the Reflector class
* @return the Reflector class if it has been defined, or {@code null} otherwise
*/
@@ -166,9 +132,8 @@ public class ReflectorLoader extends ClassLoader {
* Generates the fully qualified name of a Reflector class for the given class.
*
* <p>For {@code java.*} classes the name is prefixed with {@code "gjdk."} to
- * avoid the restricted {@code java.} package namespace. Array types are
- * handled specially with a {@code "_GroovyReflectorArray"} suffix and nesting
- * level indicators.
+ * avoid the restricted {@code java.} package namespace. Array types use a
+ * {@code "_GroovyReflectorArray"} suffix (with nesting level when {@code > 1}).
*
* @param theClass the class for which to generate the Reflector name
* @return the fully qualified name of the Reflector class
@@ -206,19 +171,4 @@ public class ReflectorLoader extends ClassLoader {
return name;
}
}
-
- /**
- * Prefers a hidden nestmate of this loader class (caller-owned
- * {@link #LOOKUP}); falls back to a visible class under {@code name}.
- */
- private Class<?> defineReflectorClass(
- final String name,
- final byte[] bytecode,
- final ProtectionDomain domain) {
- final Class<?> hidden = HiddenClassDefiner.tryDefineNestmate(LOOKUP, bytecode, false);
- if (hidden != null) {
- return hidden;
- }
- return defineClass(name, bytecode, 0, bytecode.length, domain);
- }
}
diff --git a/src/test/groovy/org/apache/groovy/util/HiddenClassDefinerTest.groovy b/src/test/groovy/org/apache/groovy/util/HiddenClassDefinerTest.groovy
index 2db8d63b48..138ec5df71 100644
--- a/src/test/groovy/org/apache/groovy/util/HiddenClassDefinerTest.groovy
+++ b/src/test/groovy/org/apache/groovy/util/HiddenClassDefinerTest.groovy
@@ -44,7 +44,8 @@ import static org.objectweb.asm.Opcodes.V17
*
* <p>The preferred production path is {@code tryDefineNestmate(Lookup, ...)}
* with a lookup captured in this test class itself (caller-sensitive). The
- * foreign-host overload is covered separately as a best-effort path.
+ * foreign-host overload and its internal {@code canAttemptPrivateLookup}
+ * pre-filter are covered as best-effort policy.
*
* @since 6.0.0
*/
@@ -56,8 +57,7 @@ class HiddenClassDefinerTest {
* {@code $$InjectedInvoker} hidden class on some JDKs; pin to this class
* with {@code privateLookupIn} so nest-host assertions are stable.
* Production Java call sites capture {@code MethodHandles.lookup()} in a
- * {@code static final} field of the nest-host class itself (see
- * {@code ReflectorLoader}, {@code ProxyGeneratorAdapter}).
+ * {@code static final} field of the nest-host class itself.
*/
private static final Lookup LOOKUP = MethodHandles.privateLookupIn(
HiddenClassDefinerTest, MethodHandles.lookup())
@@ -183,8 +183,9 @@ class HiddenClassDefinerTest {
@Test
void testLookupClassDeterminesNestHostNotUtilityClass() {
- // blackdrag: which class called MethodHandles.lookup() matters — the nest
- // host is lookup.lookupClass(), not HiddenClassDefiner.
+ // Which class called MethodHandles.lookup() matters — the nest host is
+ // lookup.lookupClass(), not HiddenClassDefiner (even though module rights
+ // are the same for every runtime-captured lookup).
byte[] bytes = minimalClassBytes('org/apache/groovy/util/NestHostCheck')
Class<?> hidden = HiddenClassDefiner.tryDefineNestmate(LOOKUP, bytes, true)
assertNotNull(hidden)
@@ -254,9 +255,34 @@ class HiddenClassDefinerTest {
}
// -------------------------------------------------------------------------
- // Best-effort API: foreign host
+ // Module-open pre-filter + best-effort foreign host
// -------------------------------------------------------------------------
+ @Test
+ void testCanAttemptPrivateLookupPrefilter() {
+ // Usable same-module / unnamed application host
+ assertTrue(HiddenClassDefiner.canAttemptPrivateLookup(HiddenClassDefinerTest))
+
+ // Unusable shapes
+ assertFalse(HiddenClassDefiner.canAttemptPrivateLookup(null))
+ assertFalse(HiddenClassDefiner.canAttemptPrivateLookup(Integer.TYPE))
+ assertFalse(HiddenClassDefiner.canAttemptPrivateLookup(String[].class))
+
+ // Nestmate of String needs private privileges into java.base.
+ // On a stock modular JDK java.lang is not open to the runtime → false.
+ // Environments that pass --add-opens may report true; either is valid,
+ // but the pre-filter must match what privateLookupIn would allow.
+ boolean stringOpen = String.module.isOpen('java.lang', HiddenClassDefiner.module)
+ assertEquals(stringOpen, HiddenClassDefiner.canAttemptPrivateLookup(String),
+ 'pre-filter must mirror Module.isOpen for java.lang')
+
+ // Hidden class cannot host further nestmates
+ Class<?> hidden = HiddenClassDefiner.tryDefineNestmate(
+ LOOKUP, minimalClassBytes('org/apache/groovy/util/PrefilterHidden'), true)
+ assertNotNull(hidden)
+ assertFalse(HiddenClassDefiner.canAttemptPrivateLookup(hidden))
+ }
+
@Test
void testTryDefineNestmateWithForeignHost() {
// Same-module unnamed class — privateLookupIn from HiddenClassDefiner works
@@ -281,15 +307,21 @@ class HiddenClassDefinerTest {
}
@Test
- void testForeignHostIntoJavaBaseTypicallyFails() {
- // java.lang is not open to unnamed modules → privateLookupIn fails → null
- // (unless the JVM was launched with --add-opens). Soft API must not throw.
+ void testForeignHostIntoJavaBaseDoesNotThrow() {
+ // Try to create a nestmate for String — privateLookupIn into java.lang
+ // fails on a stock modular JDK. Soft API must return null, never throw.
byte[] bytes = minimalClassBytes('java/lang/ShouldNotAppear')
Class<?> result = HiddenClassDefiner.tryDefineNestmate(String, bytes, true)
- // On a stock JDK this is null; if the environment opens java.lang, a
- // hidden class is still a valid outcome. Never throw.
- if (result != null) {
+ if (!HiddenClassDefiner.canAttemptPrivateLookup(String)) {
+ assertNull(result, 'unopened java.lang must soft-fail to null')
+ } else if (result != null) {
+ // Only when the JVM was launched with --add-opens java.base/java.lang=...
assertTrue(result.isHidden())
}
}
+
+ @Test
+ void testForeignHostNullBytesRejected() {
+ assertNull(HiddenClassDefiner.tryDefineNestmate(HiddenClassDefinerTest, null, true))
+ }
}
diff --git a/src/test/groovy/org/codehaus/groovy/reflection/ClassLoaderForClassArtifactsTest.groovy b/src/test/groovy/org/codehaus/groovy/reflection/ClassLoaderForClassArtifactsTest.groovy
index 0f257c4363..b606b61fb5 100644
--- a/src/test/groovy/org/codehaus/groovy/reflection/ClassLoaderForClassArtifactsTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/reflection/ClassLoaderForClassArtifactsTest.groovy
@@ -70,6 +70,21 @@ class ClassLoaderForClassArtifactsTest {
assertNotNull(cls.getDeclaredConstructor().newInstance())
}
+ @Test
+ void testDefineFallsBackForUnopenedPlatformHost() {
+ // Nestmate of String fails without --add-opens. Visible defineClass
+ // fallback must still produce a usable class.
+ def loader = new ClassLoaderForClassArtifacts(String)
+ String name = loader.createClassName('platformFallback')
+ Class<?> cls = loader.define(name, minimalBytes(name.replace('.', '/')))
+ assertNotNull(cls)
+ assertNotNull(cls.getDeclaredConstructor().newInstance())
+ if (!HiddenClassDefiner.canAttemptPrivateLookup(String)) {
+ assertFalse(cls.isHidden(),
+ 'unopened java.lang.String host must fall back to a visible class')
+ }
+ }
+
@Test
void testDefineClassAndGetConstructor() {
def loader = new ClassLoaderForClassArtifacts(Host)
@@ -100,4 +115,24 @@ class ClassLoaderForClassArtifactsTest {
'java.* artifacts must be renamed out of the restricted package')
assertTrue(javaName.startsWith('java_lang_String'))
}
+
+ @Test
+ void testCreateClassNameFromMethod() {
+ def loader = new ClassLoaderForClassArtifacts(Host)
+ def method = Object.getDeclaredMethod('toString')
+ String name = loader.createClassName(method)
+ assertTrue(name.contains('toString'))
+ }
+
+ @Test
+ void testLoadClassFindsPreviouslyDefinedVisibleClass() {
+ // When hidden path is used the binary name is not loadable; force a
+ // platform host so fallback defines a visible class under `name`.
+ def loader = new ClassLoaderForClassArtifacts(String)
+ String name = loader.createClassName('loadable')
+ Class<?> defined = loader.define(name, minimalBytes(name.replace('.', '/')))
+ if (!defined.isHidden()) {
+ assertEquals(defined, loader.loadClass(name))
+ }
+ }
}
diff --git a/src/test/groovy/org/codehaus/groovy/runtime/ProxyClassDefinerTest.groovy b/src/test/groovy/org/codehaus/groovy/runtime/ProxyClassDefinerTest.groovy
index 75fed94b35..ab078a839c 100644
--- a/src/test/groovy/org/codehaus/groovy/runtime/ProxyClassDefinerTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/runtime/ProxyClassDefinerTest.groovy
@@ -58,6 +58,20 @@ class ProxyClassDefinerTest {
cw.toByteArray()
}
+ private static byte[] classWithMapCtor(String internalName) {
+ def cw = new ClassWriter(0)
+ cw.visit(V17, ACC_PUBLIC, internalName, null, 'java/lang/Object', null)
+ def mv = cw.visitMethod(ACC_PUBLIC, '<init>', '(Ljava/util/Map;)V', null, null)
+ mv.visitCode()
+ mv.visitVarInsn(ALOAD, 0)
+ mv.visitMethodInsn(INVOKESPECIAL, 'java/lang/Object', '<init>', '()V', false)
+ mv.visitInsn(RETURN)
+ mv.visitMaxs(1, 2)
+ mv.visitEnd()
+ cw.visitEnd()
+ cw.toByteArray()
+ }
+
@Test
void testMayDefineHiddenRejectsInterfaceAggregates() {
// Object super + user interface + no delegate → must stay nameable
@@ -80,13 +94,36 @@ class ProxyClassDefinerTest {
}
@Test
- void testPreferredForeignHostPrefersDelegateThenSuper() {
- assertEquals(ArrayList, ProxyClassDefiner.preferredForeignHost(
- Object, ArrayList, [Object, ArrayList] as Set))
+ void testMayDefineHiddenWithoutUserInterfaces() {
+ // null / only Object / GroovyObject → no user interface → may be hidden
+ if (!HiddenClassDefiner.isEnabled()) return
+ assertTrue(ProxyClassDefiner.mayDefineHidden(Object, null, null))
+ assertTrue(ProxyClassDefiner.mayDefineHidden(Object, null, [Object] as Set))
+ assertTrue(ProxyClassDefiner.mayDefineHidden(Object, null, [Object, GroovyObject] as Set))
+ }
+
+ @Test
+ void testPreferredForeignHostPrefersOpenHostsAndSkipsPlatform() {
+ // Application host in an open / unnamed package is preferred
assertEquals(ConcreteHost, ProxyClassDefiner.preferredForeignHost(
ConcreteHost, null, [ConcreteHost] as Set))
+
+ // Delegate preferred over super when both are candidates
+ assertEquals(ConcreteHost, ProxyClassDefiner.preferredForeignHost(
+ Object, ConcreteHost, [Object, ConcreteHost] as Set))
+
+ // No usable foreign host for pure interface aggregate shape
assertNull(ProxyClassDefiner.preferredForeignHost(
Object, null, [Object, Iterator] as Set))
+
+ // ArrayList lives in java.base and is not open to the runtime on a stock
+ // JDK → must not be selected as foreign host (avoids a doomed
+ // privateLookupIn). Own Lookup / visible fallback handle it.
+ if (!HiddenClassDefiner.canAttemptPrivateLookup(ArrayList)) {
+ assertNull(ProxyClassDefiner.preferredForeignHost(
+ Object, ArrayList, [Object, ArrayList] as Set),
+ 'unopened java.util.ArrayList must not be a foreign host')
+ }
}
@Test
@@ -99,6 +136,23 @@ class ProxyClassDefinerTest {
assertTrue(ProxyClassDefiner.loaderCanResolve(childType, childType))
// Bootstrap types always visible
assertTrue(ProxyClassDefiner.loaderCanResolve(childType, String))
+ // null / primitive types are always "resolvable"
+ assertTrue(ProxyClassDefiner.loaderCanResolve(String, null))
+ assertTrue(ProxyClassDefiner.loaderCanResolve(String, Integer.TYPE))
+ }
+
+ @Test
+ void testCanResolveAllRequiresEveryDependency() {
+ assertTrue(ProxyClassDefiner.canResolveAll(
+ ConcreteHost, ConcreteHost, null, [ConcreteHost] as Set))
+ assertTrue(ProxyClassDefiner.canResolveAll(
+ ConcreteHost, ConcreteHost, null, null))
+
+ def childLoader = new GroovyClassLoader(this.class.classLoader)
+ Class<?> childType = childLoader.parseClass('class Unseen {}')
+ // Host in parent loader cannot see a child-defined type
+ assertFalse(ProxyClassDefiner.canResolveAll(
+ String, Object, null, [childType] as Set))
}
@Test
@@ -140,12 +194,93 @@ class ProxyClassDefinerTest {
fallback,
new Class<?>[0])
assertNotNull(result.type)
- // Own LOOKUP (this test class) may not host ConcreteHost package;
- // foreign host ConcreteHost should succeed when privateLookupIn works.
+ // Foreign host ConcreteHost should succeed when privateLookupIn works.
if (result.hidden) {
assertTrue(result.type.isHidden())
assertEquals(ConcreteHost.nestHost, result.type.nestHost)
}
assertNotNull(result.constructor)
}
+
+ @Test
+ void testDefineUsesOwnLookupWhenForeignHostUnopened() {
+ if (!HiddenClassDefiner.isEnabled()) return
+ // Typed delegate is ArrayList (java.base). Foreign try is skipped when
+ // the package is not open; own Lookup from this test class may still
+ // host the nestmate if every dependency is resolvable.
+ def bytes = publicNoArgClass('org/codehaus/groovy/runtime/OwnLookupHost')
+ def fallback = { String name, byte[] b ->
+ new GroovyClassLoader(this.class.classLoader).defineClass(name, b)
+ }
+ def ownLookup = MethodHandles.privateLookupIn(
+ ProxyClassDefinerTest, MethodHandles.lookup())
+ def result = ProxyClassDefiner.define(
+ bytes,
+ 'org.codehaus.groovy.runtime.OwnLookupHost',
+ Object,
+ ArrayList,
+ [Object, ArrayList] as Set,
+ ownLookup,
+ fallback,
+ new Class<?>[0])
+ assertNotNull(result.type)
+ assertNotNull(result.constructor)
+ if (result.hidden && !HiddenClassDefiner.canAttemptPrivateLookup(ArrayList)) {
+ // Nest host is the own-lookup class, not ArrayList
+ assertEquals(ProxyClassDefinerTest.nestHost, result.type.nestHost)
+ }
+ }
+
+ @Test
+ void testAcceptRejectsMissingPublicConstructor() {
+ if (!HiddenClassDefiner.isEnabled()) return
+ // Class with only no-arg ctor, but we ask for Map ctor → accept() nulls out
+ // and define falls back to visible (which also won't have Map ctor unless we define it).
+ def bytes = publicNoArgClass('org/codehaus/groovy/runtime/NoMapCtor')
+ def fallback = { String name, byte[] b ->
+ new GroovyClassLoader(this.class.classLoader).defineClass(name, b)
+ }
+ def result = ProxyClassDefiner.define(
+ bytes,
+ 'org.codehaus.groovy.runtime.NoMapCtor',
+ ConcreteHost,
+ null,
+ [ConcreteHost] as Set,
+ MethodHandles.lookup(),
+ fallback,
+ new Class<?>[]{Map})
+ // Visible fallback also has no Map ctor → constructor is null
+ assertNotNull(result.type)
+ assertNull(result.constructor)
+ assertFalse(result.hidden)
+ }
+
+ @Test
+ void testResolvePublicConstructor() {
+ assertNotNull(ProxyClassDefiner.resolvePublicConstructor(String, new Class<?>[]{String}))
+ assertNull(ProxyClassDefiner.resolvePublicConstructor(String, new Class<?>[]{Object}))
+ }
+
+ @Test
+ void testDefineHiddenWithMatchingCtorArgs() {
+ if (!HiddenClassDefiner.isEnabled()) return
+ def bytes = classWithMapCtor('org/codehaus/groovy/runtime/MapCtorHidden')
+ def fallback = { String name, byte[] b ->
+ new GroovyClassLoader(this.class.classLoader).defineClass(name, b)
+ }
+ def result = ProxyClassDefiner.define(
+ bytes,
+ 'org.codehaus.groovy.runtime.MapCtorHidden',
+ ConcreteHost,
+ null,
+ [ConcreteHost] as Set,
+ MethodHandles.lookup(),
+ fallback,
+ new Class<?>[]{Map})
+ assertNotNull(result.type)
+ assertNotNull(result.constructor)
+ if (result.hidden) {
+ assertTrue(result.type.isHidden())
+ }
+ }
}
diff --git a/src/test/groovy/org/codehaus/groovy/runtime/metaclass/ReflectorLoaderTest.groovy b/src/test/groovy/org/codehaus/groovy/runtime/metaclass/ReflectorLoaderTest.groovy
index 2837839ab7..452e655d31 100644
--- a/src/test/groovy/org/codehaus/groovy/runtime/metaclass/ReflectorLoaderTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/runtime/metaclass/ReflectorLoaderTest.groovy
@@ -18,13 +18,14 @@
*/
package org.codehaus.groovy.runtime.metaclass
-import org.apache.groovy.util.HiddenClassDefiner
import org.codehaus.groovy.runtime.Reflector
import org.junit.jupiter.api.Test
import org.objectweb.asm.ClassWriter
import static org.junit.jupiter.api.Assertions.assertEquals
+import static org.junit.jupiter.api.Assertions.assertFalse
import static org.junit.jupiter.api.Assertions.assertNotNull
+import static org.junit.jupiter.api.Assertions.assertNull
import static org.junit.jupiter.api.Assertions.assertTrue
import static org.objectweb.asm.Opcodes.ACC_PUBLIC
import static org.objectweb.asm.Opcodes.ALOAD
@@ -33,8 +34,10 @@ import static org.objectweb.asm.Opcodes.RETURN
import static org.objectweb.asm.Opcodes.V17
/**
- * Covers the hidden-class path in {@link ReflectorLoader}.
+ * Minimal smoke coverage for deprecated {@link ReflectorLoader} binary-compat
+ * surface — not a place to grow policy tests.
*/
+@SuppressWarnings('deprecation')
class ReflectorLoaderTest {
private static byte[] reflectorSubclassBytes(String internalName) {
@@ -54,7 +57,7 @@ class ReflectorLoaderTest {
}
@Test
- void testDefineClassPrefersHiddenNestmateOfReflectorLoader() {
+ void testDefineClassIsVisibleAndCached() {
def loader = new ReflectorLoader(this.class.classLoader)
String name = ReflectorLoader.getReflectorName(StringBuilder)
Class<?> cls = loader.defineClass(
@@ -63,20 +66,24 @@ class ReflectorLoaderTest {
this.class.protectionDomain)
assertNotNull(cls)
assertTrue(Reflector.isAssignableFrom(cls))
- if (HiddenClassDefiner.isEnabled()) {
- assertTrue(cls.isHidden())
- // Nest host is ReflectorLoader (caller-owned LOOKUP), not Reflector
- assertEquals(ReflectorLoader.nestHost, cls.nestHost)
- }
+ assertFalse(cls.isHidden(), 'deprecated loader defines visible classes only')
+ assertEquals(name, cls.name)
assertNotNull(cls.getDeclaredConstructor().newInstance())
- // Cache key is the logical name, not the hidden runtime name
assertEquals(cls, loader.getLoadedClass(name))
+ assertNull(loader.getLoadedClass('no.such.Reflector'))
}
@Test
void testGetReflectorNameForJavaAndUserTypes() {
- assertTrue(ReflectorLoader.getReflectorName(String).startsWith('gjdk.'))
- assertTrue(ReflectorLoader.getReflectorName(String[]).contains('Array'))
+ // Non-array java.* types get the gjdk. prefix (restricted package).
+ assertEquals('gjdk.java.lang.String_GroovyReflector',
+ ReflectorLoader.getReflectorName(String))
+ // Arrays use getName() form "[L…;" which does not start with "java.", so
+ // the historical path has no gjdk. prefix — preserve that contract.
+ assertEquals('java.lang.String_GroovyReflectorArray',
+ ReflectorLoader.getReflectorName(String[]))
+ assertEquals('java.lang.String_GroovyReflectorArray2',
+ ReflectorLoader.getReflectorName(String[][]))
assertTrue(ReflectorLoader.getReflectorName(ReflectorLoaderTest).endsWith('_GroovyReflector'))
}
}