This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5468-modeldriven
in repository https://gitbox.apache.org/repos/asf/struts.git
commit fead176e8a553d2d7fe5703d1075d84db1bc94e5
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Fri Oct 11 10:49:13 2024 +0200
WW-5468 Fixes @StrutsParameter for ModelDriven
---
.../java/com/opensymphony/xwork2/ModelDriven.java | 3 ++
.../parameter/ParametersInterceptor.java | 53 ++++++++++++++--------
.../parameter/StrutsParameterAnnotationTest.java | 34 ++++++++++++--
.../apache/struts2/junit/StrutsRestTestCase.java | 2 +-
4 files changed, 68 insertions(+), 24 deletions(-)
diff --git a/core/src/main/java/com/opensymphony/xwork2/ModelDriven.java
b/core/src/main/java/com/opensymphony/xwork2/ModelDriven.java
index c07c6bbe7..59641a997 100644
--- a/core/src/main/java/com/opensymphony/xwork2/ModelDriven.java
+++ b/core/src/main/java/com/opensymphony/xwork2/ModelDriven.java
@@ -18,6 +18,8 @@
*/
package com.opensymphony.xwork2;
+import org.apache.struts2.interceptor.parameter.StrutsParameter;
+
/**
* ModelDriven Actions provide a model object to be pushed onto the ValueStack
* in addition to the Action itself, allowing a FormBean type approach like
Struts.
@@ -31,6 +33,7 @@ public interface ModelDriven<T> {
*
* @return the model
*/
+ @StrutsParameter
T getModel();
}
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 353f3cb82..a1640a820 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
@@ -20,6 +20,7 @@ package org.apache.struts2.interceptor.parameter;
import com.opensymphony.xwork2.ActionContext;
import com.opensymphony.xwork2.ActionInvocation;
+import com.opensymphony.xwork2.ModelDriven;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.interceptor.MethodFilterInterceptor;
import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
@@ -348,7 +349,7 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
}
long paramDepth = name.codePoints().mapToObj(c -> (char)
c).filter(NESTING_CHARS::contains).count();
- if (requireAnnotationsTransitionMode && paramDepth == 0) {
+ if (!(action instanceof ModelDriven) &&
requireAnnotationsTransitionMode && paramDepth == 0) {
return true;
}
@@ -365,29 +366,38 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
* save computation by checking this last.
*/
protected boolean hasValidAnnotatedMember(String rootProperty, Object
action, long paramDepth) {
+ final String property = action instanceof ModelDriven ? "model" :
rootProperty;
+ LOG.debug("Using [{}] as a root property of [{}]", property,
action.getClass().getSimpleName());
+
BeanInfo beanInfo = getBeanInfo(action);
if (beanInfo == null) {
- return hasValidAnnotatedField(action, rootProperty, paramDepth);
+ return hasValidAnnotatedField(action, property, paramDepth);
}
Optional<PropertyDescriptor> propDescOpt =
Arrays.stream(beanInfo.getPropertyDescriptors())
- .filter(desc ->
desc.getName().equals(rootProperty)).findFirst();
+ .filter(desc -> desc.getName().equals(property)).findFirst();
if (propDescOpt.isEmpty()) {
- return hasValidAnnotatedField(action, rootProperty, paramDepth);
+ return hasValidAnnotatedField(action, property, paramDepth);
}
if (hasValidAnnotatedPropertyDescriptor(action, propDescOpt.get(),
paramDepth)) {
return true;
}
- return hasValidAnnotatedField(action, rootProperty, paramDepth);
+ return hasValidAnnotatedField(action, property, paramDepth);
}
protected boolean hasValidAnnotatedPropertyDescriptor(Object action,
PropertyDescriptor propDesc, long paramDepth) {
- Method relevantMethod = paramDepth == 0 ? propDesc.getWriteMethod() :
propDesc.getReadMethod();
+ Method relevantMethod = paramDepth == 0
+ ? action instanceof ModelDriven<?> ? propDesc.getReadMethod()
: propDesc.getWriteMethod()
+ : propDesc.getReadMethod();
+
if (relevantMethod == null) {
+ LOG.debug("No such property [{}] in action [{}]",
propDesc.getName(), action.getClass().getSimpleName());
return false;
}
+
+ LOG.debug("Using [{}] as a property of [{}]",
relevantMethod.getName(), action.getClass().getSimpleName());
if (getPermittedInjectionDepth(relevantMethod) < paramDepth) {
String logMessage = format(
"Parameter injection for method [%s] on action [%s]
rejected. Ensure it is annotated with @StrutsParameter with an appropriate
'depth'.",
@@ -400,6 +410,9 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
}
return false;
}
+ if (action instanceof ModelDriven) {
+ allowlistClass(propDesc.getPropertyType());
+ }
if (paramDepth >= 1) {
allowlistClass(relevantMethod.getReturnType());
}
@@ -439,18 +452,22 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
protected boolean hasValidAnnotatedField(Object action, String fieldName,
long paramDepth) {
Field field;
+ LOG.debug("Checking if field [{}] of [{}] is annotated with [{}]",
+ fieldName, action.getClass().getSimpleName(),
StrutsParameter.class.getSimpleName());
try {
field = action.getClass().getDeclaredField(fieldName);
} catch (NoSuchFieldException e) {
+ LOG.debug("Field [{}] not found", fieldName);
return false;
}
if (!Modifier.isPublic(field.getModifiers())) {
+ LOG.debug("Field [{}] is not public", fieldName);
return false;
}
if (getPermittedInjectionDepth(field) < paramDepth) {
String logMessage = format(
"Parameter injection for field [%s] on action [%s]
rejected. Ensure it is annotated with @StrutsParameter with an appropriate
'depth'.",
- fieldName,
+ field.getName(),
action.getClass().getName());
if (devMode) {
notifyDeveloperOfError(LOG, action, logMessage);
@@ -591,9 +608,9 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
if (!matchLength) {
if (devMode) {
LOG.warn("Parameter [{}] is too long, allowed length is [{}].
Use Interceptor Parameter Overriding " +
- "to override the limit, see more at\n" +
-
"https://struts.apache.org/core-developers/interceptors.html#interceptor-parameter-overriding";,
- name, paramNameMaxLength);
+ "to override the limit, see more at\n" +
+
"https://struts.apache.org/core-developers/interceptors.html#interceptor-parameter-overriding";,
+ name, paramNameMaxLength);
} else {
LOG.warn("Parameter [{}] is too long, allowed length is [{}]",
name, paramNameMaxLength);
}
@@ -621,8 +638,8 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
if (result.isExcluded()) {
if (devMode) {
LOG.warn("Parameter [{}] matches excluded pattern [{}]! See
Accepted / Excluded patterns at\n" +
-
"https://struts.apache.org/security/#accepted--excluded-patterns";,
- paramName, result.getExcludedPattern());
+
"https://struts.apache.org/security/#accepted--excluded-patterns";,
+ paramName, result.getExcludedPattern());
} else {
LOG.debug("Parameter [{}] matches excluded pattern [{}]!",
paramName, result.getExcludedPattern());
}
@@ -640,8 +657,8 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
if (excludedValuePattern.matcher(value).matches()) {
if (devMode) {
LOG.warn("Parameter value [{}] matches excluded pattern
[{}]! See Accepting/Excluding parameter values at\n" +
-
"https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values";,
- value, excludedValuePatterns);
+
"https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values";,
+ value, excludedValuePatterns);
} else {
LOG.debug("Parameter value [{}] matches excluded pattern
[{}]", value, excludedValuePattern);
}
@@ -663,8 +680,8 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
}
if (devMode) {
LOG.warn("Parameter value [{}] didn't match accepted pattern [{}]!
See Accepting/Excluding parameter values at\n" +
-
"https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values";,
- value, acceptedValuePatterns);
+
"https://struts.apache.org/core-developers/parameters-interceptor#excluding-parameter-values";,
+ value, acceptedValuePatterns);
} else {
LOG.debug("Parameter value [{}] was not accepted!", value);
}
@@ -734,7 +751,7 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
LOG.debug("Sets accepted value patterns to [{}], note this may
impact the safety of your application!", patterns);
} else {
LOG.warn("Replacing accepted patterns [{}] with [{}], be aware
that this may impact safety of your application!",
- acceptedValuePatterns, patterns);
+ acceptedValuePatterns, patterns);
}
acceptedValuePatterns = new HashSet<>(patterns.size());
try {
@@ -759,7 +776,7 @@ public class ParametersInterceptor extends
MethodFilterInterceptor {
LOG.debug("Setting excluded value patterns to [{}]", patterns);
} else {
LOG.warn("Replacing excluded value patterns [{}] with [{}], be
aware that this may impact safety of your application!",
- excludedValuePatterns, patterns);
+ excludedValuePatterns, patterns);
}
excludedValuePatterns = new HashSet<>(patterns.size());
try {
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..eb44fc6fe 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
@@ -18,6 +18,7 @@
*/
package org.apache.struts2.interceptor.parameter;
+import com.opensymphony.xwork2.ModelDriven;
import com.opensymphony.xwork2.security.AcceptedPatternsChecker;
import com.opensymphony.xwork2.security.NotExcludedAcceptedPatternsChecker;
import org.apache.commons.lang3.ClassUtils;
@@ -80,7 +81,7 @@ public class StrutsParameterAnnotationTest {
}
}
- private Set<Class<?>> getParentClasses(Class<?> ...clazzes) {
+ private Set<Class<?>> getParentClasses(Class<?>... clazzes) {
Set<Class<?>> set = new HashSet<>();
for (Class<?> clazz : clazzes) {
set.add(clazz);
@@ -258,8 +259,14 @@ public class StrutsParameterAnnotationTest {
testParameter(new MethodAction(), "publicStrNotAnnotated", true);
}
+ @Test
+ public void publicModelPojo() {
+
parametersInterceptor.setRequireAnnotationsTransitionMode(Boolean.TRUE.toString());
+ testParameter(new ModelAction(), "name", true);
+
assertThat(threadAllowlist.getAllowlist()).containsExactlyInAnyOrderElementsOf(getParentClasses(Object.class,
Pojo.class));
+ }
- class FieldAction {
+ static class FieldAction {
@StrutsParameter
private String privateStr;
@@ -275,7 +282,7 @@ public class StrutsParameterAnnotationTest {
public Pojo publicPojoDepthZero;
@StrutsParameter(depth = 1)
- public Pojo publicPojoDepthOne ;
+ public Pojo publicPojoDepthOne;
@StrutsParameter(depth = 2)
public Pojo publicPojoDepthTwo;
@@ -290,7 +297,7 @@ public class StrutsParameterAnnotationTest {
public Map<String, Pojo> publicPojoMapDepthTwo;
}
- class MethodAction {
+ static class MethodAction {
@StrutsParameter
private void setPrivateStr(String str) {
@@ -343,6 +350,23 @@ public class StrutsParameterAnnotationTest {
}
}
- class Pojo {
+ static class ModelAction implements ModelDriven<Pojo> {
+
+ @StrutsParameter
+ public Pojo getModel() {
+ return new Pojo();
+ }
+ }
+
+ static class Pojo {
+ private String name;
+
+ public String getName() {
+ return name;
+ }
+
+ public void setName(String name) {
+ this.name = name;
+ }
}
}
diff --git
a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java
b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java
index 6f55eefde..3610f0c84 100644
---
a/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java
+++
b/plugins/junit/src/main/java/org/apache/struts2/junit/StrutsRestTestCase.java
@@ -75,7 +75,7 @@ public class StrutsRestTestCase<T> extends
StrutsJUnit4TestCase<T> {
ActionMapping mapping = getActionMapping(request);
assertNotNull(mapping);
- Dispatcher.getInstance().serviceAction(request, response, mapping);
+ dispatcher.serviceAction(request, response, mapping);
if (response.getStatus() != HttpServletResponse.SC_OK)
throw new ServletException("Error code [" + response.getStatus() +
"], Error: ["
