From 6dbe7ab5abe2be2a54d00671acd3d01b89c71433 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:14:57 +0200 Subject: [PATCH 01/10] WW-5631 feat(chaining): add struts.chaining.requireAnnotations constant Co-Authored-By: Claude Opus 4.7 --- core/src/main/java/org/apache/struts2/StrutsConstants.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 13fc2d3d29..8b9ff58c18 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -734,6 +734,7 @@ public final class StrutsConstants { public static final String STRUTS_CHAINING_COPY_ERRORS = "struts.chaining.copyErrors"; public static final String STRUTS_CHAINING_COPY_FIELD_ERRORS = "struts.chaining.copyFieldErrors"; public static final String STRUTS_CHAINING_COPY_MESSAGES = "struts.chaining.copyMessages"; + public static final String STRUTS_CHAINING_REQUIRE_ANNOTATIONS = "struts.chaining.requireAnnotations"; public static final String STRUTS_OBJECT_FACTORY_CLASSLOADER = "struts.objectFactory.classloader"; /** From c9515552676ca000447aaeb89c1bce3dd1a043d6 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:15:15 +0200 Subject: [PATCH 02/10] WW-5631 feat(chaining): default struts.chaining.requireAnnotations=false Co-Authored-By: Claude Opus 4.7 --- .../src/main/resources/org/apache/struts2/default.properties | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index fcf9c8543a..e034f2835a 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -257,6 +257,11 @@ struts.parameters.requireAnnotations=true ### Useful for transitioning legacy applications, but highly recommended to set to false as soon as possible! struts.parameters.requireAnnotations.transitionMode=false +### Whether ChainingInterceptor enforces @StrutsParameter on the target action when copying properties. +### Opt-in hardening; default false preserves legacy chaining behaviour. Only has effect when +### struts.parameters.requireAnnotations is also enabled. +struts.chaining.requireAnnotations=false + ### Whether to throw a RuntimeException when a property is not found ### in an expression, or when the expression evaluation fails struts.el.throwExceptionOnFailure=false From ddc38e3581e5e2383a6458fafb7786fbdede69ba Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:15:36 +0200 Subject: [PATCH 03/10] WW-5631 test(chaining): add annotated/unannotated chaining fixtures Co-Authored-By: Claude Opus 4.7 --- .../interceptor/AnnotatedChainingAction.java | 45 +++++++++++++++++++ .../UnannotatedChainingAction.java | 43 ++++++++++++++++++ 2 files changed, 88 insertions(+) create mode 100644 core/src/test/java/org/apache/struts2/interceptor/AnnotatedChainingAction.java create mode 100644 core/src/test/java/org/apache/struts2/interceptor/UnannotatedChainingAction.java diff --git a/core/src/test/java/org/apache/struts2/interceptor/AnnotatedChainingAction.java b/core/src/test/java/org/apache/struts2/interceptor/AnnotatedChainingAction.java new file mode 100644 index 0000000000..d5d69741f4 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/AnnotatedChainingAction.java @@ -0,0 +1,45 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.interceptor; + +import org.apache.struts2.action.Action; +import org.apache.struts2.interceptor.parameter.StrutsParameter; + +/** + * Test fixture: target/source action whose {@code managerApproved} property is annotated with + * {@link StrutsParameter}. Used by {@link ChainingInterceptorTest}. + */ +public class AnnotatedChainingAction implements Action { + + private boolean managerApproved; + + public boolean getManagerApproved() { + return managerApproved; + } + + @StrutsParameter + public void setManagerApproved(boolean managerApproved) { + this.managerApproved = managerApproved; + } + + @Override + public String execute() { + return SUCCESS; + } +} diff --git a/core/src/test/java/org/apache/struts2/interceptor/UnannotatedChainingAction.java b/core/src/test/java/org/apache/struts2/interceptor/UnannotatedChainingAction.java new file mode 100644 index 0000000000..fd1502de4e --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/UnannotatedChainingAction.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.interceptor; + +import org.apache.struts2.action.Action; + +/** + * Test fixture: target action whose {@code managerApproved} property is NOT annotated with + * {@code @StrutsParameter}. Used by {@link ChainingInterceptorTest}. + */ +public class UnannotatedChainingAction implements Action { + + private boolean managerApproved; + + public boolean getManagerApproved() { + return managerApproved; + } + + public void setManagerApproved(boolean managerApproved) { + this.managerApproved = managerApproved; + } + + @Override + public String execute() { + return SUCCESS; + } +} From 4fa2782b3b20be0828ef818cb9f405dcbb132983 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:17:36 +0200 Subject: [PATCH 04/10] WW-5631 test(chaining): add failing @StrutsParameter enforcement tests Co-Authored-By: Claude Opus 4.7 --- .../interceptor/ChainingInterceptorTest.java | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java index 85df164cce..e59780a93c 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java @@ -27,6 +27,9 @@ import org.apache.struts2.TestBean; import org.apache.struts2.XWorkTestCase; import org.apache.struts2.util.ValueStack; +import org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer; +import org.apache.struts2.ognl.OgnlUtil; +import org.apache.struts2.util.ProxyService; import java.util.*; @@ -149,6 +152,94 @@ public void testNullCompoundRootElementAllowsProcessToContinue() throws Exceptio } + private StrutsParameterAuthorizer buildAuthorizer(boolean requireAnnotations, boolean transitionMode) { + StrutsParameterAuthorizer authorizer = new StrutsParameterAuthorizer(); + authorizer.setOgnlUtil(container.getInstance(OgnlUtil.class)); + authorizer.setProxyService(container.getInstance(ProxyService.class)); + authorizer.setRequireAnnotations(String.valueOf(requireAnnotations)); + authorizer.setRequireAnnotationsTransitionMode(String.valueOf(transitionMode)); + return authorizer; + } + + private void enableChainingEnforcement(boolean requireAnnotations, boolean transitionMode) { + interceptor.setParameterAuthorizer(buildAuthorizer(requireAnnotations, transitionMode)); + interceptor.setRequireAnnotations("true"); + } + + public void testFlagOffCopiesUnannotatedProperty() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + UnannotatedChainingAction target = new UnannotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + interceptor.intercept(invocation); + + assertTrue("legacy chaining should copy the property when flag is off", target.getManagerApproved()); + } + + public void testFlagOnSkipsUnannotatedProperty() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + UnannotatedChainingAction target = new UnannotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + enableChainingEnforcement(true, false); + interceptor.intercept(invocation); + + assertFalse("unannotated target property must NOT be copied when enforcement is on", + target.getManagerApproved()); + } + + public void testFlagOnCopiesAnnotatedProperty() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + AnnotatedChainingAction target = new AnnotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + enableChainingEnforcement(true, false); + interceptor.intercept(invocation); + + assertTrue("annotated target property should be copied when enforcement is on", + target.getManagerApproved()); + } + + public void testTransitionModeCopiesNonNestedUnannotatedProperty() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + UnannotatedChainingAction target = new UnannotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + enableChainingEnforcement(true, true); + interceptor.intercept(invocation); + + assertTrue("transition mode should copy depth-0 property without annotation", + target.getManagerApproved()); + } + + public void testRequireAnnotationsFalseIsNoOp() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + UnannotatedChainingAction target = new UnannotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + interceptor.setParameterAuthorizer(buildAuthorizer(false, false)); + interceptor.setRequireAnnotations("true"); + interceptor.intercept(invocation); + + assertTrue("when global requireAnnotations is off, enforcement is a no-op", + target.getManagerApproved()); + } + @Override protected void setUp() throws Exception { super.setUp(); From ca36a7d46cea0eae324933faae2b5ad15e03538a Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:18:50 +0200 Subject: [PATCH 05/10] WW-5631 feat(chaining): enforce @StrutsParameter on target when opted in Co-Authored-By: Claude Opus 4.7 --- .../interceptor/ChainingInterceptor.java | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java index 9c18d88696..c9b8a87c8f 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java @@ -24,6 +24,8 @@ import org.apache.struts2.StrutsConstants; import org.apache.struts2.Unchainable; import org.apache.struts2.inject.Inject; +import org.apache.struts2.interceptor.parameter.ParameterAuthorizer; +import org.apache.struts2.ognl.OgnlUtil; import org.apache.struts2.result.ActionChainResult; import org.apache.struts2.result.Result; import org.apache.struts2.util.CompoundRoot; @@ -32,12 +34,16 @@ import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.reflection.ReflectionProvider; +import java.beans.BeanInfo; +import java.beans.IntrospectionException; +import java.beans.PropertyDescriptor; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; /** @@ -135,6 +141,9 @@ public class ChainingInterceptor extends AbstractInterceptor { protected Collection includes; protected ReflectionProvider reflectionProvider; private ProxyService proxyService; + private boolean requireAnnotations = false; + private ParameterAuthorizer parameterAuthorizer; + private OgnlUtil ognlUtil; @Inject public void setReflectionProvider(ReflectionProvider prov) { @@ -146,6 +155,21 @@ public void setProxyService(ProxyService proxyService) { this.proxyService = proxyService; } + @Inject + public void setParameterAuthorizer(ParameterAuthorizer parameterAuthorizer) { + this.parameterAuthorizer = parameterAuthorizer; + } + + @Inject + public void setOgnlUtil(OgnlUtil ognlUtil) { + this.ognlUtil = ognlUtil; + } + + @Inject(value = StrutsConstants.STRUTS_CHAINING_REQUIRE_ANNOTATIONS, required = false) + public void setRequireAnnotations(String requireAnnotations) { + this.requireAnnotations = "true".equalsIgnoreCase(requireAnnotations); + } + @Inject(value = StrutsConstants.STRUTS_CHAINING_COPY_ERRORS, required = false) public void setCopyErrors(String copyErrors) { this.copyErrors = "true".equalsIgnoreCase(copyErrors); @@ -183,7 +207,52 @@ private void copyStack(ActionInvocation invocation, CompoundRoot root) { if (proxyService.isProxy(action)) { editable = proxyService.ultimateTargetClass(action); } - reflectionProvider.copy(object, action, ctxMap, prepareExcludes(), includes, editable); + Collection copyExcludes = prepareExcludes(); + if (requireAnnotations) { + Class targetClass = editable != null ? editable : action.getClass(); + BeanInfo beanInfo = getTargetBeanInfo(targetClass); + if (beanInfo == null) { + // Fail closed: cannot prove which properties are annotated, so copy nothing. + LOG.warn("Chaining: unable to introspect target [{}]; skipping property copy " + + "(struts.chaining.requireAnnotations enabled)", targetClass.getName()); + continue; + } + copyExcludes = excludeUnauthorizedProperties(copyExcludes, beanInfo, targetClass, action); + } + reflectionProvider.copy(object, action, ctxMap, copyExcludes, includes, editable); + } + } + + /** + * Returns the excludes to use for the copy: the base excludes unioned with the names of all + * writable target properties that are not authorized by {@code @StrutsParameter}. + */ + private Collection excludeUnauthorizedProperties(Collection baseExcludes, + BeanInfo beanInfo, Class targetClass, Object action) { + Set merged = new HashSet<>(); + if (baseExcludes != null) { + merged.addAll(baseExcludes); + } + for (PropertyDescriptor descriptor : beanInfo.getPropertyDescriptors()) { + if (descriptor.getWriteMethod() == null) { + continue; + } + String name = descriptor.getName(); + if (!parameterAuthorizer.isAuthorized(name, action, action)) { + LOG.warn("Chaining: property [{}] not copied to [{}] because it is not annotated with @StrutsParameter", + name, targetClass.getName()); + merged.add(name); + } + } + return merged; + } + + private BeanInfo getTargetBeanInfo(Class targetClass) { + try { + return ognlUtil.getBeanInfo(targetClass); + } catch (IntrospectionException e) { + LOG.warn("Error introspecting Action {} for chaining @StrutsParameter enforcement", targetClass, e); + return null; } } From 2fcae6c77504bbc341d38499d4f3f91e20f5df49 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:24:09 +0200 Subject: [PATCH 06/10] WW-5631 refactor(chaining): align requireAnnotations parsing with BooleanUtils Use BooleanUtils.toBoolean for the chaining requireAnnotations flag so it accepts the same values (yes/on/1) as the sibling struts.parameters.requireAnnotations switch, and unify the enforcement WARN message prefix. Co-Authored-By: Claude Opus 4.7 --- .../org/apache/struts2/interceptor/ChainingInterceptor.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java index c9b8a87c8f..a10e8aaa3d 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.interceptor; +import org.apache.commons.lang3.BooleanUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.ActionInvocation; @@ -167,7 +168,7 @@ public void setOgnlUtil(OgnlUtil ognlUtil) { @Inject(value = StrutsConstants.STRUTS_CHAINING_REQUIRE_ANNOTATIONS, required = false) public void setRequireAnnotations(String requireAnnotations) { - this.requireAnnotations = "true".equalsIgnoreCase(requireAnnotations); + this.requireAnnotations = BooleanUtils.toBoolean(requireAnnotations); } @Inject(value = StrutsConstants.STRUTS_CHAINING_COPY_ERRORS, required = false) @@ -251,7 +252,7 @@ private BeanInfo getTargetBeanInfo(Class targetClass) { try { return ognlUtil.getBeanInfo(targetClass); } catch (IntrospectionException e) { - LOG.warn("Error introspecting Action {} for chaining @StrutsParameter enforcement", targetClass, e); + LOG.warn("Chaining: error introspecting target [{}] for @StrutsParameter enforcement", targetClass, e); return null; } } From 7d91295290cba8a4ac998b69ad2ab7f4df0e3a5a Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:25:35 +0200 Subject: [PATCH 07/10] WW-5631 test(chaining): cover includes interaction and proxied target Co-Authored-By: Claude Opus 4.7 --- .../interceptor/ChainingInterceptorTest.java | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java index e59780a93c..ab9305bcec 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java @@ -30,6 +30,8 @@ import org.apache.struts2.interceptor.parameter.StrutsParameterAuthorizer; import org.apache.struts2.ognl.OgnlUtil; import org.apache.struts2.util.ProxyService; +import org.mockito.ArgumentMatchers; +import org.mockito.Mockito; import java.util.*; @@ -240,6 +242,42 @@ public void testRequireAnnotationsFalseIsNoOp() throws Exception { target.getManagerApproved()); } + public void testEnforcementStillFiltersWithIncludesConfigured() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + UnannotatedChainingAction target = new UnannotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + interceptor.setIncludes("managerApproved"); + enableChainingEnforcement(true, false); + interceptor.intercept(invocation); + + assertFalse("unauthorized property must be excluded even when listed in includes", + target.getManagerApproved()); + } + + public void testEnforcementResolvesProxiedTargetClass() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + UnannotatedChainingAction target = new UnannotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + ProxyService proxyService = Mockito.mock(ProxyService.class); + Mockito.when(proxyService.isProxy(ArgumentMatchers.any())).thenReturn(true); + Mockito.when(proxyService.ultimateTargetClass(ArgumentMatchers.any())) + .thenReturn((Class) UnannotatedChainingAction.class); + interceptor.setProxyService(proxyService); + + enableChainingEnforcement(true, false); + interceptor.intercept(invocation); + + assertFalse("proxied unannotated target property must NOT be copied", target.getManagerApproved()); + } + @Override protected void setUp() throws Exception { super.setUp(); From 52995f88278169f10a64b4408c598c992b53e67e Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:26:17 +0200 Subject: [PATCH 08/10] WW-5631 docs(chaining): document struts.chaining.requireAnnotations Co-Authored-By: Claude Opus 4.7 --- .../org/apache/struts2/interceptor/ChainingInterceptor.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java index a10e8aaa3d..b294b47f61 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java @@ -75,6 +75,9 @@ *
  • struts.chaining.copyErrors - set to true to copy Action Errors
    • *
  • struts.chaining.copyFieldErrors - set to true to copy Field Errors
    • *
  • struts.chaining.copyMessages - set to true to copy Action Messages
    • + *
  • struts.chaining.requireAnnotations - set to true to only copy properties whose target + * Action member is annotated with {@code @StrutsParameter} (opt-in, default false). When the + * target cannot be introspected, no properties are copied (fail closed).
    • * * *

    From ab5b25f7f067c489d92087b9ecf0258bf0d4893e Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Wed, 27 May 2026 08:30:07 +0200 Subject: [PATCH 09/10] WW-5631 test(chaining): cover fail-closed introspection; clarify target==action Add a test asserting nothing is copied when the target action cannot be introspected (fail-closed), and document why isAuthorized is called with target == action for chaining (no ModelDriven exemption). Co-Authored-By: Claude Opus 4.7 --- .../interceptor/ChainingInterceptor.java | 2 ++ .../interceptor/ChainingInterceptorTest.java | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java index b294b47f61..34af10d228 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java @@ -242,6 +242,8 @@ private Collection excludeUnauthorizedProperties(Collection base continue; } String name = descriptor.getName(); + // target == action is deliberate: chaining copies onto the action object itself (not a + // ModelDriven model), so the authorizer's ModelDriven exemption must not apply here. if (!parameterAuthorizer.isAuthorized(name, action, action)) { LOG.warn("Chaining: property [{}] not copied to [{}] because it is not annotated with @StrutsParameter", name, targetClass.getName()); diff --git a/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java index ab9305bcec..1a35bd73b2 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ChainingInterceptorTest.java @@ -33,6 +33,7 @@ import org.mockito.ArgumentMatchers; import org.mockito.Mockito; +import java.beans.IntrospectionException; import java.util.*; /** @@ -278,6 +279,27 @@ public void testEnforcementResolvesProxiedTargetClass() throws Exception { assertFalse("proxied unannotated target property must NOT be copied", target.getManagerApproved()); } + public void testFailsClosedWhenTargetCannotBeIntrospected() throws Exception { + AnnotatedChainingAction source = new AnnotatedChainingAction(); + source.setManagerApproved(true); + AnnotatedChainingAction target = new AnnotatedChainingAction(); + mockInvocation.matchAndReturn("getAction", target); + stack.push(source); + stack.push(target); + + // Introspection failure must fail closed: copy nothing, even for an annotated property. + OgnlUtil ognlUtil = Mockito.mock(OgnlUtil.class); + Mockito.when(ognlUtil.getBeanInfo(ArgumentMatchers.any(Class.class))) + .thenThrow(new IntrospectionException("boom")); + interceptor.setOgnlUtil(ognlUtil); + + enableChainingEnforcement(true, false); + interceptor.intercept(invocation); + + assertFalse("nothing should be copied when the target cannot be introspected", + target.getManagerApproved()); + } + @Override protected void setUp() throws Exception { super.setUp(); From ed4036311dba08d223074dedff38c4ccb37a1a01 Mon Sep 17 00:00:00 2001 From: Lukasz Lenart Date: Fri, 29 May 2026 11:30:39 +0200 Subject: [PATCH 10/10] WW-5631 fix(chaining): address SonarCloud findings - Mark injected parameterAuthorizer/ognlUtil fields transient (S1948); they are re-injected by the container, not serialized. - Extract per-object copy into copyObjectToAction so the copyStack loop uses no break/continue (S135); fail-closed path now returns from the helper instead of continuing the loop. Co-Authored-By: Claude Opus 4.7 --- .../interceptor/ChainingInterceptor.java | 44 ++++++++++--------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java index 34af10d228..6508ed0472 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ChainingInterceptor.java @@ -146,8 +146,8 @@ public class ChainingInterceptor extends AbstractInterceptor { protected ReflectionProvider reflectionProvider; private ProxyService proxyService; private boolean requireAnnotations = false; - private ParameterAuthorizer parameterAuthorizer; - private OgnlUtil ognlUtil; + private transient ParameterAuthorizer parameterAuthorizer; + private transient OgnlUtil ognlUtil; @Inject public void setReflectionProvider(ReflectionProvider prov) { @@ -203,28 +203,30 @@ private void copyStack(ActionInvocation invocation, CompoundRoot root) { List list = prepareList(root); Map ctxMap = invocation.getInvocationContext().getContextMap(); for (Object object : list) { - if (!shouldCopy(object)) { - continue; - } - Object action = invocation.getAction(); - Class editable = null; - if (proxyService.isProxy(action)) { - editable = proxyService.ultimateTargetClass(action); + if (shouldCopy(object)) { + copyObjectToAction(object, invocation.getAction(), ctxMap); } - Collection copyExcludes = prepareExcludes(); - if (requireAnnotations) { - Class targetClass = editable != null ? editable : action.getClass(); - BeanInfo beanInfo = getTargetBeanInfo(targetClass); - if (beanInfo == null) { - // Fail closed: cannot prove which properties are annotated, so copy nothing. - LOG.warn("Chaining: unable to introspect target [{}]; skipping property copy " + - "(struts.chaining.requireAnnotations enabled)", targetClass.getName()); - continue; - } - copyExcludes = excludeUnauthorizedProperties(copyExcludes, beanInfo, targetClass, action); + } + } + + private void copyObjectToAction(Object object, Object action, Map ctxMap) { + Class editable = null; + if (proxyService.isProxy(action)) { + editable = proxyService.ultimateTargetClass(action); + } + Collection copyExcludes = prepareExcludes(); + if (requireAnnotations) { + Class targetClass = editable != null ? editable : action.getClass(); + BeanInfo beanInfo = getTargetBeanInfo(targetClass); + if (beanInfo == null) { + // Fail closed: cannot prove which properties are annotated, so copy nothing. + LOG.warn("Chaining: unable to introspect target [{}]; skipping property copy " + + "(struts.chaining.requireAnnotations enabled)", targetClass.getName()); + return; } - reflectionProvider.copy(object, action, ctxMap, copyExcludes, includes, editable); + copyExcludes = excludeUnauthorizedProperties(copyExcludes, beanInfo, targetClass, action); } + reflectionProvider.copy(object, action, ctxMap, copyExcludes, includes, editable); } /**