You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/09/02 11:21:46 UTC
(groovy) branch master updated (8e017c0a83 -> 3c3a99f0ce)
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
from 8e017c0a83 cleanup: remove legacy .muse config
new 3e11ee970e GROOVY-12314: MOP: treat a field whose access cannot be established as absent
new 76ccd4990b GROOVY-12314: CachedField: cache the deep-reflection handles, drop the synchronization
new 3c3a99f0ce GROOVY-12314: MetaClassImpl: replace reflective-failure catches with a use-site pre-check
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/main/java/groovy/lang/MetaClassImpl.java | 25 +++-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 26 ++--
.../codehaus/groovy/reflection/CachedField.java | 92 +++++++-----
.../transform/stc/StaticTypeCheckingVisitor.java | 46 +++++-
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 24 +--
...y => ParameterizedReceiverTestExtension.groovy} | 14 +-
src/test/groovy/bugs/Groovy12310.groovy | 118 +++++++++++++++
src/test/groovy/bugs/Groovy12314.groovy | 164 +++++++++++++++++++++
.../test/groovy/bugs/Groovy12314Support.java | 19 +--
src/test/groovy/groovy/MapTest.groovy | 33 +++++
.../stc/FieldsAndPropertiesSTCTest.groovy | 85 +++++++++++
.../packageScope/DifferentPackageTest.groovy | 6 +-
12 files changed, 571 insertions(+), 81 deletions(-)
copy src/test-resources/groovy/transform/stc/{UndefinedVariableTestExtension.groovy => ParameterizedReceiverTestExtension.groovy} (69%)
create mode 100644 src/test/groovy/bugs/Groovy12310.groovy
create mode 100644 src/test/groovy/bugs/Groovy12314.groovy
copy subprojects/groovy-ant/src/test-resources/org/codehaus/groovy/ant/IncorrectGenericsUsage.java => src/test/groovy/bugs/Groovy12314Support.java (70%)
(groovy) 02/03: GROOVY-12314: CachedField: cache the deep-reflection handles, drop the synchronization
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 76ccd4990b62854495cc431d957b2c7fceab57ec
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Sep 2 09:55:24 2026 +1000
GROOVY-12314: CachedField: cache the deep-reflection handles, drop the synchronization
Review follow-up. Once setAccessible succeeds, a getter/setter handle
unreflected through Groovy's own lookup is caller-independent and is
created once and cached; a handle from a caller's lookup carries that
caller's access rights and is never cached. The once-only deep-reflection
attempt is lock-free: a concurrent duplicate is benign (idempotent
setAccessible on the same Field, equivalent handles), and recording the
attempt only after it completes gives any thread that observes it a
happens-before edge to the outcome and the field's accessibility, so the
Field.get/set paths need no blocking either. No deep setter is created
for a final field, so writing one keeps failing the caller's access
check. asAccessMethod now tries deep reflection before the caller
lookup; on the supported JDKs setAccessible either succeeds silently or
throws, so the GROOVY-9144/9596 unreflect-first ordering no longer buys
anything and every access outcome is unchanged.
---
.../codehaus/groovy/reflection/CachedField.java | 84 ++++++++++------------
1 file changed, 39 insertions(+), 45 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedField.java b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
index 97ef6bba43..5889584914 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedField.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
@@ -46,19 +46,27 @@ public class CachedField extends MetaProperty {
}
private final Field field;
- private volatile boolean madeAccessible;
- private boolean accessAttempted; // guarded by synchronization on this
- private void makeAccessible() {
- // at most one attempt, remembering either outcome: a failed attempt (strongly
- // encapsulated declaring class) cannot succeed later. The attempt is recorded
- // only once it has completed, under synchronization, so a concurrent caller
- // waits instead of reading the field before setAccessible has taken effect.
- synchronized (this) {
- if (!accessAttempted) {
- madeAccessible = ReflectionUtils.makeAccessibleInPrivilegedAction(field).isPresent();
- accessAttempted = true;
- }
+ private volatile boolean madeAccessible; // outcome of the deep-reflection attempt
+ private volatile boolean accessAttempted; // Groovy's deep-reflection path has been tried
+ private volatile MethodHandle getter, setter; // deep-reflection handles, which are
+ // caller-independent and so may be cached; a handle obtained from a caller's
+ // lookup carries that caller's access rights and is never cached
+
+ /**
+ * Tries once to establish deep-reflection access to the field, remembering
+ * either outcome: a failed attempt (strongly encapsulated declaring class)
+ * cannot succeed later. Lock-free: a concurrent duplicate attempt is benign
+ * (both threads force the same {@code Field}, idempotently), and the attempt
+ * is recorded only after it completed, so a thread that observes
+ * {@code accessAttempted} also observes the outcome and the field's
+ * accessibility.
+ */
+ private boolean makeAccessible() {
+ if (!accessAttempted) {
+ madeAccessible = ReflectionUtils.makeAccessibleInPrivilegedAction(field).isPresent();
+ accessAttempted = true;
}
+ return madeAccessible;
}
/**
@@ -130,53 +138,39 @@ public class CachedField extends MetaProperty {
/**
* Creates a method handle that provides getter access to this field via MethodHandles API.
- * Attempts to unreflect the field, automatically making it accessible if needed.
+ * When deep-reflection access can be established ({@code setAccessible} on the field),
+ * the resulting handle is caller-independent and is created once and cached; otherwise
+ * the given caller lookup decides, and its handle -- which carries that caller's access
+ * rights -- is never cached.
*
- * @param lookup the method handles lookup context
+ * @param lookup the method handles lookup context of the caller
* @return a method handle providing getter access to this field
- * @throws IllegalAccessException if the field cannot be accessed even with accessibility adjustments
+ * @throws IllegalAccessException if neither deep reflection nor the caller's lookup can access the field
*/
public MethodHandle asAccessMethod(final MethodHandles.Lookup lookup) throws IllegalAccessException {
- try {
- return lookup.unreflectGetter(field);
- } catch (IllegalAccessException e) {
- if (!madeAccessible) {
- try {
- makeAccessible();
- return lookup.unreflectGetter(field);
- } catch (IllegalAccessException ignore) {
- } catch (Throwable t) {
- e.addSuppressed(t);
- }
- }
- throw e;
+ MethodHandle h = getter;
+ if (h == null && makeAccessible()) {
+ h = getter = MethodHandles.lookup().unreflectGetter(field); // cannot fail: access was forced
}
+ return h != null ? h : lookup.unreflectGetter(field);
}
/**
* Creates a method handle that writes this field via the MethodHandles API,
- * the mutating counterpart of {@link #asAccessMethod(MethodHandles.Lookup)}.
- * Attempts to unreflect the field, automatically making it accessible if needed.
+ * the mutating counterpart of {@link #asAccessMethod(MethodHandles.Lookup)},
+ * with the same caching policy. No deep-reflection handle is created for a
+ * final field: writing it must keep failing the caller's access check.
*
- * @param lookup the method handles lookup context
+ * @param lookup the method handles lookup context of the caller
* @return a setter handle of type {@code (declaringClass, fieldType)void}
- * @throws IllegalAccessException if the field cannot be accessed even with accessibility adjustments
+ * @throws IllegalAccessException if neither deep reflection nor the caller's lookup can write the field
* @since 6.0.0
*/
public MethodHandle asWriteAccessMethod(final MethodHandles.Lookup lookup) throws IllegalAccessException {
- try {
- return lookup.unreflectSetter(field);
- } catch (IllegalAccessException e) {
- if (!madeAccessible) {
- try {
- makeAccessible();
- return lookup.unreflectSetter(field);
- } catch (IllegalAccessException ignore) {
- } catch (Throwable t) {
- e.addSuppressed(t);
- }
- }
- throw e;
+ MethodHandle h = setter;
+ if (h == null && !isFinal() && makeAccessible()) {
+ h = setter = MethodHandles.lookup().unreflectSetter(field); // cannot fail: access was forced
}
+ return h != null ? h : lookup.unreflectSetter(field);
}
}
(groovy) 01/03: GROOVY-12314: MOP: treat a field whose access cannot be established as absent
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3e11ee970e461d77f5f09c338d9fac2a5230988c
Author: Paul King <pa...@asert.com.au>
AuthorDate: Fri Aug 28 12:32:03 2026 +1000
GROOVY-12314: MOP: treat a field whose access cannot be established as absent
A non-public field of a strongly encapsulated class (e.g. a JDK class
without --add-opens) can be selected as a meta property but only fails
when read or written: the indy property path escalated the refusal to
GroovyBugError and attribute access leaked a raw IllegalAccessException.
Skip such fields during selection so the normal missing-member handling
applies (MissingPropertyException / MissingFieldException, as in 4.x),
and make the indy selectors degrade to the generic MetaProperty or
adapter path instead of GroovyBugError. Forceable access (open modules,
class-path classes, --add-opens) is unaffected.
---
src/main/java/groovy/lang/MetaClassImpl.java | 34 ++++-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 26 ++--
.../codehaus/groovy/reflection/CachedField.java | 15 +-
.../transform/stc/StaticTypeCheckingVisitor.java | 46 +++++-
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 24 +--
.../stc/ParameterizedReceiverTestExtension.groovy | 33 +++++
src/test/groovy/bugs/Groovy12310.groovy | 118 +++++++++++++++
src/test/groovy/bugs/Groovy12314.groovy | 164 +++++++++++++++++++++
src/test/groovy/bugs/Groovy12314Support.java | 31 ++++
src/test/groovy/groovy/MapTest.groovy | 33 +++++
.../stc/FieldsAndPropertiesSTCTest.groovy | 85 +++++++++++
.../packageScope/DifferentPackageTest.groovy | 6 +-
12 files changed, 579 insertions(+), 36 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 170760a531..053358e6a7 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1451,7 +1451,13 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
Object value = null;
if (!(metaProperty instanceof ReadOnlyMetaProperty)) {
- value = metaProperty.getProperty(object);
+ try {
+ value = metaProperty.getProperty(object);
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: a field the reflective path cannot force access to
+ // (strongly encapsulated declaring class) is treated as absent
+ if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
+ }
} else if (object instanceof Map<?, ?> map) {
value = map.get(methodName);
} else if (object instanceof Script script) {
@@ -3071,8 +3077,14 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
&& (!isMap || isStatic // GROOVY-8065
|| field.isPublic())) { // GROOVY-11367
if (!field.isFinal()) {
- field.setProperty(object, newValue);
- return;
+ try {
+ field.setProperty(object, newValue);
+ return;
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: the reflective path cannot force access (strongly
+ // encapsulated declaring class): continue as if the field were absent
+ if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
+ }
} else {
throw new ReadOnlyPropertyException(name, theClass); // GROOVY-5985
}
@@ -3229,6 +3241,12 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
try {
// delegate the get operation to the metaproperty
if (mp != null) return mp.getProperty(object);
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: a field the reflective path cannot force access to
+ // (strongly encapsulated declaring class) reports missing instead
+ if (!(gre.getCause() instanceof IllegalAccessException)) {
+ throw new GroovyRuntimeException("Cannot read field: " + attribute, gre);
+ }
} catch (Exception e) {
throw new GroovyRuntimeException("Cannot read field: " + attribute, e);
}
@@ -3268,8 +3286,14 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
mp = mbp.getField();
}
if (mp != null) {
- mp.setProperty(object, newValue);
- return;
+ try {
+ mp.setProperty(object, newValue);
+ return;
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: a field the reflective path cannot force access to
+ // (strongly encapsulated declaring class) reports missing instead
+ if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
+ }
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 577c09d23f..421ed1f5de 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -61,7 +61,6 @@ import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
import static org.apache.groovy.util.BeanUtils.capitalize;
import static org.codehaus.groovy.ast.ClassHelper.CLASS_Type;
import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.COLLECTION_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.GROOVY_OBJECT_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Iterator_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.LIST_TYPE;
@@ -123,7 +122,6 @@ import static org.objectweb.asm.Opcodes.PUTSTATIC;
public class StaticTypesCallSiteWriter extends CallSiteWriter {
private static final ClassNode INVOKERHELPER_TYPE = ClassHelper.make(InvokerHelper.class);
- private static final MethodNode COLLECTION_SIZE_METHOD = COLLECTION_TYPE.getMethod("size", Parameter.EMPTY_ARRAY);
private static final MethodNode MAP_GET_METHOD = MAP_TYPE.getMethod("get", new Parameter[]{new Parameter(OBJECT_TYPE, "key")});
private static final MethodNode GROOVYOBJECT_GETPROPERTY_METHOD = GROOVY_OBJECT_TYPE.getMethod("getProperty", new Parameter[]{new Parameter(STRING_TYPE, "propertyName")});
private static final MethodNode INVOKERHELPER_GETPROPERTY_METHOD = INVOKERHELPER_TYPE.getMethod("getProperty", new Parameter[]{new Parameter(OBJECT_TYPE, "object"), new Parameter(STRING_TYPE, "propertyName")});
@@ -175,14 +173,10 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
controller.getMethodVisitor().visitInsn(ARRAYLENGTH);
controller.getOperandStack().replace(int_TYPE);
return;
- } else if (isOrImplements(receiverType, COLLECTION_TYPE) && ("size".equals(propertyName) || "length".equals(propertyName))) {
- MethodCallExpression expr = callX(receiver, "size");
- expr.setMethodTarget(COLLECTION_SIZE_METHOD);
- expr.setImplicitThis(implicitThis);
- expr.setSafe(safe);
- expr.visit(controller.getAcg());
- return;
}
+ // GROOVY-12314: no special case for "size"/"length" on Collection receivers: the
+ // type checker no longer admits either name unless a genuine member backs it, and
+ // rewriting an admitted member to size() produced silently wrong values
if (makeGetPropertyWithGetter(receiver, receiverType, propertyName, safe, implicitThis)) return;
@@ -883,8 +877,16 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
}*/
private void addPropertyAccessError(final Expression receiver, final String propertyName, final ClassNode receiverType) {
- String receiverName = (receiver instanceof ClassExpression ? receiver.getType() : receiverType).toString(false);
- String message = "Access to " + receiverName + "#" + propertyName + " is forbidden";
- controller.getSourceUnit().addError(new SyntaxException(message, receiver));
+ ClassNode receiverNode = (receiver instanceof ClassExpression ? receiver.getType() : receiverType);
+ // GROOVY-12314: report a placeholder's erasure, not "E"; an unconditional redirect would
+ // instead degrade a parameterized receiver to its declaration's type parameters ("List<E>")
+ if (receiverNode.isGenericsPlaceHolder()) receiverNode = receiverNode.redirect();
+ String message = "Access to " + receiverNode.toString(false) + "#" + propertyName + " is forbidden";
+ int line = receiver.getLineNumber(), column = receiver.getColumnNumber();
+ if (line < 1) { // GROOVY-12314: synthetic receiver: fall back to the current statement position
+ line = controller.getLineNumber();
+ column = 1;
+ }
+ controller.getSourceUnit().addError(new SyntaxException(message, line, column));
}
}
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedField.java b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
index 77206de2de..97ef6bba43 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedField.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
@@ -46,10 +46,19 @@ public class CachedField extends MetaProperty {
}
private final Field field;
- private boolean madeAccessible;
+ private volatile boolean madeAccessible;
+ private boolean accessAttempted; // guarded by synchronization on this
private void makeAccessible() {
- ReflectionUtils.makeAccessibleInPrivilegedAction(field);
- madeAccessible = true;
+ // at most one attempt, remembering either outcome: a failed attempt (strongly
+ // encapsulated declaring class) cannot succeed later. The attempt is recorded
+ // only once it has completed, under synchronization, so a concurrent caller
+ // waits instead of reading the field before setAccessible has taken effect.
+ synchronized (this) {
+ if (!accessAttempted) {
+ madeAccessible = ReflectionUtils.makeAccessibleInPrivilegedAction(field).isPresent();
+ accessAttempted = true;
+ }
+ }
}
/**
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index f2442ad3e9..bd4de48501 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -892,6 +892,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private void recordMissingProperty(final PropertyExpression expression) {
+ FieldNode inaccessibleField = expression.getNodeMetaData("inaccessible field");
+ if (inaccessibleField != null) { // GROOVY-12314: the property exists but Java access rules reject it
+ addStaticTypeError("Cannot access field: " + inaccessibleField.getName() + " of class: " + prettyPrintTypeName(inaccessibleField.getDeclaringClass())
+ + " from class: " + prettyPrintTypeName(typeCheckingContext.getEnclosingClassNode()), expression.getLineNumber() > 0 ? expression : expression.getProperty());
+ return;
+ }
+
var objectExpression = expression.getObjectExpression();
var objectExpressionType = findCurrentInstanceOfClass(objectExpression, getType(objectExpression));
@@ -2482,6 +2489,21 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
return false;
}
+ /**
+ * Determines if the given type itself may be referenced from the accessor
+ * (JLS 6.6.1): a member of an inaccessible type is out of reach no matter
+ * what the member's own modifiers say, because resolving any reference to
+ * the type fails first. A nested type is accessible when its enclosing
+ * type is and its own modifiers admit the accessor.
+ */
+ private static boolean hasAccessToClass(final ClassNode accessor, final ClassNode type) {
+ ClassNode outer = type.getOuterClass();
+ if (outer != null && !hasAccessToClass(accessor, outer)) {
+ return false;
+ }
+ return hasAccessToMember(accessor, outer != null ? outer : type, type.getModifiers());
+ }
+
private ClassNode getTypeForMultiValueExpression(final ClassNode compositeType, final Expression prop) {
GenericsType[] gts = compositeType.getGenericsTypes();
ClassNode itemType = (gts != null && gts.length == 1 ? getCombinedBoundType(gts[0]) : OBJECT_TYPE);
@@ -2594,19 +2616,27 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
* Determines if the field can back the given property (or attribute)
* expression: accessible by Java rules, or admitted by the receiver-type
* leniency (GROOVY-7300, GROOVY-11358) that models dynamic Groovy's
- * permissive private access. Since GROOVY-12290 that leniency no longer
- * admits plain property syntax to a private field of a foreign nest — a
+ * permissive private access. Since GROOVY-12290 (private) and GROOVY-12314
+ * (package-private, protected) that leniency no longer admits plain property
+ * syntax to a field of a foreign nest that Java rules would reject — a
* direct field access that static compilation cannot honour (no access
* bridge exists). The deliberate dynamic escape hatches remain: attribute
* access (.@), and closure bodies (GROOVY-9195) — including delegate-resolved
* access — whose property dispatch stays dynamic-capable under static compilation.
*/
private boolean isFieldAccessible(final FieldNode field, final ClassNode receiver, final PropertyExpression expression, final String delegationData) {
+ // GROOVY-12310: when the qualifying type itself is out of reach (e.g. a public
+ // enum constant of a package-private nested enum), no static emission can name
+ // it -- the JVM rejects the class resolution before it looks at the member
+ if (!hasAccessToClass(typeCheckingContext.getEnclosingClassNode(), receiver)) {
+ return false;
+ }
boolean superField = isSuperExpression(expression.getObjectExpression());
boolean exactReceiver = (!superField && receiver.equals(field.getDeclaringClass()) && !field.getDeclaringClass().isAbstract()); // GROOVY-7300, GROOVY-11358
- if (exactReceiver && field.isPrivate() && delegationData == null
+ if (exactReceiver && delegationData == null
&& typeCheckingContext.getEnclosingClosure() == null
&& !(expression instanceof AttributeExpression)
+ && !hasAccessToMember(typeCheckingContext.getEnclosingClassNode(), field.getDeclaringClass(), field.getModifiers()) // GROOVY-12314
&& !inSameNest(field.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode())) {
exactReceiver = false; // GROOVY-12290
}
@@ -2619,7 +2649,15 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
if (!accessible) {
if (expressionToStoreOn instanceof AttributeExpression) {
addStaticTypeError("Cannot access field: " + field.getName() + " of class: " + prettyPrintTypeName(field.getDeclaringClass()), expressionToStoreOn.getProperty());
- } else if (!field.isProtected()) { // private or package-private
+ } else { // GROOVY-12314: fall through to accessor, extension or map/list resolution
+ // remember a rejected package-private or protected field declared by the
+ // receiver's own class: if nothing else resolves the property, the error
+ // reports the access violation rather than a missing property. A private
+ // field stays hidden (GROOVY-12290), and an inaccessible inherited field is
+ // not a member of the subclass (JLS 8.2; GROOVY-9093, GROOVY-9293).
+ if (!field.isPrivate() && receiver.equals(field.getDeclaringClass())) {
+ expressionToStoreOn.putNodeMetaData("inaccessible field", field);
+ }
return false;
}
}
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
index 47f8e426c4..198a9a73e3 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -438,12 +438,16 @@ public abstract class Selector {
insertName = true; // pass "name" field as argument
} else if (mp instanceof CachedField && !mp.isStatic()) {
try {
- // GROOVY-9144, GROOVY-9596: get lookup for sender and unreflect before forcing access
- @SuppressWarnings("removal")
- MethodHandles.Lookup lookup = ((Java8) VMPluginFactory.getPlugin()).newLookup(sender);
- handle = ((CachedField) mp).asAccessMethod(lookup);
+ // GROOVY-9144, GROOVY-9596: unreflect against the call-site lookup before
+ // forcing access; the lookup carries the caller's own access rights, exactly
+ // as the bytecode a Java compiler would emit, so it may still reach e.g. an
+ // inherited protected field (FilterReader#in from a subclass)
+ handle = ((CachedField) mp).asAccessMethod(callSite.getLookup());
} catch (IllegalAccessException e) {
- throw new GroovyBugError(e);
+ // GROOVY-12314: refusal is an access-control outcome, not an internal
+ // error; leave the handle unset so the full MOP path applies -- it
+ // retries the reflective route and converts a reflective failure
+ // into the normal missing-member handling
}
} else {
handle = META_PROPERTY_GETTER.bindTo(mp);
@@ -604,13 +608,13 @@ public abstract class Selector {
: TypeHelper.getWrapperClass(fieldType).isInstance(value);
if (!accepts) return; // needs coercion: adapter path
try {
- // like the property-get field path: lookup for the sender, then unreflect
- @SuppressWarnings("removal")
- MethodHandles.Lookup lookup = ((Java8) VMPluginFactory.getPlugin()).newLookup(sender);
- handle = field.asWriteAccessMethod(lookup);
+ // like the property-get field path: unreflect against the call-site lookup
+ handle = field.asWriteAccessMethod(callSite.getLookup());
if (LOG_ENABLED) LOG.info("direct field write handle set for property write");
} catch (IllegalAccessException e) {
- throw new GroovyBugError(e);
+ // GROOVY-12314: refusal is an access-control outcome, not an internal
+ // error; leave handle unset so the sender-aware adapter path applies
+ if (LOG_ENABLED) LOG.info("direct field write refused, using adapter path");
}
}
diff --git a/src/test-resources/groovy/transform/stc/ParameterizedReceiverTestExtension.groovy b/src/test-resources/groovy/transform/stc/ParameterizedReceiverTestExtension.groovy
new file mode 100644
index 0000000000..25d98de1d5
--- /dev/null
+++ b/src/test-resources/groovy/transform/stc/ParameterizedReceiverTestExtension.groovy
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import org.codehaus.groovy.ast.GenericsType
+
+import static org.codehaus.groovy.ast.tools.GenericsUtils.makeClassSafeWithGenerics
+
+// resolves every dynamic variable dynamically with type Optional<String> and
+// silences all property missing errors
+unresolvedVariable { var ->
+ if (isDynamic(var)) {
+ makeDynamic(var, makeClassSafeWithGenerics(make(Optional), new GenericsType(STRING_TYPE)))
+ handled = true
+ }
+}
+unresolvedProperty { pexp ->
+ handled = true
+}
diff --git a/src/test/groovy/bugs/Groovy12310.groovy b/src/test/groovy/bugs/Groovy12310.groovy
new file mode 100644
index 0000000000..659ba56f25
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12310.groovy
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package bugs
+
+import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.shouldFail
+
+/**
+ * GROOVY-12310: a member of an inaccessible declaring type -- here a public
+ * enum constant of a package-private nested enum, the {@code jakarta.faces}
+ * {@code UIInput.PropertyKeys} shape -- must be rejected by the type checker
+ * instead of compiling to a direct reference that fails at runtime with
+ * {@code IllegalAccessError}.
+ */
+final class Groovy12310 {
+
+ private GroovyShell shell
+
+ @BeforeEach
+ void setUp() {
+ def loader = new GroovyClassLoader()
+ loader.parseClass '''
+ package p
+ class Outer {
+ @groovy.transform.PackageScope
+ enum PropertyKeys { localValueSet, other }
+ }
+ '''
+ shell = new GroovyShell(loader)
+ }
+
+ @Test
+ void testDynamicAccessUnaffected() {
+ assert shell.evaluate('p.Outer.PropertyKeys.localValueSet').name() == 'localValueSet'
+ assert shell.evaluate('p.Outer.PropertyKeys.@localValueSet').name() == 'localValueSet'
+ assert shell.evaluate("p.Outer.PropertyKeys['localValueSet']").name() == 'localValueSet'
+ }
+
+ @Test
+ void testStaticClassReferenceAndSubscriptUnaffected() {
+ assert shell.evaluate('''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys }
+ f()
+ ''').simpleName == 'PropertyKeys'
+ assert shell.evaluate('''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys['localValueSet'] }
+ f()
+ ''').name() == 'localValueSet'
+ }
+
+ @Test
+ void testStaticPropertyAccessRejected() {
+ // was: compiled to a direct field reference, then at runtime
+ // "IllegalAccessError: failed to access class p.Outer$PropertyKeys"
+ def err = shouldFail MultipleCompilationErrorsException, {
+ shell.evaluate '''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys.localValueSet }
+ '''
+ }
+ assert err.message.contains('Cannot access field: localValueSet of class: p.Outer$PropertyKeys')
+ }
+
+ @Test
+ void testStaticAttributeAccessRejected() {
+ def err = shouldFail MultipleCompilationErrorsException, {
+ shell.evaluate '''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys.@localValueSet }
+ '''
+ }
+ assert err.message.contains('Cannot access field: localValueSet of class: p.Outer$PropertyKeys')
+ }
+
+ @Test
+ void testMakeDynamicExtensionEscapeHatch() {
+ // an extension may resolve the reference dynamically; the accessibility
+ // rejection must leave that route open (the identifier stays in source,
+ // the emission goes through the dynamic runtime)
+ def extDir = File.createTempDir()
+ new File(extDir, 'Groovy12310Extension.groovy').text = '''
+ unresolvedProperty { pexp ->
+ if (pexp.propertyAsString == 'localValueSet') {
+ makeDynamic(pexp)
+ handled = true
+ }
+ }
+ '''
+ shell.classLoader.addClasspath(extDir.absolutePath)
+ def result = shell.evaluate '''
+ @groovy.transform.CompileStatic(extensions='Groovy12310Extension.groovy')
+ def f() { p.Outer.PropertyKeys.localValueSet }
+ f()
+ '''
+ assert result.name() == 'localValueSet'
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy12314.groovy b/src/test/groovy/bugs/Groovy12314.groovy
new file mode 100644
index 0000000000..7ee5799dc9
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12314.groovy
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package bugs
+
+import groovy.test.GroovyAssert
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+import static org.junit.jupiter.api.Assumptions.assumeFalse
+
+/**
+ * GROOVY-12314: property/attribute access to a field whose reflective access
+ * cannot be established (e.g. a non-public field of a strongly encapsulated
+ * JDK class, absent {@code --add-opens}) must degrade to the normal missing
+ * member handling instead of GroovyBugError or a raw IllegalAccessException.
+ * The JDK field tests assume the JVM does not open {@code java.base/java.util}
+ * and skip themselves when it does.
+ */
+final class Groovy12314 {
+
+ private static void assumeJavaUtilIsEncapsulated() {
+ assumeFalse(ArrayList.class.getModule().isOpen('java.util', Groovy12314.class.getModule()),
+ 'the JVM opens java.base/java.util, so JDK fields are reflectively reachable')
+ }
+
+ @Test
+ void testProtectedJdkFieldPropertyRead() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ // was: GroovyBugError "BUG! UNCAUGHT EXCEPTION: member is protected"
+ shouldFail(MissingPropertyException) {
+ list.modCount
+ }
+ }
+
+ @Test
+ void testPackagePrivateJdkFieldPropertyRead() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ shouldFail(MissingPropertyException) {
+ list.elementData
+ }
+ }
+
+ @Test
+ void testProtectedJdkFieldAttributeRead() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ // was: raw IllegalAccessException escaping from CachedField#getProperty
+ shouldFail(MissingFieldException) {
+ list.@modCount
+ }
+ }
+
+ @Test
+ void testProtectedJdkFieldPropertyWrite() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ // the field exists but cannot be written via property syntax
+ shouldFail(ReadOnlyPropertyException) {
+ list.modCount = 7
+ }
+ }
+
+ @Test
+ void testProtectedJdkFieldAttributeWrite() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ shouldFail(MissingFieldException) {
+ list.@modCount = 7
+ }
+ }
+
+ @Test
+ void testPermissiveAccessToOpenClassesUnaffected() {
+ // classes on the class path live in an unnamed (open) module: dynamic
+ // Groovy's permissive field access there must keep working
+ assertScript '''
+ class Holder {
+ private int secret = 42
+ protected int shielded = 43
+ }
+ def h = new Holder()
+ assert h.secret == 42
+ assert h.@secret == 42
+ assert h.shielded == 43
+ assert h.@shielded == 43
+ '''
+ }
+
+ @Test
+ void testPermissiveWriteToOpenClassesUnaffected() {
+ // a non-public field is never given a direct write handle (only public
+ // fields are sender-independent), so the write must actually succeed
+ // through the sender-aware adapter path
+ assertScript '''
+ class Holder {
+ private int secret = 42
+ protected int shielded = 43
+ }
+ def h = new Holder()
+ h.secret = 52
+ assert h.@secret == 52
+ h.shielded = 53
+ assert h.@shielded == 53
+ '''
+ }
+
+ @Test
+ void testPublicFieldOfPackagePrivateClass() {
+ // the call-site lookup cannot see the declaring class, so unreflecting the
+ // public field is refused at first; forcing accessibility and retrying
+ // (GROOVY-9144, GROOVY-9596) must still yield working read/write handles
+ def loader = new GroovyClassLoader()
+ loader.parseClass '''
+ package p
+ @groovy.transform.PackageScope
+ class Holder12314 {
+ public int value = 42
+ }
+ class Factory12314 {
+ static Object make() { new Holder12314() }
+ }
+ '''
+ new GroovyShell(loader).evaluate '''
+ def obj = p.Factory12314.make()
+ assert obj.value == 42
+ obj.value = 52
+ assert obj.value == 52
+ '''
+ }
+
+ @Test
+ void testForbiddenAccessErrorReportsTypeArguments() {
+ // the classgen safety-net error must name the receiver's type arguments
+ // (Optional<String>), not the declaration's type parameters (Optional<T>)
+ def err = shouldFail '''
+ @groovy.transform.CompileStatic(extensions='groovy/transform/stc/ParameterizedReceiverTestExtension.groovy')
+ class C {
+ def foo() {
+ bar.baz
+ }
+ }
+ '''
+ assert err.message =~ /Access to java\.util\.Optional<java\.lang\.String>#baz is forbidden/
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy12314Support.java b/src/test/groovy/bugs/Groovy12314Support.java
new file mode 100644
index 0000000000..da2414717e
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12314Support.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package bugs;
+
+import java.util.ArrayList;
+
+/**
+ * GROOVY-12314: a Java collection type with public fields named for the
+ * {@code size}/{@code length} property conventions; property access must
+ * resolve to the fields, not be rewritten to {@code Collection#size()}.
+ */
+public class Groovy12314Support extends ArrayList<Object> {
+ public int size = 42;
+ public int length = 99;
+}
diff --git a/src/test/groovy/groovy/MapTest.groovy b/src/test/groovy/groovy/MapTest.groovy
index 3729ec739d..783e161102 100644
--- a/src/test/groovy/groovy/MapTest.groovy
+++ b/src/test/groovy/groovy/MapTest.groovy
@@ -308,6 +308,39 @@ import static groovy.test.GroovyAssert.shouldFail
"""
}
+ // GROOVY-5001, GROOVY-5491, GROOVY-11367, GROOVY-12314
+ @ParameterizedTest @ValueSource(strings=['','static'])
+ void testMapSubclassPropertyAccess(String mode) {
+ assertScript """import groovy.transform.*
+ class M extends HashMap { // just like GROOVY-662, GROOVY-8065, GROOVY-8074
+ def $mode v = 'v'
+ public $mode w = 'w'
+ protected $mode x = 'x'
+ @PackageScope $mode y = 'y'
+ private $mode z = 'z'
+ }
+
+ def map = new M()
+ assert map.v == 'v' // property accessor before map entry
+ assert map.w == 'w' // public field before map entry
+ assert map.x == null // non-public field: map entry, even though absent
+ assert map.y == null // and even though the caller is in the same package
+ assert map.z == null
+
+ ['v','w','x','y','z'].each { map.put(it, 'entry') }
+
+ assert map.v == 'v'
+ assert map.w == 'w'
+ assert map.x == 'entry'
+ assert map.y == 'entry'
+ assert map.z == 'entry'
+
+ assert map.@x == 'x' // attribute access still reaches the field
+ assert map.@y == 'y'
+ assert map.@z == 'z'
+ """
+ }
+
@Test
void testMapLeftShift() {
def map = [a:1, b:2]
diff --git a/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 555bab9206..4e1ab160f3 100644
--- a/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -1992,6 +1992,91 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-12314: package-private and protected fields that Java access rules
+ // reject are not admitted via property syntax either; resolution falls
+ // through to accessors, extensions and map/list handling. When nothing else
+ // resolves the property, a field declared by the receiver's own class is
+ // reported as an access violation (as javac does), while an inaccessible
+ // inherited field is not a member of the subclass (JLS 8.2) and stays missing
+ @Test
+ void testNonPublicForeignFieldViaPropertySyntax() {
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.modCount } // protected, inherited from java.util.AbstractList
+ ''',
+ 'No such property: modCount for class: java.util.ArrayList'
+
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.elementData } // package-private, declared by java.util.ArrayList
+ ''',
+ 'Cannot access field: elementData of class: java.util.ArrayList from class: '
+
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.modCount = 1 }
+ ''',
+ 'No such property: modCount for class: java.util.ArrayList'
+
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.elementData = null }
+ ''',
+ 'Cannot access field: elementData of class: java.util.ArrayList from class: '
+ }
+
+ // GROOVY-12314: "size" and "length" are methods, not properties, of collections
+ @Test
+ void testCollectionSizeIsNotAProperty() {
+ shouldFailWithMessages '''
+ List list = [1, 2]
+ list.size
+ ''',
+ 'No such property: size for class: java.util.ArrayList'
+
+ shouldFailWithMessages '''
+ List list = [1, 2]
+ list.length
+ ''',
+ 'No such property: length for class: java.util.ArrayList'
+ }
+
+ // GROOVY-12314: a public field named "size" or "length" on a Java collection
+ // type is honoured; it is not rewritten to Collection#size()
+ @Test
+ void testPublicSizeOrLengthFieldOfCollectionType() {
+ assertScript '''
+ def j = new bugs.Groovy12314Support()
+ j.add(1)
+ assert j.size == 42
+ assert j.length == 99
+ '''
+ assertScript '''
+ class SizedGroovy extends ArrayList<Object> {
+ public int size = 42
+ }
+ def g = new SizedGroovy()
+ g.add(1)
+ assert g.size == 42
+ '''
+ }
+
+ // GROOVY-12314: Java access rules still admit these
+ @Test
+ void testAccessibleNonPublicFieldViaPropertySyntax() {
+ assertScript '''
+ class A { protected int x = 1 }
+ class B extends A { def m() { x + this.x } }
+ assert new B().m() == 2
+ '''
+ assertScript '''
+ class A { protected int x = 1 }
+ class B { def m(A a) { a.x } } // same package
+ assert new B().m(new A()) == 1
+ '''
+ assertScript '''
+ class A { @groovy.transform.PackageScope int x = 1 }
+ class B { def m(A a) { a.x } } // same package
+ assert new B().m(new A()) == 1
+ '''
+ }
+
// GROOVY-5737
@Test
void testGeneratedFieldAccessInClosure() {
diff --git a/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
index c03cc964a4..28dc21f0a3 100644
--- a/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
@@ -367,7 +367,8 @@ final class DifferentPackageTest {
'''
)
}
- assert err.message =~ /Access to p.One#value is forbidden/
+ // GROOVY-12314: rejected by the type checker as an access violation, no longer during class generation
+ assert err.message =~ /Cannot access field: value of class: p.One from class: q.Two/
}
// GROOVY-9093
@@ -431,7 +432,8 @@ final class DifferentPackageTest {
'''
)
}
- assert err.message =~ /Access to p.One#CONST is forbidden/
+ // GROOVY-12314: rejected by the type checker as an access violation, no longer during class generation
+ assert err.message =~ /Cannot access field: CONST of class: p.One from class: q.Other/
}
@Test
(groovy) 03/03: GROOVY-12314: MetaClassImpl: replace reflective-failure catches with a use-site pre-check
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3c3a99f0ce939b74f3df9435390a42848359a3df
Author: Paul King <pa...@asert.com.au>
AuthorDate: Wed Sep 2 10:24:59 2026 +1000
GROOVY-12314: MetaClassImpl: replace reflective-failure catches with a use-site pre-check
Review follow-up. With the deep-reflection attempt latched and its outcome
queryable, the classic paths can ask whether reflection can reach a field
before touching it, so the four sites that caught GroovyRuntimeException
and inspected its cause for IllegalAccessException become plain control
flow: CachedField#canAccessViaReflection answers for the reflective path
only, and an unreachable field is treated as absent where it would have
been read or written. Member selection (getEffectiveGetMetaProperty) and
indy linkage stay unfiltered -- the check is deliberately not part of
selection, so a call-site lookup with its own access rights can still
reach such a field.
---
src/main/java/groovy/lang/MetaClassImpl.java | 51 ++++++++++------------
.../codehaus/groovy/reflection/CachedField.java | 15 +++++++
2 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 053358e6a7..7e48ffeca4 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1443,6 +1443,18 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
};
}
+ /**
+ * Determines whether the reflective access path can actually reach the given
+ * property, treating a field of a strongly encapsulated declaring class as
+ * absent at the point of use so the normal missing-member handling applies.
+ * This is a use-site decision only: member <em>selection</em> (see
+ * {@code getEffectiveGetMetaProperty}) and indy linkage stay unfiltered, so a
+ * call-site {@code Lookup} with its own rights can still reach such a field.
+ */
+ private static boolean isReflectivelyAccessible(final MetaProperty mp) {
+ return !(mp instanceof CachedField cf) || cf.canAccessViaReflection();
+ }
+
/**
* Tries to find a callable property and make the call.
*/
@@ -1451,12 +1463,8 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
Object value = null;
if (!(metaProperty instanceof ReadOnlyMetaProperty)) {
- try {
+ if (isReflectivelyAccessible(metaProperty)) { // GROOVY-12314: unreachable field is absent
value = metaProperty.getProperty(object);
- } catch (GroovyRuntimeException gre) {
- // GROOVY-12314: a field the reflective path cannot force access to
- // (strongly encapsulated declaring class) is treated as absent
- if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
}
} else if (object instanceof Map<?, ?> map) {
value = map.get(methodName);
@@ -3075,16 +3083,11 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
//----------------------------------------------------------------------
if (field != null && (method == null || isVisibleProperty(field, method, sender))
&& (!isMap || isStatic // GROOVY-8065
- || field.isPublic())) { // GROOVY-11367
+ || field.isPublic()) // GROOVY-11367
+ && isReflectivelyAccessible(field)) { // GROOVY-12314: unreachable field is absent
if (!field.isFinal()) {
- try {
- field.setProperty(object, newValue);
- return;
- } catch (GroovyRuntimeException gre) {
- // GROOVY-12314: the reflective path cannot force access (strongly
- // encapsulated declaring class): continue as if the field were absent
- if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
- }
+ field.setProperty(object, newValue);
+ return;
} else {
throw new ReadOnlyPropertyException(name, theClass); // GROOVY-5985
}
@@ -3240,12 +3243,8 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
try {
// delegate the get operation to the metaproperty
- if (mp != null) return mp.getProperty(object);
- } catch (GroovyRuntimeException gre) {
- // GROOVY-12314: a field the reflective path cannot force access to
- // (strongly encapsulated declaring class) reports missing instead
- if (!(gre.getCause() instanceof IllegalAccessException)) {
- throw new GroovyRuntimeException("Cannot read field: " + attribute, gre);
+ if (mp != null && isReflectivelyAccessible(mp)) { // GROOVY-12314: unreachable field reports missing
+ return mp.getProperty(object);
}
} catch (Exception e) {
throw new GroovyRuntimeException("Cannot read field: " + attribute, e);
@@ -3285,15 +3284,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
if (mp instanceof MetaBeanProperty mbp) {
mp = mbp.getField();
}
- if (mp != null) {
- try {
- mp.setProperty(object, newValue);
- return;
- } catch (GroovyRuntimeException gre) {
- // GROOVY-12314: a field the reflective path cannot force access to
- // (strongly encapsulated declaring class) reports missing instead
- if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
- }
+ if (mp != null && isReflectivelyAccessible(mp)) { // GROOVY-12314: unreachable field reports missing
+ mp.setProperty(object, newValue);
+ return;
}
}
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedField.java b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
index 5889584914..67dfb64aba 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedField.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
@@ -69,6 +69,21 @@ public class CachedField extends MetaProperty {
return madeAccessible;
}
+ /**
+ * Determines whether the reflective access path ({@code Field.get}/{@code set})
+ * can reach this field: deep access has been, or can now be, established. The
+ * reflective path's callers use this to treat an unreachable field (strongly
+ * encapsulated declaring class) as absent instead of failing when it is read
+ * or written. It says nothing about what a caller's own {@code Lookup} may
+ * reach -- see {@link #asAccessMethod(MethodHandles.Lookup)} -- and so must
+ * not be used to decide member selection or call-site linkage.
+ *
+ * @since 6.0.0
+ */
+ public boolean canAccessViaReflection() {
+ return makeAccessible();
+ }
+
/**
* Returns the underlying Java {@code Field} object, making it accessible if necessary.
*