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
commit f840ff12b6ecfe4911072e5d541a4d7c7fcae083 Author: Emil Huseynov <[email protected]> AuthorDate: Thu Oct 29 16:19:31 2026 +0400 WW-5820 Hold the declared-on-neither fallback to objects further down the value stack StrutsParameterAuthorizer allowed a property declared on neither the ModelDriven model nor the action, so that Map-backed models keep working. CompoundRootAccessor then writes to the first object in the root with a matching setter, which after a chain result can be the previous action, an object the authorizer never looked at. Before taking the fallback, look at the other objects in the value stack root. If one of them declares the property, apply the @StrutsParameter requirement to it. Model properties, annotated action members and Map-backed models are unchanged. --- .../parameter/StrutsParameterAuthorizer.java | 35 ++++++++++++++ .../parameter/ParameterAuthorizerTest.java | 56 ++++++++++++++++++++++ 2 files changed, 91 insertions(+) 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 a0feb6cd2..e17d8ce7d 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 @@ -171,6 +171,11 @@ public class StrutsParameterAuthorizer implements ParameterAuthorizer { * being a {@code Map} itself for instance. That is no more permissive than the blanket exemption this * scoping replaces. * <p> + * The fallback only applies when no other object in the value stack root declares the property either. A + * chain result leaves the previous action in the root below the chained action and its model, and OGNL + * writes to the first object in the root with a matching accessor, so a name declared there is held to the + * annotation requirement on that object, see {@link #findDeclaringObjectBelow}. + * <p> * {@code class} is the exception to that fallback: it is invisible to introspection here rather than absent, * so it is rejected instead of taking the fallback, which keeps a ModelDriven action from handing OGNL a * {@code class} path that the ordinary non-ModelDriven path would have rejected for want of an annotation. @@ -186,6 +191,12 @@ public class StrutsParameterAuthorizer implements ParameterAuthorizer { LOG.debug("Property [class] is not an unknown property but Object.getClass() on every object alike, so the fallback for a custom accessor does not apply; rejecting"); return false; } + Object declaringObject = findDeclaringObjectBelow(rootProperty, model, action, paramDepth); + if (declaringObject != null) { + LOG.debug("Property [{}] is declared on neither the model nor the action but on [{}] further down the value stack, applying the @StrutsParameter annotation requirement to it", + rootProperty, declaringObject.getClass().getName()); + return hasValidAnnotatedMember(rootProperty, declaringObject, paramDepth); + } LOG.debug("Property [{}] is declared on neither the model nor the action, exempting from @StrutsParameter annotation requirement", rootProperty); return true; @@ -195,6 +206,30 @@ public class StrutsParameterAuthorizer implements ParameterAuthorizer { return hasValidAnnotatedMember(rootProperty, action, paramDepth); } + /** + * Finds the first object in the value stack root, other than the model and the action, that declares the + * property. {@code CompoundRootAccessor} writes a parameter to the first object in the root with a matching + * accessor, so a name declared on neither the model nor the action can still land on an object further down, + * such as the previous action left in the root by a chain result. + * + * @return the declaring object, or {@code null} if no other object in the root declares the property + */ + protected Object findDeclaringObjectBelow(String rootProperty, Object model, Object action, long paramDepth) { + ActionContext context = ActionContext.getContext(); + if (context == null || context.getValueStack() == null) { + return null; + } + for (Object candidate : context.getValueStack().getRoot()) { + if (candidate == null || candidate == model || candidate == action) { + continue; + } + if (declaresProperty(candidate, rootProperty, paramDepth)) { + return candidate; + } + } + return null; + } + /** * Whether {@code target} can itself take {@code property} at this depth - as a bean property whose relevant * accessor exists, the setter for a depth-0 parameter and the getter for a nested one, or as a public instance diff --git a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizerTest.java b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizerTest.java index b412da4d7..325c559fc 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizerTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizerTest.java @@ -404,8 +404,64 @@ public class ParameterAuthorizerTest { assertThat(authorizer.resolveTarget(action)).isSameAs(action); } + // --- ModelDriven fallback and objects further down the value stack (WW-5820) --- + + private ModelActionWithOwnMembers chainedModelDrivenAction(Object previousAction) { + // A chain result runs the next action on the same value stack, so the previous action stays in the root + // below the chained action and its model. + var action = new ModelActionWithOwnMembers(); + var valueStack = new StubValueStack(); + valueStack.push(previousAction); + valueStack.push(action); + valueStack.push(action.getModel()); + ActionContext.of().withValueStack(valueStack).bind(); + return action; + } + + @Test + public void modelDriven_propertyOnNeither_declaredUnannotatedBelowInStack_rejected() { + var action = chainedModelDrivenAction(new PreviousAction()); + assertThat(authorizer.isAuthorized("foo", action.getModel(), action)).isFalse(); + } + + @Test + public void modelDriven_propertyOnNeither_declaredAnnotatedBelowInStack_authorized() { + var action = chainedModelDrivenAction(new AnnotatedPreviousAction()); + assertThat(authorizer.isAuthorized("foo", action.getModel(), action)).isTrue(); + } + + @Test + public void modelDriven_propertyOnNothingInStack_stillTakesFallback() { + var action = chainedModelDrivenAction(new PreviousAction()); + assertThat(authorizer.isAuthorized("mapKey", action.getModel(), action)).isTrue(); + } + + @Test + public void modelDriven_modelAndAnnotatedActionMembers_unaffectedByObjectsBelow() { + var action = chainedModelDrivenAction(new PreviousAction()); + assertThat(authorizer.isAuthorized("name", action.getModel(), action)).isTrue(); + assertThat(authorizer.isAuthorized("actionAllowed", action.getModel(), action)).isTrue(); + assertThat(authorizer.isAuthorized("actionSecret", action.getModel(), action)).isFalse(); + } + // --- Inner test classes --- + public static class PreviousAction { + private String foo; + + // NO @StrutsParameter + public void setFoo(String foo) { this.foo = foo; } + public String getFoo() { return foo; } + } + + public static class AnnotatedPreviousAction { + private String foo; + + @StrutsParameter + public void setFoo(String foo) { this.foo = foo; } + public String getFoo() { return foo; } + } + public static class SecureAction { private String name; private String role;
