You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2023/07/25 22:53:17 UTC
[groovy] branch master updated: GROOVY-10438, GROOVY-10555: accessible properties
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new dce378e1e2 GROOVY-10438, GROOVY-10555: accessible properties
dce378e1e2 is described below
commit dce378e1e242f551eb697a30f6aca5a9f6188e26
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Tue Jul 25 17:29:51 2023 -0500
GROOVY-10438, GROOVY-10555: accessible properties
---
src/main/java/groovy/lang/MetaClassImpl.java | 40 +++++++++++---------------
src/test/groovy/IllegalAccessTests.groovy | 42 ++++++++++++++++++++++------
2 files changed, 50 insertions(+), 32 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 49d005253e..f38adbd899 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -117,6 +117,7 @@ import static java.lang.Character.isUpperCase;
import static org.apache.groovy.util.Arrays.concat;
import static org.codehaus.groovy.ast.tools.GeneralUtils.inSamePackage;
import static org.codehaus.groovy.reflection.ReflectionCache.isAssignableFrom;
+import static org.codehaus.groovy.reflection.ReflectionUtils.checkAccessible;
/**
* Allows methods to be dynamically added to existing classes at runtime
@@ -2160,9 +2161,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
/**
- * Get all the properties defined for this type
+ * Returns the available properties for this type.
*
- * @return a list of MetaProperty objects
+ * @return a list of {@code MetaProperty} objects
*/
@Override
public List<MetaProperty> getProperties() {
@@ -2175,10 +2176,14 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
// simply return the values of the metaproperty map as a List
List<MetaProperty> ret = new ArrayList<>(propertyMap.size());
for (MetaProperty mp : propertyMap.values()) {
- if (mp instanceof CachedField && (mp.getModifiers() & Opcodes.ACC_SYNTHETIC) != 0) {
- continue;
- }
- if (mp instanceof MetaBeanProperty) {
+ if (mp instanceof CachedField) {
+ int modifiers = mp.getModifiers();
+ if ((modifiers & Opcodes.ACC_SYNTHETIC) != 0
+ // GROOVY-5169, GROOVY-9081, GROOVY-9103, GROOVY-10438, GROOVY-10555, et al.
+ || (!permissivePropertyAccess && !checkAccessible(getClass(), ((CachedField) mp).getDeclaringClass(), modifiers, false))) {
+ continue;
+ }
+ } else if (mp instanceof MetaBeanProperty) {
MetaBeanProperty mbp = (MetaBeanProperty) mp;
// filter out extrinsic properties (DGM, ...)
boolean getter = true, setter = true;
@@ -2190,17 +2195,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
if (setterMetaMethod == null || setterMetaMethod instanceof GeneratedMetaMethod || setterMetaMethod instanceof NewInstanceMetaMethod) {
setter = false;
}
- if (!setter && !getter) continue;
-
-// TODO: I (ait) don't know why these strange tricks needed and comment following as it effects some Grails tests
-// if (!setter && mbp.getSetter() != null) {
-// mp = new MetaBeanProperty(mbp.getName(), mbp.getType(), mbp.getGetter(), null);
-// }
-// if (!getter && mbp.getGetter() != null) {
-// mp = new MetaBeanProperty(mbp.getName(), mbp.getType(), null, mbp.getSetter());
-// }
+ if (!getter && !setter) continue;
- if (!permissivePropertyAccess) {
+ if (!permissivePropertyAccess) { // GROOVY-5169, GROOVY-9081, GROOVY-9103
boolean getterAccessible = canAccessLegally(getterMetaMethod);
boolean setterAccessible = canAccessLegally(setterMetaMethod);
if (!(getterAccessible && setterAccessible)) continue;
@@ -2212,14 +2209,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
return ret;
}
- private static boolean canAccessLegally(MetaMethod accessor) {
- boolean accessible = true;
- if (accessor instanceof CachedMethod) {
- CachedMethod cm = (CachedMethod) accessor;
- accessible = cm.canAccessLegally(MetaClassImpl.class);
- }
-
- return accessible;
+ private static boolean canAccessLegally(final MetaMethod method) {
+ return !(method instanceof CachedMethod)
+ || ((CachedMethod) method).canAccessLegally(MetaClassImpl.class);
}
/**
diff --git a/src/test/groovy/IllegalAccessTests.groovy b/src/test/groovy/IllegalAccessTests.groovy
index 0a617ee4e0..66ccae3460 100644
--- a/src/test/groovy/IllegalAccessTests.groovy
+++ b/src/test/groovy/IllegalAccessTests.groovy
@@ -18,6 +18,7 @@
*/
package groovy
+import groovy.transform.PackageScope
import org.junit.Test
import java.awt.Font
@@ -37,6 +38,18 @@ import static groovy.test.GroovyAssert.shouldFail
*/
final class IllegalAccessTests {
+ static class MultipleProperties {
+ def a
+ public b
+ private c
+ protected d
+ @PackageScope e
+ public getF() {}
+ private getG() {}
+ protected getH() {}
+ @PackageScope getI() {}
+ }
+
static class ProtectedConstructor {
protected ProtectedConstructor() {}
void run() {}
@@ -75,7 +88,7 @@ final class IllegalAccessTests {
@Test
void testClone3() {
Object obj = new Tuple1('abc')
- assert obj.clone().getClass() === Tuple1.class
+ assert obj.clone().getClass() === Tuple1
}
@Test
@@ -159,8 +172,8 @@ final class IllegalAccessTests {
@Test
void testAsType2() {
- assertScript """import ${this.class.name}.ProtectedConstructor
- [run: {}] as ProtectedConstructor
+ assertScript """
+ [run: {}] as ${ProtectedConstructor.name.replace('$','.')}
"""
}
@@ -172,10 +185,23 @@ final class IllegalAccessTests {
}
}
+ // GROOVY-9081
@Test
void testGetProperties() {
- String str = ''
- assert str.properties
+ def kv = "".properties
+ assert 'value' !in kv.keySet()
+ assert kv.keySet().containsAll(['blank','bytes','class','empty'])
+ }
+
+ // GROOVY-10438, GROOVY-10555
+ @Test
+ void testGetProperties2() {
+ assertScript """
+ Object obj = new ${MultipleProperties.name.replace('$','.')}()
+ assert obj.properties.keySet() == ['a','b','c','class','d','e','f','g','h','i'] as Set
+ // TODO work out why all properties are returned and then test with:
+ obj.metaClass.permissivePropertyAccess = true
+ """
}
@Test
@@ -268,14 +294,14 @@ final class IllegalAccessTests {
@Test
void testFavorMethodWithExactParameterType() {
- def em1 = new EnumMap(RetentionPolicy.class)
- def em2 = new EnumMap(RetentionPolicy.class)
+ def em1 = new EnumMap(RetentionPolicy)
+ def em2 = new EnumMap(RetentionPolicy)
assert em1 == em2
}
@Test
void testShouldChoosePublicGetterInsteadOfPrivateField1() {
- def f = Integer.class.getDeclaredField('MIN_VALUE')
+ def f = Integer.getDeclaredField('MIN_VALUE')
assert f.modifiers != 0
}