This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/main by this push:
new 9aa77f721 WW-5759 Resolve @StrutsParameter accessors the way OGNL does
(#1992)
9aa77f721 is described below
commit 9aa77f7219db00c5ae1c7b728e0bca93d16e2053
Author: Lukasz Lenart <[email protected]>
AuthorDate: Tue Oct 6 15:15:06 2026 +0200
WW-5759 Resolve @StrutsParameter accessors the way OGNL does (#1992)
* WW-5759 fix(core): resolve @StrutsParameter accessors the way OGNL does
OGNL 3.4.14 tries setuRange before setURange for uRange, and reaches
getURL() for both
uRL and URL, while StrutsParameterAuthorizer looked only for set/get plus
the capitalised
name and matched nested parameters against the java.beans property name.
findBindableAccessor
now tries OGNL's accessor base names in OGNL's order, and the root property
is normalised
with Introspector.decapitalize so the candidates match OGNL's for the name
as written.
Bumps OGNL from 3.4.13 to 3.4.14.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* WW-5759 test(core): pin that case variants of an accessor name bind
nothing on a ModelDriven action
They resolve to no accessor on the model or the action and take the
"declared on neither"
route, which is only safe while OGNL does not bind them either.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* WW-5759 chore(core): address Sonar findings on the accessor resolution
change
Make the non-null target contract of ultimateClass explicit, drop throws
clauses the new
tests do not need and record the values the probe setters receive.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---------
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../parameter/StrutsParameterAuthorizer.java | 68 ++++++----
.../parameter/ParametersInterceptorTest.java | 142 +++++++++++++++++++++
pom.xml | 2 +-
3 files changed, 187 insertions(+), 25 deletions(-)
diff --git
a/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameterAuthorizer.java
b/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameterAuthorizer.java
index 481556eaf..a0feb6cd2 100644
---
a/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameterAuthorizer.java
+++
b/core/src/main/java/org/apache/struts2/interceptor/parameter/StrutsParameterAuthorizer.java
@@ -30,6 +30,7 @@ import org.apache.struts2.util.ProxyService;
import java.beans.BeanInfo;
import java.beans.IntrospectionException;
+import java.beans.Introspector;
import java.beans.MethodDescriptor;
import java.beans.PropertyDescriptor;
import java.lang.reflect.AnnotatedElement;
@@ -37,6 +38,7 @@ import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Arrays;
+import java.util.List;
import java.util.Objects;
import java.util.Optional;
@@ -132,7 +134,7 @@ public class StrutsParameterAuthorizer implements
ParameterAuthorizer {
parameterName);
return false;
}
- String normalisedRootProperty =
Character.toLowerCase(rootProperty.charAt(0)) + rootProperty.substring(1);
+ String normalisedRootProperty =
Introspector.decapitalize(rootProperty);
// Transition mode: depth-0 (non-nested) parameters are exempt.
Checked before the ModelDriven
// exemption so that it also covers a ModelDriven action's own
members, which would otherwise
@@ -213,37 +215,54 @@ public class StrutsParameterAuthorizer implements
ParameterAuthorizer {
* The method OGNL would go through to bind {@code property} on {@code
target} at this depth: the setter for a
* depth-0 parameter, the getter for a nested one.
* <p>
- * The setter is matched the way OGNL matches it - a public instance
method named {@code set} plus the
- * capitalised property name, taking one argument - and not through {@link
PropertyDescriptor#getWriteMethod()},
- * which {@link java.beans.Introspector} only fills in for a {@code void}
setter. A fluent setter returning
- * {@code this} is just as bindable to OGNL, so it has to be just as
visible here, both to carry a
- * {@link StrutsParameter} annotation and to count as declared on a {@link
ModelDriven} action.
+ * The accessor is matched the way OGNL matches it - a public instance
method named {@code set}, or {@code is}
+ * then {@code get}, plus each of {@link #accessorBaseNames}, taking one
argument or none, the first name with a
+ * match winning - and not through a {@link PropertyDescriptor}. {@link
java.beans.Introspector} only fills in
+ * the write method for a {@code void} setter, and names the property of
{@code getURL} {@code URL} where OGNL
+ * reaches it as {@code uRL} too. A fluent setter returning {@code this}
is just as bindable to OGNL, so it has
+ * to be just as visible here, both to carry a {@link StrutsParameter}
annotation and to count as declared on a
+ * {@link ModelDriven} action.
* <p>
- * Where several setters qualify, the one declared furthest down the
hierarchy wins - an override is what OGNL
- * invokes and what the developer annotated, while the erased setter of a
generic superclass is listed
- * alongside it and carries no annotation - and among overloads declared
at that level an annotated one, since
- * annotating any overload declares the property request surface.
+ * Where several methods of the winning name qualify, the one declared
furthest down the hierarchy wins - an
+ * override is what OGNL invokes and what the developer annotated, while
the erased setter of a generic
+ * superclass is listed alongside it and carries no annotation - and among
overloads declared at that level an
+ * annotated one, since annotating any overload declares the property
request surface.
*/
protected Optional<Method> findBindableAccessor(Object target, String
property, long paramDepth) {
BeanInfo beanInfo = getBeanInfo(target);
if (beanInfo == null) {
return Optional.empty();
}
- if (paramDepth == 0) {
- String setterName = "set" +
Character.toUpperCase(property.charAt(0)) + property.substring(1);
- return Arrays.stream(beanInfo.getMethodDescriptors())
- .map(MethodDescriptor::getMethod)
- .filter(method -> method.getName().equals(setterName)
- && method.getParameterCount() == 1
- && !Modifier.isStatic(method.getModifiers()))
- .max(comparingInt((Method method) ->
inheritanceDepth(method.getDeclaringClass()))
- .thenComparing(method ->
getParameterAnnotation(method) != null));
+ List<String> prefixes = paramDepth == 0 ? List.of("set") :
List.of("is", "get");
+ int parameterCount = paramDepth == 0 ? 1 : 0;
+ for (String baseName : accessorBaseNames(property)) {
+ for (String prefix : prefixes) {
+ String accessorName = prefix + baseName;
+ Optional<Method> accessor =
Arrays.stream(beanInfo.getMethodDescriptors())
+ .map(MethodDescriptor::getMethod)
+ .filter(method -> method.getName().equals(accessorName)
+ && method.getParameterCount() == parameterCount
+ && !Modifier.isStatic(method.getModifiers()))
+ .max(comparingInt((Method method) ->
inheritanceDepth(method.getDeclaringClass()))
+ .thenComparing(method ->
getParameterAnnotation(method) != null));
+ if (accessor.isPresent()) {
+ return accessor;
+ }
+ }
+ }
+ return Optional.empty();
+ }
+
+ /**
+ * The accessor base names OGNL tries for {@code property}, in its order:
for a name like {@code uRange} the
+ * JavaBeans {@code uRange} before the capitalised {@code URange},
otherwise the capitalised name alone.
+ */
+ private static List<String> accessorBaseNames(String property) {
+ String capitalised = Character.toUpperCase(property.charAt(0)) +
property.substring(1);
+ if (property.length() > 1 && Character.isLowerCase(property.charAt(0))
&& Character.isUpperCase(property.charAt(1))) {
+ return List.of(property, capitalised);
}
- return Arrays.stream(beanInfo.getPropertyDescriptors())
- .filter(desc -> desc.getName().equals(property))
- .map(PropertyDescriptor::getReadMethod)
- .filter(Objects::nonNull)
- .findFirst();
+ return List.of(capitalised);
}
private static int inheritanceDepth(Class<?> type) {
@@ -353,6 +372,7 @@ public class StrutsParameterAuthorizer implements
ParameterAuthorizer {
}
protected Class<?> ultimateClass(Object target) {
+ Objects.requireNonNull(target, "target");
if (proxyService.isProxy(target)) {
return proxyService.ultimateTargetClass(target);
}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
index 9b0d52b6b..d3c15279e 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParametersInterceptorTest.java
@@ -320,6 +320,104 @@ public class ParametersInterceptorTest extends
XWorkTestCase {
}
}
+ /**
+ * OGNL resolves {@code uRange} to the JavaBeans setter {@code setuRange}
before {@code setURange}, so that is the
+ * method which has to carry the annotation. The model property alongside
proves the parameters were applied.
+ */
+ public void
testModelDrivenJavaBeansStyleSetterOnActionRequiresAnnotation() {
+
loadButSet(Map.of(StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS,
"true"));
+ ParametersInterceptor pi = createParametersInterceptor();
+
+ JavaBeansNamedModelDrivenAction action = new
JavaBeansNamedModelDrivenAction();
+ ValueStack stack =
container.getInstance(ValueStackFactory.class).createValueStack();
+ stack.push(action);
+ stack.push(action.getModel());
+
ActionContext.of().withContainer(container).withValueStack(stack).bind();
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("uRange", "leaked through the unannotated setter");
+ params.put("xCoord", "bound through the annotated setter");
+ params.put("name", "bound on the model");
+ pi.applyParameters(action, stack,
HttpParameters.create(params).build());
+
+ assertEquals("bound on the model", action.getModel().getName());
+ assertEquals("bound through the annotated setter", action.getxCoord());
+ assertNull(action.getuRange());
+ }
+
+ /**
+ * A nested parameter goes through the getter OGNL resolves, {@code
getURL} for {@code uRL}, which
+ * introspection names property {@code URL}.
+ */
+ public void testModelDrivenUppercaseGetterOnActionRequiresAnnotation() {
+
loadButSet(Map.of(StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS,
"true"));
+ ParametersInterceptor pi = createParametersInterceptor();
+
+ JavaBeansNamedModelDrivenAction action = new
JavaBeansNamedModelDrivenAction();
+ ValueStack stack =
container.getInstance(ValueStackFactory.class).createValueStack();
+ stack.push(action);
+ stack.push(action.getModel());
+
ActionContext.of().withContainer(container).withValueStack(stack).bind();
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("uRL.host", "leaked through the unannotated getter");
+ params.put("URL.path", "leaked through the unannotated getter");
+ params.put("name", "bound on the model");
+ pi.applyParameters(action, stack,
HttpParameters.create(params).build());
+
+ assertEquals("bound on the model", action.getModel().getName());
+ assertTrue(action.getURL().isEmpty());
+ }
+
+ /**
+ * The annotation counts only on the setter OGNL actually invokes for the
parameter name as written.
+ */
+ public void testAnnotationOnlyCountsOnTheSetterOgnlInvokes() {
+
loadButSet(Map.of(StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS,
"true"));
+ ParametersInterceptor pi = createParametersInterceptor();
+
+ JavaBeansNamedAction action = new JavaBeansNamedAction();
+ ValueStack stack =
container.getInstance(ValueStackFactory.class).createValueStack();
+ stack.push(action);
+
ActionContext.of().withContainer(container).withValueStack(stack).bind();
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("xCoord", "bound through the annotated setter");
+ params.put("uRange", "leaked through the unannotated setuRange");
+ params.put("VRange", "leaked through the unannotated setVRange");
+ pi.applyParameters(action, stack,
HttpParameters.create(params).build());
+
+ assertEquals("bound through the annotated setter", action.getxCoord());
+ assertEquals(List.of(), action.getInvoked());
+ }
+
+ /**
+ * Names differing from an accessor's property only in case resolve to no
accessor on either object, so they
+ * take the "declared on neither" route - which is safe only as long as
OGNL does not bind them either.
+ */
+ public void testModelDrivenCaseVariantsBindNothingOnAction() {
+
loadButSet(Map.of(StrutsConstants.STRUTS_PARAMETERS_REQUIRE_ANNOTATIONS,
"true"));
+ ParametersInterceptor pi = createParametersInterceptor();
+
+ JavaBeansNamedModelDrivenAction action = new
JavaBeansNamedModelDrivenAction();
+ ValueStack stack =
container.getInstance(ValueStackFactory.class).createValueStack();
+ stack.push(action);
+ stack.push(action.getModel());
+
ActionContext.of().withContainer(container).withValueStack(stack).bind();
+
+ Map<String, Object> params = new HashMap<>();
+ params.put("urange", "not bound");
+ params.put("URANGE", "not bound");
+ params.put("url.host", "not bound");
+ params.put("Url.path", "not bound");
+ params.put("name", "bound on the model");
+ pi.applyParameters(action, stack,
HttpParameters.create(params).build());
+
+ assertEquals("bound on the model", action.getModel().getName());
+ assertNull(action.getuRange());
+ assertTrue(action.getURL().isEmpty());
+ }
+
public void testParametersDoesNotAffectSession() throws Exception {
Map<String, Object> params = new HashMap<>();
params.put("blah", "This is blah");
@@ -1110,6 +1208,50 @@ public class ParametersInterceptorTest extends
XWorkTestCase {
public Address getAddress() { return address; }
}
+ public static class JavaBeansNamedModelDrivenAction implements
ModelDriven<TestBean> {
+ private final TestBean model = new TestBean();
+ private final Map<String, String> url = new HashMap<>();
+ private String uRange;
+ private String xCoord;
+
+ @Override
+ public TestBean getModel() { return model; }
+
+ // NO @StrutsParameter
+ public void setuRange(String uRange) { this.uRange = uRange; }
+ public String getuRange() { return uRange; }
+
+ @StrutsParameter
+ public void setxCoord(String xCoord) { this.xCoord = xCoord; }
+ public String getxCoord() { return xCoord; }
+
+ // NO @StrutsParameter
+ public Map<String, String> getURL() { return url; }
+ }
+
+ public static class JavaBeansNamedAction {
+ private final List<String> invoked = new ArrayList<>();
+ private String xCoord;
+
+ @StrutsParameter
+ public void setxCoord(String xCoord) { this.xCoord = xCoord; }
+ public String getxCoord() { return xCoord; }
+
+ // NO @StrutsParameter - OGNL prefers it to the annotated setURange
for "uRange"
+ public void setuRange(String uRange) { invoked.add("setuRange=" +
uRange); }
+
+ @StrutsParameter
+ public void setURange(String uRange) { invoked.add("setURange=" +
uRange); }
+
+ @StrutsParameter
+ public void setvRange(String vRange) { invoked.add("setvRange=" +
vRange); }
+
+ // NO @StrutsParameter - the only setter OGNL considers for "VRange"
+ public void setVRange(String vRange) { invoked.add("setVRange=" +
vRange); }
+
+ public List<String> getInvoked() { return invoked; }
+ }
+
public static class Address {
private String city;
public String getCity() { return city; }
diff --git a/pom.xml b/pom.xml
index 89594f359..b24d1c78d 100644
--- a/pom.xml
+++ b/pom.xml
@@ -126,7 +126,7 @@
<jaxb-impl.version>4.0.9</jaxb-impl.version>
<log4j2.version>2.26.1</log4j2.version>
<mockito.version>5.23.0</mockito.version>
- <ognl.version>3.4.13</ognl.version>
+ <ognl.version>3.4.14</ognl.version>
<spring.version>6.2.19</spring.version>
<struts-annotations.version>2.0</struts-annotations.version>
<velocity-tools.version>3.1</velocity-tools.version>
