This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5626-cleanup in repository https://gitbox.apache.org/repos/asf/struts.git
commit 1ac148eb59c34aeae72d2730e668df415d29ed2b Author: Lukasz Lenart <[email protected]> AuthorDate: Mon May 4 12:22:31 2026 +0200 WW-5626 add ParameterAuthorizer#resolveTarget for centralized ModelDriven resolution Move the ValueStack peek logic that derives the target object from action+ModelDriven state out of ParametersInterceptor and into ParameterAuthorizer. Callers that need both authorization and the resolved target (for downstream OGNL allowlisting) can now call resolveTarget once and reuse the result. --- .../interceptor/parameter/ParameterAuthorizer.java | 14 ++++++++ .../parameter/StrutsParameterAuthorizer.java | 12 +++++++ .../parameter/ParameterAuthorizerTest.java | 39 ++++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizer.java b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizer.java index d064fbf68..5eb54a08e 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizer.java +++ b/core/src/main/java/org/apache/struts2/interceptor/parameter/ParameterAuthorizer.java @@ -45,4 +45,18 @@ public interface ParameterAuthorizer { * @return {@code true} if the parameter is authorized for injection, {@code false} otherwise */ boolean isAuthorized(String parameterName, Object target, Object action); + + /** + * Resolves the target object whose annotations should be checked for authorization. + * For {@link org.apache.struts2.ModelDriven} actions, this returns the model from the value stack; + * for non-ModelDriven actions, it returns the action itself. + * + * <p>Callers that need both authorization checks AND the resolved target (e.g. for downstream OGNL allowlisting) + * should call this once and reuse the result.</p> + * + * @param action the action instance + * @return the resolved target — either the action or its model + * @since 7.2.0 + */ + Object resolveTarget(Object action); } 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 82e47717e..03cc22c0e 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 @@ -21,6 +21,7 @@ package org.apache.struts2.interceptor.parameter; import org.apache.commons.lang3.BooleanUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.ActionContext; import org.apache.struts2.ModelDriven; import org.apache.struts2.StrutsConstants; import org.apache.struts2.inject.Inject; @@ -91,6 +92,17 @@ public class StrutsParameterAuthorizer implements ParameterAuthorizer { this.requireAnnotationsTransitionMode = BooleanUtils.toBoolean(transitionMode); } + @Override + public Object resolveTarget(Object action) { + if (action instanceof ModelDriven<?>) { + Object stackTop = ActionContext.getContext().getValueStack().peek(); + if (!stackTop.equals(action)) { + return stackTop; + } + } + return action; + } + @Override public boolean isAuthorized(String parameterName, Object target, Object action) { if (parameterName == null || parameterName.isEmpty()) { 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 f7c16fbb9..43eb57bcc 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 @@ -18,13 +18,16 @@ */ package org.apache.struts2.interceptor.parameter; +import org.apache.struts2.ActionContext; import org.apache.struts2.ModelDriven; +import org.apache.struts2.StubValueStack; import org.apache.struts2.ognl.DefaultOgnlBeanInfoCacheFactory; import org.apache.struts2.ognl.DefaultOgnlExpressionCacheFactory; import org.apache.struts2.ognl.OgnlUtil; import org.apache.struts2.ognl.StrutsOgnlGuard; import org.apache.struts2.ognl.StrutsProxyCacheFactory; import org.apache.struts2.util.StrutsProxyService; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -56,6 +59,11 @@ public class ParameterAuthorizerTest { authorizer.setProxyService(proxyService); } + @After + public void tearDown() { + ActionContext.clear(); + } + // --- requireAnnotations=false (backward compat) --- @Test @@ -182,6 +190,37 @@ public class ParameterAuthorizerTest { assertThat(authorizer.isAuthorized(null, action, action)).isFalse(); } + // --- resolveTarget --- + + @Test + public void resolveTarget_nonModelDriven_returnsAction() { + var action = new SecureAction(); + assertThat(authorizer.resolveTarget(action)).isSameAs(action); + } + + @Test + public void resolveTarget_modelDriven_returnsModelFromValueStack() { + var action = new ModelAction(); + var model = action.getModel(); + var valueStack = new StubValueStack(); + valueStack.push(model); + ActionContext.of().withValueStack(valueStack).bind(); + + assertThat(authorizer.resolveTarget(action)).isSameAs(model); + } + + @Test + public void resolveTarget_modelDriven_stackTopEqualsAction_returnsAction() { + // Edge case: ModelDriven action where stack top equals the action itself. + // No exemption applies — target stays as action. + var action = new ModelAction(); + var valueStack = new StubValueStack(); + valueStack.push(action); + ActionContext.of().withValueStack(valueStack).bind(); + + assertThat(authorizer.resolveTarget(action)).isSameAs(action); + } + // --- Inner test classes --- public static class SecureAction {
