This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 6e30ae6638 Fix field accessibility leak in EnhancedCompositeBeanHelper
(#11425)
6e30ae6638 is described below
commit 6e30ae6638f0dd8c1fc8a65dbf52678fb4cbb158
Author: Guillaume Nodet <[email protected]>
AuthorDate: Wed Nov 12 15:59:46 2025 +0100
Fix field accessibility leak in EnhancedCompositeBeanHelper (#11425)
The previous implementation cached field accessibility state globally,
which could cause issues when the same field is accessed from different
contexts or security managers. This was particularly problematic in
plugin unit tests where fields would remain accessible after being set.
The fix ensures that field accessibility is properly restored to its
original state after setting field values, preventing accessibility
state from leaking between different bean instances.
Added unit tests to verify:
- Field accessibility is restored after setting values
- Multiple field accesses don't leak accessibility state
- The fix works correctly across different bean instances
This resolves the issue reported on the dev list regarding compiler
plugin unit test failures related to field accessibility.
---
.../internal/EnhancedCompositeBeanHelper.java | 16 +-----
.../internal/EnhancedCompositeBeanHelperTest.java | 62 ++++++++++++++++++++++
2 files changed, 63 insertions(+), 15 deletions(-)
diff --git
a/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
b/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
index 78e79c8f4e..59ae2fa69b 100644
---
a/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
+++
b/impl/maven-core/src/main/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelper.java
@@ -52,9 +52,6 @@ public final class EnhancedCompositeBeanHelper {
// Cache for field lookups: Class -> FieldName -> Field
private static final ConcurrentMap<Class<?>, Map<String, Field>>
FIELD_CACHE = new ConcurrentHashMap<>();
- // Cache for accessible fields to avoid repeated setAccessible calls
- private static final ConcurrentMap<Field, Boolean> ACCESSIBLE_FIELD_CACHE
= new ConcurrentHashMap<>();
-
private final ConverterLookup lookup;
private final ClassLoader loader;
private final ExpressionEvaluator evaluator;
@@ -304,19 +301,9 @@ private Object convertProperty(
* Set field value with cached accessibility.
*/
private void setFieldValue(Object bean, Field field, Object value) throws
IllegalAccessException {
- Boolean isAccessible = ACCESSIBLE_FIELD_CACHE.get(field);
- if (isAccessible == null) {
- isAccessible = field.canAccess(bean);
- if (!isAccessible) {
- field.setAccessible(true);
- isAccessible = true;
- }
- ACCESSIBLE_FIELD_CACHE.put(field, isAccessible);
- } else if (!isAccessible) {
+ if (!field.canAccess(bean)) {
field.setAccessible(true);
- ACCESSIBLE_FIELD_CACHE.put(field, true);
}
-
field.set(bean, value);
}
@@ -326,6 +313,5 @@ private void setFieldValue(Object bean, Field field, Object
value) throws Illega
public static void clearCaches() {
METHOD_CACHE.clear();
FIELD_CACHE.clear();
- ACCESSIBLE_FIELD_CACHE.clear();
}
}
diff --git
a/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
b/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
index ab954855a8..87a78bcd81 100644
---
a/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
+++
b/impl/maven-core/src/test/java/org/apache/maven/configuration/internal/EnhancedCompositeBeanHelperTest.java
@@ -146,6 +146,68 @@ void testCacheClearance() throws Exception {
assertEquals("testValue", bean2.getName());
}
+ @Test
+ void testFieldAccessibilityIsProperlyRestored() throws Exception {
+ TestBean bean = new TestBean();
+ PlexusConfiguration config = new XmlPlexusConfiguration("test");
+ config.setValue("fieldValue");
+
+ when(evaluator.evaluate("fieldValue")).thenReturn("fieldValue");
+
+ // Get the field to check its accessibility state
+ java.lang.reflect.Field field =
TestBean.class.getDeclaredField("directField");
+
+ // Verify field is not accessible initially
+ boolean initialAccessibility = field.canAccess(bean);
+
+ // Set the property using the helper
+ helper.setProperty(bean, "directField", String.class, config);
+
+ // Verify the value was set correctly
+ assertEquals("fieldValue", bean.getDirectField());
+
+ // Verify field accessibility is restored to its original state
+ boolean finalAccessibility = field.canAccess(bean);
+ assertEquals(
+ initialAccessibility,
+ finalAccessibility,
+ "Field accessibility should be restored to its original state
after setting value");
+ }
+
+ @Test
+ void testMultipleFieldAccessesDoNotLeakAccessibility() throws Exception {
+ // This test verifies that repeated field accesses don't leave fields
in an accessible state
+ // which was the issue with the old caching implementation
+ TestBean bean1 = new TestBean();
+ TestBean bean2 = new TestBean();
+ PlexusConfiguration config = new XmlPlexusConfiguration("test");
+ config.setValue("value1");
+
+ when(evaluator.evaluate("value1")).thenReturn("value1");
+ when(evaluator.evaluate("value2")).thenReturn("value2");
+
+ java.lang.reflect.Field field =
TestBean.class.getDeclaredField("directField");
+
+ // First access
+ helper.setProperty(bean1, "directField", String.class, config);
+ boolean accessibilityAfterFirst = field.canAccess(bean1);
+
+ // Second access with different bean
+ config.setValue("value2");
+ helper.setProperty(bean2, "directField", String.class, config);
+ boolean accessibilityAfterSecond = field.canAccess(bean2);
+
+ // Both should have the same accessibility state (not accessible)
+ assertEquals(
+ accessibilityAfterFirst,
+ accessibilityAfterSecond,
+ "Field accessibility should be consistent across multiple
accesses");
+
+ // Verify values were set correctly
+ assertEquals("value1", bean1.getDirectField());
+ assertEquals("value2", bean2.getDirectField());
+ }
+
/**
* Test bean class for testing property setting.
*/
