This is an automated email from the ASF dual-hosted git repository.
kusal pushed a commit to branch WW-5352-parameter-annotation-4
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 805f2ba724cd04870cc2d00932dceb8a50e98b46
Author: Kusal Kithul-Godage <g...@kusal.io>
AuthorDate: Tue Jan 30 01:47:30 2024 +1100
WW-5352 Ensure StrutsParameter compatibility with TypeConversion setters
---
.../xwork2/ognl/SecurityMemberAccess.java | 8 ++-
.../parameter/ParametersInterceptor.java | 59 +++++++++++++---------
.../parameter/StrutsParameterAnnotationTest.java | 22 ++++++++
3 files changed, 65 insertions(+), 24 deletions(-)
diff --git
a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
index 510a65c60..0776c90b7 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ognl/SecurityMemberAccess.java
@@ -67,12 +67,18 @@ public class SecurityMemberAccess implements MemberAccess {
)));
private static final Set<Class<?>> ALLOWLIST_REQUIRED_CLASSES =
unmodifiableSet(new HashSet<>(Arrays.asList(
+ com.opensymphony.xwork2.conversion.impl.XWorkList.class,
java.lang.Enum.class,
java.lang.String.class,
+ java.util.ArrayList.class,
+ java.util.Collection.class,
java.util.Date.class,
java.util.HashMap.class,
+ java.util.HashSet.class,
+ java.util.List.class,
java.util.Map.class,
- java.util.Map.Entry.class
+ java.util.Map.Entry.class,
+ java.util.Set.class
)));
private final ProviderAllowlist providerAllowlist;
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
index e9215e533..2766bc36c 100644
---
a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
+++
b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParametersInterceptor.java
@@ -51,7 +51,6 @@ import java.beans.Introspector;
import java.beans.PropertyDescriptor;
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Field;
-import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
@@ -368,7 +367,14 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
String rootProperty = nestingIndex == -1 ? name : name.substring(0,
nestingIndex);
String normalisedRootProperty =
Character.toLowerCase(rootProperty.charAt(0)) + rootProperty.substring(1);
- return hasValidAnnotatedMember(normalisedRootProperty, action,
paramDepth);
+ boolean result = hasValidAnnotatedMember(normalisedRootProperty,
action, paramDepth);
+ if (!result) {
+ LOG.debug(
+ "Parameter injection for root property [{}] on action [{}]
rejected. Ensure the corresponding getter or setter is annotated with
@StrutsParameter with an appropriate 'depth'.",
+ normalisedRootProperty,
+ action.getClass().getName());
+ }
+ return result;
}
/**
@@ -396,28 +402,39 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
}
protected boolean hasValidAnnotatedPropertyDescriptor(PropertyDescriptor
propDesc, long paramDepth) {
- Method relevantMethod = paramDepth == 0 ? propDesc.getWriteMethod() :
propDesc.getReadMethod();
- if (relevantMethod == null) {
- return false;
- }
- if (getPermittedInjectionDepth(relevantMethod) < paramDepth) {
- LOG.debug(
- "Parameter injection for method [{}] on action [{}]
rejected. Ensure it is annotated with @StrutsParameter with an appropriate
'depth'.",
- relevantMethod.getName(),
- relevantMethod.getDeclaringClass().getName());
- return false;
+ int permittedDepth = -1;
+ if (paramDepth > 0 && propDesc.getReadMethod() != null) {
+ permittedDepth =
getPermittedInjectionDepth(propDesc.getReadMethod());
}
- if (paramDepth >= 1) {
- allowlistClass(relevantMethod.getReturnType());
+ if (permittedDepth == -1 && propDesc.getWriteMethod() != null) {
+ permittedDepth =
getPermittedInjectionDepth(propDesc.getWriteMethod());
}
- if (paramDepth >= 2) {
- allowlistReturnTypeIfParameterized(relevantMethod);
+
+ if (permittedDepth < paramDepth) {
+ return false;
}
+
+ autoAllowlistTypes(propDesc, paramDepth);
return true;
}
- protected void allowlistReturnTypeIfParameterized(Method method) {
- allowlistParameterizedTypeArg(method.getGenericReturnType());
+ protected void autoAllowlistTypes(PropertyDescriptor propDesc, long
paramDepth) {
+ if (propDesc.getReadMethod() != null) {
+ if (paramDepth >= 1) {
+ allowlistClass(propDesc.getReadMethod().getReturnType());
+ }
+ if (paramDepth >= 2) {
+
allowlistParameterizedTypeArg(propDesc.getReadMethod().getGenericReturnType());
+ }
+ }
+ if (propDesc.getWriteMethod() != null) {
+ if (paramDepth >= 1) {
+
allowlistClass(propDesc.getWriteMethod().getParameterTypes()[0]);
+ }
+ if (paramDepth >= 2) {
+
allowlistParameterizedTypeArg(propDesc.getWriteMethod().getGenericParameterTypes()[0]);
+ }
+ }
}
protected void allowlistParameterizedTypeArg(Type genericType) {
@@ -465,15 +482,11 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
allowlistClass(field.getType());
}
if (paramDepth >= 2) {
- allowlistFieldIfParameterized(field);
+ allowlistParameterizedTypeArg(field.getGenericType());
}
return true;
}
- protected void allowlistFieldIfParameterized(Field field) {
- allowlistParameterizedTypeArg(field.getGenericType());
- }
-
/**
* @return permitted injection depth where -1 indicates not permitted
*/
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
index 53fa14717..e8d51d31a 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/parameter/StrutsParameterAnnotationTest.java
@@ -246,6 +246,17 @@ public class StrutsParameterAnnotationTest {
assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Map.class,
String.class, Pojo.class));
}
+ @Test
+ public void setterPojoListDepthOneMethod() {
+ testParameter(new SetterOnlyAction(), "pojosDepthOne[0].key", false);
+ }
+
+ @Test
+ public void setterPojoListDepthTwoMethod() {
+ testParameter(new SetterOnlyAction(), "pojosDepthTwo[0].key", true);
+
assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(List.class,
Pojo.class));
+ }
+
@Test
public void publicStrNotAnnotated_transitionMode() {
parametersInterceptor.setRequireAnnotationsTransitionMode(Boolean.TRUE.toString());
@@ -343,6 +354,17 @@ public class StrutsParameterAnnotationTest {
}
}
+ class SetterOnlyAction {
+
+ @StrutsParameter(depth = 1)
+ public void setPojosDepthOne(List<Pojo> pojos) {
+ }
+
+ @StrutsParameter(depth = 2)
+ public void setPojosDepthTwo(List<Pojo> pojos) {
+ }
+ }
+
class Pojo {
}
}
