You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2026/07/27 10:19:01 UTC
(struts) branch WW-5659-lazy-params-request-scoping created (now ac4676db5)
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a change to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
at ac4676db5 WW-5659 chore(core): harden the policy sets and tidy the lazy params dispatch
This branch includes the following new commits:
new 08c9ac40c WW-5659 docs: design for request-scoped lazy interceptor params
new e51a5ce36 WW-5659 docs: implementation plan for request-scoped lazy params
new 34ffa9a67 WW-5659 docs: clarify test base class constraint in the plan
new c35483c65 WW-5659 feat(core): add InterceptorParams contract and DisableParams holder
new 687963aba WW-5659 feat(core): resolve lazy params into a holder instead of the interceptor
new 31dc7f667 WW-5659 docs(core): correct isUnresolved javadoc and pin empty-value fail-closed behavior
new af6e5fb81 WW-5659 refactor(core): hold upload policy in one value object
new 84aca8cd5 WW-5659 fix(core): resolve lazy interceptor params per invocation
new 73c7ec350 WW-5659 test(core): cover both lazy-params skip branches and per-invocation disabled
new 9d66a7b9f WW-5659 fix(core): reject uploads when the policy cannot be resolved
new 83c52c9f0 WW-5659 test(core): exercise real lazy param resolution in dynamic upload tests
new 0cafec4d2 WW-5659 fix(core): mark params unusable when a lazy value cannot be applied
new ac4676db5 WW-5659 chore(core): harden the policy sets and tidy the lazy params dispatch
The 13 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(struts) 03/13: WW-5659 docs: clarify test base class constraint in the plan
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 34ffa9a67471b39fb28754c57b625a17066f49b6
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:16:47 2026 +0200
WW-5659 docs: clarify test base class constraint in the plan
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md b/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md
index f1af340e8..8391167ae 100644
--- a/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md
+++ b/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md
@@ -14,7 +14,7 @@
- **Ticket prefix:** every commit message starts with `WW-5659`.
- **Target version:** 7.3.0. This is a deliberate public API break; no backport to 7.2.x.
-- **Test style:** `core` tests are JUnit 3 style — extend `StrutsInternalTestCase` (which extends `XWorkTestCase`, which extends `junit.framework.TestCase`), use `protected void setUp()`, and name tests `public void testXxx()`. **A JUnit 5 `@Test` annotation compiles and silently never runs.** Never add one.
+- **Test style:** `core` tests are JUnit 3 style — `protected void setUp()`, tests named `public void testXxx()`, no annotations. **A JUnit 5 `@Test` annotation compiles and silently never runs.** Never add one. Extend `StrutsInternalTestCase` (which extends `XWorkTestCase`, which extends `junit.framework.TestCase`) when the test needs the Struts container or `ActionContext`; extend `junit.framework.TestCase` directly for pure value-object tests that need neither.
- **Assertions:** use AssertJ (`import static org.assertj.core.api.Assertions.assertThat;`), matching the existing `ActionFileUploadInterceptorTest`.
- **Package:** all new production types go in `org.apache.struts2.interceptor`.
- **License header:** every new `.java` file starts with the ASF header, copied verbatim from `core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java:1-18`.
(struts) 13/13: WW-5659 chore(core): harden the policy sets and tidy the lazy params dispatch
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit ac4676db5daf7f453f993331b90dcc3e8c88846b
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 11:59:41 2026 +0200
WW-5659 chore(core): harden the policy sets and tidy the lazy params dispatch
UploadPolicy handed out the mutable HashSet built by commaDelimitedStringToSet,
which the copy constructor shares by reference with the configured policy, so a
subclass overriding the protected acceptFile could have rewritten process-wide
config from a request thread. The sets are unmodifiable now.
Also document that unresolved() records `disabled` like any other param, so an
unresolvable disabled expression leaves the interceptor enabled and rejects
every upload; and in DefaultActionInvocation use normal imports for the params
types, make the interceptor local final, word the three skip logs consistently
and identify the interceptor by its mapping name, and note that the name-based
param merge is inherited behaviour whose duplicate-ref handling is questionable.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../apache/struts2/DefaultActionInvocation.java | 38 +++++++++++++++++-----
.../apache/struts2/interceptor/UploadPolicy.java | 14 +++++++-
2 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java
index 913590410..72a35afe5 100644
--- a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java
+++ b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java
@@ -30,7 +30,9 @@ import org.apache.struts2.config.entities.ResultConfig;
import org.apache.struts2.inject.Container;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.interceptor.ConditionalInterceptor;
+import org.apache.struts2.interceptor.DisableParams;
import org.apache.struts2.interceptor.Interceptor;
+import org.apache.struts2.interceptor.InterceptorParams;
import org.apache.struts2.interceptor.PreResultListener;
import org.apache.struts2.interceptor.WithLazyParams;
import org.apache.struts2.ognl.OgnlUtil;
@@ -258,11 +260,11 @@ public class DefaultActionInvocation implements ActionInvocation {
if (asyncManager == null || !asyncManager.hasAsyncActionResult()) {
if (interceptors.hasNext()) {
final InterceptorMapping interceptorMapping = interceptors.next();
- Interceptor interceptor = interceptorMapping.getInterceptor();
+ final Interceptor interceptor = interceptorMapping.getInterceptor();
if (interceptor instanceof WithLazyParams<?> lazyInterceptor) {
resultCode = invokeWithLazyParams(lazyInterceptor, interceptorMapping);
} else if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) {
- resultCode = executeConditional(conditionalInterceptor);
+ resultCode = executeConditional(conditionalInterceptor, interceptorMapping.getName());
} else {
LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName());
resultCode = interceptor.intercept(this);
@@ -313,18 +315,18 @@ public class DefaultActionInvocation implements ActionInvocation {
* lazily resolved {@code disabled} flag and any custom {@code shouldIntercept} must be honoured
* here, because the single-argument {@code intercept} is not the entry point on this path.
*/
- private <P extends org.apache.struts2.interceptor.InterceptorParams> String invokeWithLazyParams(
+ private <P extends InterceptorParams> String invokeWithLazyParams(
WithLazyParams<P> lazyInterceptor, InterceptorMapping interceptorMapping) throws Exception {
P lazyParams = lazyParamInjector.resolveInto(
lazyInterceptor.newLazyParams(), mergedParams(interceptorMapping), invocationContext);
- if (lazyParams instanceof org.apache.struts2.interceptor.DisableParams disableParams && disableParams.isDisabled()) {
- LOG.debug("Interceptor: {} is disabled for this invocation, skipping to next", interceptorMapping.getName());
+ if (lazyParams instanceof DisableParams disableParams && disableParams.isDisabled()) {
+ LOG.debug("Interceptor: {} is disabled by its lazily resolved params, skipping to next", interceptorMapping.getName());
return this.invoke();
}
if (lazyInterceptor instanceof ConditionalInterceptor conditionalInterceptor
&& !conditionalInterceptor.shouldIntercept(this)) {
- LOG.debug("Interceptor: {} is disabled, skipping to next", interceptorMapping.getName());
+ LOG.debug("Interceptor: {} declined by shouldIntercept() on the lazy params path, skipping to next", interceptorMapping.getName());
return this.invoke();
}
LOG.debug("Executing lazy params interceptor: {}", interceptorMapping.getName());
@@ -332,6 +334,13 @@ public class DefaultActionInvocation implements ActionInvocation {
}
/**
+ * Merges the params declared on the interceptor-ref with those of the mapping being invoked.
+ * <p>
+ * The name-based lookup is inherited behaviour, kept as-is: the mapping is normally the very one
+ * found by name, so the merge is a no-op, and when a stack references the same interceptor name
+ * twice with different params it merges the first mapping's params over the current one, which
+ * is questionable. Changing it is out of scope here.
+ *
* @return a fresh map; the mapping's own param map is shared across requests and must not be mutated
*/
private Map<String, String> mergedParams(InterceptorMapping interceptorMapping) {
@@ -343,12 +352,25 @@ public class DefaultActionInvocation implements ActionInvocation {
return merged;
}
+ /**
+ * @deprecated since 7.3.0, use {@link #executeConditional(ConditionalInterceptor, String)} so the
+ * interceptor is identified by its mapping name in the logs.
+ */
+ @Deprecated
protected String executeConditional(ConditionalInterceptor conditionalInterceptor) throws Exception {
+ return executeConditional(conditionalInterceptor, conditionalInterceptor.getClass().getSimpleName());
+ }
+
+ /**
+ * @param interceptorName the name of the interceptor mapping being invoked, used for logging
+ * @since 7.3.0
+ */
+ protected String executeConditional(ConditionalInterceptor conditionalInterceptor, String interceptorName) throws Exception {
if (conditionalInterceptor.shouldIntercept(this)) {
- LOG.debug("Executing conditional interceptor: {}", conditionalInterceptor.getClass().getSimpleName());
+ LOG.debug("Executing conditional interceptor: {}", interceptorName);
return conditionalInterceptor.intercept(this);
} else {
- LOG.debug("Interceptor: {} is disabled, skipping to next", conditionalInterceptor.getClass().getSimpleName());
+ LOG.debug("Interceptor: {} declined by shouldIntercept(), skipping to next", interceptorName);
return this.invoke();
}
}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java b/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
index 08702dee4..5911a401e 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
@@ -87,6 +87,13 @@ public class UploadPolicy extends DisableParams {
* A parameter that could not be resolved makes this policy unusable: the upload is rejected
* rather than validated against a partially-resolved policy, so a broken expression cannot
* silently relax validation.
+ * <p>
+ * <strong>Any</strong> param name is recorded, {@code disabled} included. An unresolvable
+ * {@code <param name="disabled">${...}</param>} therefore leaves the interceptor enabled (the
+ * flag keeps its configured value) <em>and</em> marks this policy unusable, so every upload in
+ * that invocation is rejected. That is deliberate — a broken dispatch flag is a broken
+ * configuration, and refusing the upload is the safe reading — but it means a typo in
+ * {@code disabled} takes out upload validation rather than only the disabling.
*/
@Override
public void unresolved(String paramName) {
@@ -108,9 +115,14 @@ public class UploadPolicy extends DisableParams {
return new UploadPolicy(this);
}
+ /**
+ * The resulting set is unmodifiable: the copy constructor shares it by reference with the
+ * configured policy, so every per-invocation policy that does not override the param would
+ * otherwise hand out a live handle on process-wide configuration.
+ */
private static Set<String> toSet(String commaDelimited) {
return commaDelimited == null
? Collections.emptySet()
- : TextParseUtil.commaDelimitedStringToSet(commaDelimited);
+ : Collections.unmodifiableSet(TextParseUtil.commaDelimitedStringToSet(commaDelimited));
}
}
(struts) 08/13: WW-5659 fix(core): resolve lazy interceptor params per invocation
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 84aca8cd5e76e137cb125bebb07f8dc7b0575c9b
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:56:03 2026 +0200
WW-5659 fix(core): resolve lazy interceptor params per invocation
Co-Authored-By: deprrous <su...@gmail.com>
---
.../apache/struts2/DefaultActionInvocation.java | 53 ++++--
.../interceptor/ActionFileUploadInterceptor.java | 14 +-
.../apache/struts2/interceptor/WithLazyParams.java | 25 ++-
.../ActionFileUploadInterceptorTest.java | 186 ++++++++++++++++++++-
.../apache/struts2/mock/MockLazyInterceptor.java | 48 +++++-
5 files changed, 291 insertions(+), 35 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java
index 6885dd50d..913590410 100644
--- a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java
+++ b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java
@@ -40,6 +40,7 @@ import org.apache.struts2.util.ValueStack;
import org.apache.struts2.util.ValueStackFactory;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
@@ -258,17 +259,9 @@ public class DefaultActionInvocation implements ActionInvocation {
if (interceptors.hasNext()) {
final InterceptorMapping interceptorMapping = interceptors.next();
Interceptor interceptor = interceptorMapping.getInterceptor();
- if (interceptor instanceof WithLazyParams) {
- Map<String, String> params = interceptorMapping.getParams();
-
- proxy.getConfig().getInterceptors().stream()
- .filter(im -> im.getName().equals(interceptorMapping.getName()))
- .findFirst()
- .ifPresent(im -> params.putAll(im.getParams()));
-
- interceptor = lazyParamInjector.injectParams(interceptor, params, invocationContext);
- }
- if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) {
+ if (interceptor instanceof WithLazyParams<?> lazyInterceptor) {
+ resultCode = invokeWithLazyParams(lazyInterceptor, interceptorMapping);
+ } else if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) {
resultCode = executeConditional(conditionalInterceptor);
} else {
LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName());
@@ -312,6 +305,44 @@ public class DefaultActionInvocation implements ActionInvocation {
return resultCode;
}
+ /**
+ * Resolves lazy params into a per-invocation holder and dispatches to the interceptor.
+ * <p>
+ * {@link org.apache.struts2.interceptor.AbstractInterceptor} implements
+ * {@link ConditionalInterceptor}, so a lazy interceptor is normally conditional too; both the
+ * lazily resolved {@code disabled} flag and any custom {@code shouldIntercept} must be honoured
+ * here, because the single-argument {@code intercept} is not the entry point on this path.
+ */
+ private <P extends org.apache.struts2.interceptor.InterceptorParams> String invokeWithLazyParams(
+ WithLazyParams<P> lazyInterceptor, InterceptorMapping interceptorMapping) throws Exception {
+ P lazyParams = lazyParamInjector.resolveInto(
+ lazyInterceptor.newLazyParams(), mergedParams(interceptorMapping), invocationContext);
+
+ if (lazyParams instanceof org.apache.struts2.interceptor.DisableParams disableParams && disableParams.isDisabled()) {
+ LOG.debug("Interceptor: {} is disabled for this invocation, skipping to next", interceptorMapping.getName());
+ return this.invoke();
+ }
+ if (lazyInterceptor instanceof ConditionalInterceptor conditionalInterceptor
+ && !conditionalInterceptor.shouldIntercept(this)) {
+ LOG.debug("Interceptor: {} is disabled, skipping to next", interceptorMapping.getName());
+ return this.invoke();
+ }
+ LOG.debug("Executing lazy params interceptor: {}", interceptorMapping.getName());
+ return lazyInterceptor.intercept(this, lazyParams);
+ }
+
+ /**
+ * @return a fresh map; the mapping's own param map is shared across requests and must not be mutated
+ */
+ private Map<String, String> mergedParams(InterceptorMapping interceptorMapping) {
+ Map<String, String> merged = new HashMap<>(interceptorMapping.getParams());
+ proxy.getConfig().getInterceptors().stream()
+ .filter(im -> im.getName().equals(interceptorMapping.getName()))
+ .findFirst()
+ .ifPresent(im -> merged.putAll(im.getParams()));
+ return merged;
+ }
+
protected String executeConditional(ConditionalInterceptor conditionalInterceptor) throws Exception {
if (conditionalInterceptor.shouldIntercept(this)) {
LOG.debug("Executing conditional interceptor: {}", conditionalInterceptor.getClass().getSimpleName());
diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
index 4aea0f782..82ff798e3 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
@@ -202,12 +202,22 @@ import java.util.List;
* @see UploadedFilesAware
* @see AbstractFileUploadInterceptor
*/
-public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor implements WithLazyParams {
+public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor implements WithLazyParams<UploadPolicy> {
protected static final Logger LOG = LogManager.getLogger(ActionFileUploadInterceptor.class);
+ @Override
+ public UploadPolicy newLazyParams() {
+ return copyConfiguredPolicy();
+ }
+
@Override
public String intercept(ActionInvocation invocation) throws Exception {
+ return intercept(invocation, newLazyParams());
+ }
+
+ @Override
+ public String intercept(ActionInvocation invocation, UploadPolicy policy) throws Exception {
HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper
? findMultipartRequestWrapper(wrapper)
@@ -228,8 +238,6 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor i
return invocation.invoke();
}
- UploadPolicy policy = copyConfiguredPolicy();
-
applyValidation(action, multiWrapper);
// bind allowed Files
diff --git a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
index f683ebb7c..85d8e6d71 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
@@ -21,6 +21,7 @@ package org.apache.struts2.interceptor;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.ActionContext;
+import org.apache.struts2.ActionInvocation;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.ognl.OgnlUtil;
import org.apache.struts2.util.TextParseUtil;
@@ -48,7 +49,21 @@ import java.util.Map;
*
* @since 2.5.9
*/
-public interface WithLazyParams {
+public interface WithLazyParams<P extends InterceptorParams> {
+
+ /**
+ * @return a fresh holder for one invocation, seeded from the configured values
+ * @since 7.3.0
+ */
+ P newLazyParams();
+
+ /**
+ * Invoked in place of {@link Interceptor#intercept(ActionInvocation)} when lazy params apply.
+ *
+ * @param lazyParams params resolved for this invocation only
+ * @since 7.3.0
+ */
+ String intercept(ActionInvocation invocation, P lazyParams) throws Exception;
class LazyParamInjector {
@@ -80,14 +95,6 @@ public interface WithLazyParams {
this.ognlUtil = ognlUtil;
}
- public Interceptor injectParams(Interceptor interceptor, Map<String, String> params, ActionContext invocationContext) {
- for (Map.Entry<String, String> entry : params.entrySet()) {
- Object paramValue = textParser.evaluate(new char[]{'$'}, entry.getValue(), valueEvaluator, TextParser.DEFAULT_LOOP_COUNT);
- ognlUtil.setProperty(entry.getKey(), paramValue, interceptor, invocationContext.getContextMap());
- }
- return interceptor;
- }
-
/**
* Resolves configured params into a per-invocation holder, leaving the interceptor untouched.
* <p>
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index c45091d77..c2cf40719 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -34,6 +34,8 @@ import org.apache.struts2.locale.DefaultLocaleProvider;
import org.apache.struts2.mock.MockActionInvocation;
import org.apache.struts2.mock.MockActionProxy;
import org.apache.struts2.util.ClassLoaderUtil;
+import org.apache.struts2.util.ValueStack;
+import org.apache.struts2.util.ValueStackFactory;
import org.assertj.core.util.Files;
import org.springframework.mock.web.MockHttpServletRequest;
@@ -41,8 +43,16 @@ import java.io.File;
import java.net.URI;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
import java.util.List;
import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import static org.assertj.core.api.Assertions.assertThat;
@@ -536,14 +546,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
}
private MultiPartRequestWrapper createMultipartRequest(int maxsize, int maxfilesize, int maxfiles, int maxStringLength) {
- JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
- jak.setMaxSize(String.valueOf(maxsize));
- jak.setMaxFileSize(String.valueOf(maxfilesize));
- jak.setMaxFiles(String.valueOf(maxfiles));
- jak.setMaxStringLength(String.valueOf(maxStringLength));
- jak.setDefaultEncoding(StandardCharsets.UTF_8.name());
-
- return new MultiPartRequestWrapper(jak, request, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
+ return createMultipartRequest(request, maxsize, maxfilesize, maxfiles, maxStringLength);
}
protected void setUp() throws Exception {
@@ -955,4 +958,171 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
assertThat(policy.getAllowedExtensions()).isEmpty();
}
+ /**
+ * Regression for WW-5659: two concurrent invocations resolving different policies must not
+ * see each other's values. Scenario contributed by @deprrous in GitHub PR #1815.
+ */
+ public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Exception {
+ CoordinatedActionFileUploadInterceptor sharedInterceptor = new CoordinatedActionFileUploadInterceptor();
+ container.inject(sharedInterceptor);
+
+ MyDynamicFileUploadAction plainPolicyAction = new MyDynamicFileUploadAction();
+ plainPolicyAction.setAllowedMimeTypes("text/plain");
+ container.inject(plainPolicyAction);
+
+ MyDynamicFileUploadAction htmlPolicyAction = new MyDynamicFileUploadAction();
+ htmlPolicyAction.setAllowedMimeTypes("text/html");
+ container.inject(htmlPolicyAction);
+
+ ExecutorService executor = Executors.newFixedThreadPool(2);
+ try {
+ Future<String> plainResult = executor.submit(() -> runUploadAttempt(
+ sharedInterceptor, plainPolicyAction, createUploadRequest("plain-policy.html", "text/html", htmlContent)));
+
+ assertThat(sharedInterceptor.awaitFirstValidation()).isTrue();
+
+ Future<String> htmlResult = executor.submit(() -> runUploadAttempt(
+ sharedInterceptor, htmlPolicyAction, createUploadRequest("html-policy.html", "text/html", htmlContent)));
+
+ assertThat(htmlResult.get(10, TimeUnit.SECONDS)).isEqualTo("success");
+ sharedInterceptor.releaseFirstValidation();
+ assertThat(plainResult.get(10, TimeUnit.SECONDS)).isEqualTo("success");
+ } finally {
+ sharedInterceptor.releaseFirstValidation();
+ executor.shutdownNow();
+ sharedInterceptor.destroy();
+ }
+
+ // the text/plain policy must have rejected the text/html upload despite the concurrent
+ // text/html invocation resolving a more permissive policy on the same interceptor
+ assertThat(plainPolicyAction.getUploadFiles()).isNull();
+ assertThat(plainPolicyAction.getFieldErrors()).containsKey("file");
+
+ assertThat(htmlPolicyAction.hasFieldErrors()).isFalse();
+ assertThat(htmlPolicyAction.getUploadFiles()).isNotNull().hasSize(1);
+ assertThat(htmlPolicyAction.getUploadFiles().get(0).getOriginalName()).isEqualTo("html-policy.html");
+ }
+
+ /**
+ * Regression for WW-5659: resolving an invocation's params must leave the interceptor
+ * singleton exactly as configured.
+ */
+ public void testResolutionDoesNotMutateTheInterceptor() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+ interceptor.setAllowedTypes("text/plain");
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes("text/html");
+ container.inject(action);
+
+ runUploadAttempt(interceptor, action, createUploadRequest("f.html", "text/html", htmlContent));
+
+ assertThat(interceptor.newLazyParams().getAllowedTypes()).containsExactly("text/plain");
+ }
+
+ /**
+ * Regression for WW-5659: a lazily resolved {@code disabled} must apply to one invocation only.
+ */
+ public void testDisabledIsResolvedPerInvocation() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes("text/plain");
+ container.inject(action);
+
+ UploadPolicy policy = interceptor.newLazyParams();
+ policy.setDisabled("true");
+
+ assertThat(policy.isDisabled()).isTrue();
+ assertThat(interceptor.newLazyParams().isDisabled()).isFalse();
+ }
+
+ private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInterceptor,
+ MyDynamicFileUploadAction action,
+ MockHttpServletRequest uploadRequest) throws Exception {
+ MultiPartRequestWrapper multiPartRequest = createMultipartRequest(uploadRequest, -1, -1, 3, -1);
+ ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack();
+ valueStack.push(action);
+
+ ActionContext context = ActionContext.of(valueStack.getContext())
+ .withContainer(container)
+ .withValueStack(valueStack)
+ .withServletRequest(multiPartRequest)
+ .bind();
+ try {
+ MockActionInvocation invocation = new MockActionInvocation();
+ invocation.setAction(action);
+ invocation.setResultCode("success");
+ invocation.setInvocationContext(context);
+
+ Map<String, String> params = new HashMap<>();
+ params.put("allowedTypes", "${allowedMimeTypes}");
+
+ WithLazyParams.LazyParamInjector injector = new WithLazyParams.LazyParamInjector(valueStack);
+ container.inject(injector);
+ UploadPolicy policy = injector.resolveInto(actionFileUploadInterceptor.newLazyParams(), params, context);
+
+ return actionFileUploadInterceptor.intercept(invocation, policy);
+ } finally {
+ ActionContext.clear();
+ }
+ }
+
+ private MockHttpServletRequest createUploadRequest(String filename, String contentType, String content) {
+ MockHttpServletRequest uploadRequest = new MockHttpServletRequest();
+ uploadRequest.setCharacterEncoding(StandardCharsets.UTF_8.name());
+ uploadRequest.setMethod("POST");
+ uploadRequest.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\"");
+ uploadRequest.setContent((encodeTextFile(filename, contentType, content) + endLine + "--" + boundary + "--")
+ .getBytes(StandardCharsets.UTF_8));
+ return uploadRequest;
+ }
+
+ private MultiPartRequestWrapper createMultipartRequest(MockHttpServletRequest multipartRequest, int maxsize, int maxfilesize, int maxfiles, int maxStringLength) {
+ JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
+ jak.setMaxSize(String.valueOf(maxsize));
+ jak.setMaxFileSize(String.valueOf(maxfilesize));
+ jak.setMaxFiles(String.valueOf(maxfiles));
+ jak.setMaxStringLength(String.valueOf(maxStringLength));
+ jak.setDefaultEncoding(StandardCharsets.UTF_8.name());
+ return new MultiPartRequestWrapper(jak, multipartRequest, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
+ }
+
+ /** Pauses the first validation so a second invocation can overlap it. From PR #1815 by @deprrous. */
+ private static final class CoordinatedActionFileUploadInterceptor extends ActionFileUploadInterceptor {
+ private final AtomicBoolean pauseFirstValidation = new AtomicBoolean(true);
+ private final CountDownLatch firstValidationEntered = new CountDownLatch(1);
+ private final CountDownLatch allowFirstValidationToContinue = new CountDownLatch(1);
+
+ @Override
+ protected boolean acceptFile(UploadPolicy policy, Object action, UploadedFile file, String originalFilename, String contentType, String inputName) {
+ if (pauseFirstValidation.compareAndSet(true, false)) {
+ firstValidationEntered.countDown();
+ awaitUnchecked(allowFirstValidationToContinue);
+ }
+ return super.acceptFile(policy, action, file, originalFilename, contentType, inputName);
+ }
+
+ private boolean awaitFirstValidation() throws InterruptedException {
+ return firstValidationEntered.await(10, TimeUnit.SECONDS);
+ }
+
+ private void releaseFirstValidation() {
+ allowFirstValidationToContinue.countDown();
+ }
+
+ private void awaitUnchecked(CountDownLatch latch) {
+ try {
+ if (!latch.await(10, TimeUnit.SECONDS)) {
+ throw new AssertionError("Timed out waiting for concurrent validation release");
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new AssertionError("Interrupted while waiting for concurrent validation release", e);
+ }
+ }
+ }
+
}
diff --git a/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java b/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java
index ae3aea3cc..75d3d021a 100644
--- a/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java
+++ b/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java
@@ -21,9 +21,35 @@ package org.apache.struts2.mock;
import org.apache.struts2.ActionInvocation;
import org.apache.struts2.SimpleAction;
import org.apache.struts2.interceptor.AbstractInterceptor;
+import org.apache.struts2.interceptor.InterceptorParams;
import org.apache.struts2.interceptor.WithLazyParams;
-public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams {
+public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams<MockLazyInterceptor.MockLazyParams> {
+
+ /**
+ * Per-invocation holder, seeded from the configured values.
+ */
+ public static class MockLazyParams implements InterceptorParams {
+
+ private String foo = "";
+ private String bar = "";
+
+ public void setFoo(String foo) {
+ this.foo = foo;
+ }
+
+ public String getFoo() {
+ return foo;
+ }
+
+ public void setBar(String bar) {
+ this.bar = bar;
+ }
+
+ public String getBar() {
+ return bar;
+ }
+ }
private String foo = "";
private String bar = "";
@@ -44,12 +70,26 @@ public class MockLazyInterceptor extends AbstractInterceptor implements WithLazy
return bar;
}
+ @Override
+ public MockLazyParams newLazyParams() {
+ MockLazyParams params = new MockLazyParams();
+ params.setFoo(foo);
+ params.setBar(bar);
+ return params;
+ }
+
+ @Override
public String intercept(ActionInvocation invocation) throws Exception {
+ return intercept(invocation, newLazyParams());
+ }
+
+ @Override
+ public String intercept(ActionInvocation invocation, MockLazyParams lazyParams) throws Exception {
if (invocation.getAction() instanceof SimpleAction) {
- ((SimpleAction) invocation.getAction()).setName(foo);
+ ((SimpleAction) invocation.getAction()).setName(lazyParams.getFoo());
// Only set blah if bar is configured (not empty)
- if (bar != null && !bar.isEmpty()) {
- ((SimpleAction) invocation.getAction()).setBlah(bar);
+ if (lazyParams.getBar() != null && !lazyParams.getBar().isEmpty()) {
+ ((SimpleAction) invocation.getAction()).setBlah(lazyParams.getBar());
}
}
return invocation.invoke();
(struts) 06/13: WW-5659 docs(core): correct isUnresolved javadoc and pin empty-value fail-closed behavior
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 31dc7f667aece368c0f9f8c03a81de8b3fcaa22b
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:33:34 2026 +0200
WW-5659 docs(core): correct isUnresolved javadoc and pin empty-value fail-closed behavior
isUnresolved cannot distinguish a failed ${...} resolution from an expression that
legitimately evaluates to an empty string; the parser gives no other signal. The
previous javadoc wrongly claimed the raw template let it tell the two apart. Fix
the javadoc to state the actual, intentional rule (fail-closed: treat both as
unusable), and add a test pinning that a legitimately-empty expression is treated
as unresolved rather than written.
---
.../apache/struts2/interceptor/WithLazyParams.java | 21 +++++++++++++++------
.../struts2/interceptor/LazyParamInjectorTest.java | 14 ++++++++++++++
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
index 8c74b398d..f683ebb7c 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
@@ -91,9 +91,13 @@ public interface WithLazyParams {
/**
* Resolves configured params into a per-invocation holder, leaving the interceptor untouched.
* <p>
- * A {@code ${...}} expression that cannot be resolved is not written: the holder keeps its
- * seeded configuration value and is notified via {@link InterceptorParams#unresolved(String)},
- * so a broken expression cannot silently relax a validation policy.
+ * A {@code ${...}} expression that resolves to null or an empty value is not written: the
+ * holder keeps its seeded configuration value and is notified via
+ * {@link InterceptorParams#unresolved(String)}. This also catches an expression that
+ * legitimately evaluates to an empty string, which is indistinguishable from a failed
+ * resolution (see {@link #isUnresolved}); for a fail-closed policy such as an allowlist,
+ * treating both as unusable is the safe reading, so a broken expression cannot silently
+ * relax a validation policy.
*
* @since 7.3.0
*/
@@ -122,9 +126,14 @@ public interface WithLazyParams {
}
/**
- * {@link org.apache.struts2.util.OgnlTextParser} yields an empty string for an expression that
- * does not resolve and gives no other signal, so the raw template is needed to tell that apart
- * from a legitimately empty value.
+ * A {@code ${...}} param is treated as unresolved when its evaluated value is null or empty.
+ * <p>
+ * {@link org.apache.struts2.util.OgnlTextParser} yields the same empty string both when an
+ * expression fails to resolve and when it resolves to a legitimately empty value — there is
+ * no way to tell the two apart from the parser's output alone. This method does not attempt
+ * to; a param that legitimately evaluates to an empty string is therefore also reported as
+ * unresolved. That is a deliberate fail-closed choice: for a security-sensitive param (e.g.
+ * an allowlist), silently accepting an unintended empty value is worse than refusing it.
*/
private boolean isUnresolved(String rawValue, Object paramValue) {
return rawValue != null
diff --git a/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java b/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
index 8939c1313..46f5282f0 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
@@ -50,6 +50,7 @@ public class LazyParamInjectorTest extends StrutsInternalTestCase {
public static class Bean {
public String getLabel() { return "resolved-label"; }
public Long getLimit() { return 4096L; }
+ public String getBlank() { return ""; }
}
private ActionContext context;
@@ -113,6 +114,19 @@ public class LazyParamInjectorTest extends StrutsInternalTestCase {
assertThat(holder.getUnresolvedCalls()).containsExactly("name");
}
+ public void testExpressionResolvingToEmptyIsTreatedAsUnresolved() {
+ Holder seeded = new Holder();
+ seeded.setName("seeded-value");
+
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "${blank}");
+
+ Holder holder = injector.resolveInto(seeded, params, context);
+
+ assertThat(holder.getName()).isEqualTo("seeded-value");
+ assertThat(holder.getUnresolvedCalls()).containsExactly("name");
+ }
+
public void testResolvesDisabledOntoDisableParams() {
Map<String, String> params = new HashMap<>();
params.put("disabled", "true");
(struts) 09/13: WW-5659 test(core): cover both lazy-params skip branches and per-invocation disabled
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 73c7ec350e3cda786c541fc71d3c0e4ac6c8b24d
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 11:23:16 2026 +0200
WW-5659 test(core): cover both lazy-params skip branches and per-invocation disabled
The two skip branches in DefaultActionInvocation#invokeWithLazyParams had no
coverage: deleting either left the whole suite green. The only tests reaching
that method used LazyFoo/LazyFooWithStackParams, which declare no disabled
param, and MockLazyParams did not extend DisableParams, so the holder branch
was unreachable and shouldIntercept was always true.
Make MockLazyParams extend DisableParams and add two action configs that
isolate one branch each:
- LazyFooLazilyDisabled passes disabled as an interceptor-ref param, so it
reaches InterceptorMapping#getParams(), resolves onto the holder, and
exercises the DisableParams branch.
- LazyFooStaticallyDisabled sets disabled on the interceptor definition
instead. InterceptorBuilder only puts interceptor-ref params into the
mapping, so the holder never sees it and it can only be honoured through
ConditionalInterceptor#shouldIntercept.
Verified by deleting each branch in turn: each deletion fails exactly the one
test that targets it, and no other.
Also make testDisabledIsResolvedPerInvocation earn its name. It previously
asserted only that newLazyParams() returns a fresh object, never resolving
anything, and built a MyDynamicFileUploadAction it never used. It now routes
two actions through a real LazyParamInjector#resolveInto of a
disabled=${uploadDisabled} param and pins that one invocation's resolved flag
survives the other's, and that neither reaches the interceptor singleton.
---
.../struts2/DefaultActionInvocationTest.java | 53 ++++++++++++++++++++++
.../ActionFileUploadInterceptorTest.java | 47 ++++++++++++++++---
.../apache/struts2/mock/MockLazyInterceptor.java | 7 +--
core/src/test/resources/xwork-sample.xml | 21 +++++++++
core/src/test/resources/xwork-test-default.xml | 5 ++
5 files changed, 124 insertions(+), 9 deletions(-)
diff --git a/core/src/test/java/org/apache/struts2/DefaultActionInvocationTest.java b/core/src/test/java/org/apache/struts2/DefaultActionInvocationTest.java
index 6a8e9fc21..398fa7f77 100644
--- a/core/src/test/java/org/apache/struts2/DefaultActionInvocationTest.java
+++ b/core/src/test/java/org/apache/struts2/DefaultActionInvocationTest.java
@@ -42,6 +42,7 @@ import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
import static org.apache.struts2.ognl.OgnlUtilTest.createOgnlUtil;
+import static org.assertj.core.api.Assertions.assertThat;
/**
@@ -423,6 +424,58 @@ public class DefaultActionInvocationTest extends XWorkTestCase {
assertEquals("static value", action.getBlah());
}
+ /**
+ * Regression for WW-5659: a {@code disabled} param resolved lazily from the value stack must skip
+ * the interceptor for that invocation. It arrives through the interceptor mapping's params, so it
+ * lands on the per-invocation holder and is honoured there, never on the shared interceptor.
+ */
+ public void testInvokeWithLazyParamsSkipsLazilyDisabledInterceptor() throws Exception {
+ HashMap<String, Object> params = new HashMap<>();
+ params.put("blah", "true");
+
+ ActionContext extraContext = ActionContext.of()
+ .withParameters(HttpParameters.create(params).build());
+
+ DefaultActionInvocation defaultActionInvocation = new DefaultActionInvocation(extraContext.getContextMap(), true);
+ container.inject(defaultActionInvocation);
+
+ ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "LazyFooLazilyDisabled", null, extraContext.getContextMap());
+ defaultActionInvocation.init(actionProxy);
+ defaultActionInvocation.invoke();
+
+ SimpleAction action = (SimpleAction) defaultActionInvocation.getAction();
+
+ // the rest of the stack still ran, so the params interceptor applied blah...
+ assertThat(action.getBlah()).isEqualTo("true");
+ // ...but the lazy interceptor was skipped, so it never applied its foo param to the action
+ assertThat(action.getName()).isNull();
+ }
+
+ /**
+ * Regression for WW-5659: {@code disabled} configured on the interceptor definition never reaches
+ * the params holder, so it can only be honoured through {@link org.apache.struts2.interceptor.ConditionalInterceptor#shouldIntercept}.
+ * The lazy path must still consult it.
+ */
+ public void testInvokeWithLazyParamsSkipsStaticallyDisabledInterceptor() throws Exception {
+ HashMap<String, Object> params = new HashMap<>();
+ params.put("blah", "dynamic value");
+
+ ActionContext extraContext = ActionContext.of()
+ .withParameters(HttpParameters.create(params).build());
+
+ DefaultActionInvocation defaultActionInvocation = new DefaultActionInvocation(extraContext.getContextMap(), true);
+ container.inject(defaultActionInvocation);
+
+ ActionProxy actionProxy = actionProxyFactory.createActionProxy("", "LazyFooStaticallyDisabled", null, extraContext.getContextMap());
+ defaultActionInvocation.init(actionProxy);
+ defaultActionInvocation.invoke();
+
+ SimpleAction action = (SimpleAction) defaultActionInvocation.getAction();
+
+ assertThat(action.getBlah()).isEqualTo("dynamic value");
+ assertThat(action.getName()).isNull();
+ }
+
public void testInvokeWithAsyncManager() throws Exception {
DefaultActionInvocation dai = new DefaultActionInvocation(new HashMap<>(), false);
dai.stack = container.getInstance(ValueStackFactory.class).createValueStack();
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index c2cf40719..e9bca76cc 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -888,6 +888,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
private String allowedMimeTypes;
private String allowedExtensions;
private Long maxFileSize;
+ private String uploadDisabled;
@Override
public void withUploadedFiles(List<UploadedFile> uploadedFiles) {
@@ -921,6 +922,14 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
public void setMaxFileSize(Long maxFileSize) {
this.maxFileSize = maxFileSize;
}
+
+ public String getUploadDisabled() {
+ return uploadDisabled;
+ }
+
+ public void setUploadDisabled(String uploadDisabled) {
+ this.uploadDisabled = uploadDisabled;
+ }
}
public void testUploadPolicyParsesAndCopies() {
@@ -1028,17 +1037,43 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
container.inject(interceptor);
- MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
- action.setAllowedMimeTypes("text/plain");
- container.inject(action);
+ MyDynamicFileUploadAction disablingAction = new MyDynamicFileUploadAction();
+ disablingAction.setUploadDisabled("true");
+ container.inject(disablingAction);
- UploadPolicy policy = interceptor.newLazyParams();
- policy.setDisabled("true");
+ MyDynamicFileUploadAction enablingAction = new MyDynamicFileUploadAction();
+ enablingAction.setUploadDisabled("false");
+ container.inject(enablingAction);
+
+ UploadPolicy disabledPolicy = resolveDisabled(interceptor, disablingAction);
+ UploadPolicy enabledPolicy = resolveDisabled(interceptor, enablingAction);
- assertThat(policy.isDisabled()).isTrue();
+ // the second resolution must not have cleared the first invocation's flag...
+ assertThat(disabledPolicy.isDisabled()).isTrue();
+ assertThat(enabledPolicy.isDisabled()).isFalse();
+ // ...and neither resolution may reach the shared interceptor
assertThat(interceptor.newLazyParams().isDisabled()).isFalse();
}
+ private UploadPolicy resolveDisabled(ActionFileUploadInterceptor actionFileUploadInterceptor,
+ MyDynamicFileUploadAction action) {
+ ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack();
+ valueStack.push(action);
+
+ ActionContext context = ActionContext.of(valueStack.getContext())
+ .withContainer(container)
+ .withValueStack(valueStack)
+ .bind();
+ try {
+ WithLazyParams.LazyParamInjector injector = new WithLazyParams.LazyParamInjector(valueStack);
+ container.inject(injector);
+ return injector.resolveInto(actionFileUploadInterceptor.newLazyParams(),
+ Map.of("disabled", "${uploadDisabled}"), context);
+ } finally {
+ ActionContext.clear();
+ }
+ }
+
private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInterceptor,
MyDynamicFileUploadAction action,
MockHttpServletRequest uploadRequest) throws Exception {
diff --git a/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java b/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java
index 75d3d021a..f875c043c 100644
--- a/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java
+++ b/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java
@@ -21,15 +21,16 @@ package org.apache.struts2.mock;
import org.apache.struts2.ActionInvocation;
import org.apache.struts2.SimpleAction;
import org.apache.struts2.interceptor.AbstractInterceptor;
-import org.apache.struts2.interceptor.InterceptorParams;
+import org.apache.struts2.interceptor.DisableParams;
import org.apache.struts2.interceptor.WithLazyParams;
public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams<MockLazyInterceptor.MockLazyParams> {
/**
- * Per-invocation holder, seeded from the configured values.
+ * Per-invocation holder, seeded from the configured values. Extends {@link DisableParams} so a
+ * lazily resolved {@code disabled} param applies to a single invocation.
*/
- public static class MockLazyParams implements InterceptorParams {
+ public static class MockLazyParams extends DisableParams {
private String foo = "";
private String bar = "";
diff --git a/core/src/test/resources/xwork-sample.xml b/core/src/test/resources/xwork-sample.xml
index 5bc189d82..5615e8ef2 100644
--- a/core/src/test/resources/xwork-sample.xml
+++ b/core/src/test/resources/xwork-sample.xml
@@ -72,6 +72,27 @@
</interceptor-ref>
</action>
+ <!-- disabled resolved lazily per invocation: reaches the interceptor mapping's params,
+ so it lands on the params holder and not on the shared interceptor -->
+ <action name="LazyFooLazilyDisabled" class="org.apache.struts2.SimpleAction">
+ <result name="error" type="void"/>
+ <interceptor-ref name="params"/>
+ <interceptor-ref name="lazy">
+ <param name="foo">should not be applied</param>
+ <param name="disabled">${blah}</param>
+ </interceptor-ref>
+ </action>
+
+ <!-- disabled configured statically on the interceptor definition: invisible to the params
+ holder, honoured only via ConditionalInterceptor#shouldIntercept -->
+ <action name="LazyFooStaticallyDisabled" class="org.apache.struts2.SimpleAction">
+ <result name="error" type="void"/>
+ <interceptor-ref name="params"/>
+ <interceptor-ref name="lazyStaticallyDisabled">
+ <param name="foo">should not be applied</param>
+ </interceptor-ref>
+ </action>
+
<action name="WildCard" class="org.apache.struts2.SimpleAction">
<param name="foo">17</param>
<param name="bar">23</param>
diff --git a/core/src/test/resources/xwork-test-default.xml b/core/src/test/resources/xwork-test-default.xml
index 31f92d9d6..87931c7d2 100644
--- a/core/src/test/resources/xwork-test-default.xml
+++ b/core/src/test/resources/xwork-test-default.xml
@@ -42,6 +42,11 @@
<param name="foo">expectedFoo</param>
</interceptor>
<interceptor name="lazy" class="org.apache.struts2.mock.MockLazyInterceptor"/>
+ <!-- disabled at interceptor-definition level, so it never reaches InterceptorMapping#getParams()
+ and is only visible through ConditionalInterceptor#shouldIntercept -->
+ <interceptor name="lazyStaticallyDisabled" class="org.apache.struts2.mock.MockLazyInterceptor">
+ <param name="disabled">true</param>
+ </interceptor>
<interceptor-stack name="defaultStack">
<interceptor-ref name="staticParams"/>
(struts) 07/13: WW-5659 refactor(core): hold upload policy in one value object
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit af6e5fb81809dedacf29f7f05c9a8690c779627c
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:42:20 2026 +0200
WW-5659 refactor(core): hold upload policy in one value object
Introduce UploadPolicy (extends DisableParams) to consolidate the three
loose maximumSize/allowedTypes/allowedExtensions fields on
AbstractFileUploadInterceptor into a single config-time value object.
acceptFile now takes the effective policy as an explicit parameter
instead of reading interceptor-level state directly.
Pure refactor, no behaviour change: the existing setters still mutate
the shared singleton via configuredPolicy, and ActionFileUploadInterceptor
copies it once per invocation via copyConfiguredPolicy() before calling
acceptFile. This groundwork lets a later change route lazily-resolved
per-request params into the copy instead of the singleton.
---
.../interceptor/AbstractFileUploadInterceptor.java | 43 ++++++----
.../interceptor/ActionFileUploadInterceptor.java | 4 +-
.../apache/struts2/interceptor/UploadPolicy.java | 95 ++++++++++++++++++++++
.../ActionFileUploadInterceptorTest.java | 61 +++++++++++---
4 files changed, 172 insertions(+), 31 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
index c7cbb65c8..f8529732b 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
@@ -24,7 +24,6 @@ import org.apache.struts2.text.TextProvider;
import org.apache.struts2.text.TextProviderFactory;
import org.apache.struts2.inject.Container;
import org.apache.struts2.inject.Inject;
-import org.apache.struts2.util.TextParseUtil;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.dispatcher.LocalizedMessage;
@@ -35,7 +34,6 @@ import org.apache.struts2.util.ContentTypeMatcher;
import java.text.NumberFormat;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Set;
@@ -59,9 +57,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
public static final String STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY = "struts.messages.error.content.type.not.allowed";
public static final String STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY = "struts.messages.error.file.extension.not.allowed";
- private Long maximumSize;
- private Set<String> allowedTypesSet = Collections.emptySet();
- private Set<String> allowedExtensionsSet = Collections.emptySet();
+ private final UploadPolicy configuredPolicy = new UploadPolicy();
private ContentTypeMatcher<Object> matcher;
private Container container;
@@ -77,35 +73,48 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
}
/**
- * Sets the allowed extensions
+ * Sets the allowed extensions. Applied at configuration time only; the effective policy for
+ * an invocation is a copy, see {@link ActionFileUploadInterceptor#newLazyParams()}.
*
* @param allowedExtensions A comma-delimited list of extensions
*/
public void setAllowedExtensions(String allowedExtensions) {
- allowedExtensionsSet = TextParseUtil.commaDelimitedStringToSet(allowedExtensions);
+ configuredPolicy.setAllowedExtensions(allowedExtensions);
}
/**
- * Sets the allowed mimetypes
+ * Sets the allowed mimetypes. Applied at configuration time only; the effective policy for
+ * an invocation is a copy, see {@link ActionFileUploadInterceptor#newLazyParams()}.
*
* @param allowedTypes A comma-delimited list of types
*/
public void setAllowedTypes(String allowedTypes) {
- allowedTypesSet = TextParseUtil.commaDelimitedStringToSet(allowedTypes);
+ configuredPolicy.setAllowedTypes(allowedTypes);
}
/**
- * Sets the maximum size of an uploaded file
+ * Sets the maximum size of an uploaded file. Applied at configuration time only; the
+ * effective policy for an invocation is a copy, see
+ * {@link ActionFileUploadInterceptor#newLazyParams()}.
*
* @param maximumSize The maximum size in bytes
*/
public void setMaximumSize(Long maximumSize) {
- this.maximumSize = maximumSize;
+ configuredPolicy.setMaximumSize(maximumSize);
+ }
+
+ /**
+ * @return an independent copy of the configured policy, to be resolved for one invocation
+ * @since 7.3.0
+ */
+ protected UploadPolicy copyConfiguredPolicy() {
+ return configuredPolicy.copy();
}
/**
* Override for added functionality. Checks if the proposed file is acceptable based on contentType and size.
*
+ * @param policy - the effective upload policy for this invocation.
* @param action - uploading action for message retrieval.
* @param file - proposed upload file.
* @param originalFilename - name of the file.
@@ -113,7 +122,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
* @param inputName - inputName of the file.
* @return true if the proposed file is acceptable by contentType and size.
*/
- protected boolean acceptFile(Object action, UploadedFile file, String originalFilename, String contentType, String inputName) {
+ protected boolean acceptFile(UploadPolicy policy, Object action, UploadedFile file, String originalFilename, String contentType, String inputName) {
Set<String> errorMessages = new HashSet<>();
ValidationAware validation = null;
@@ -131,21 +140,21 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
return false;
}
- if (maximumSize != null && maximumSize < file.length()) {
+ if (policy.getMaximumSize() != null && policy.getMaximumSize() < file.length()) {
String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY, new String[]{
- inputName, originalFilename, file.getName(), "" + file.length(), getMaximumSizeStr(action)
+ inputName, originalFilename, file.getName(), "" + file.length(), getMaximumSizeStr(action, policy.getMaximumSize())
});
errorMessages.add(errMsg);
LOG.warn(errMsg);
}
- if ((!allowedTypesSet.isEmpty()) && (!containsItem(allowedTypesSet, contentType))) {
+ if ((!policy.getAllowedTypes().isEmpty()) && (!containsItem(policy.getAllowedTypes(), contentType))) {
String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY, new String[]{
inputName, originalFilename, file.getName(), contentType
});
errorMessages.add(errMsg);
LOG.warn(errMsg);
}
- if ((!allowedExtensionsSet.isEmpty()) && (!hasAllowedExtension(allowedExtensionsSet, originalFilename))) {
+ if ((!policy.getAllowedExtensions().isEmpty()) && (!hasAllowedExtension(policy.getAllowedExtensions(), originalFilename))) {
String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY, new String[]{
inputName, originalFilename, file.getName(), contentType
});
@@ -161,7 +170,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
return errorMessages.isEmpty();
}
- private String getMaximumSizeStr(Object action) {
+ private String getMaximumSizeStr(Object action, Long maximumSize) {
return NumberFormat.getNumberInstance(getLocaleProvider(action).getLocale()).format(maximumSize);
}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
index 79d020a34..4aea0f782 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
@@ -228,6 +228,8 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor i
return invocation.invoke();
}
+ UploadPolicy policy = copyConfiguredPolicy();
+
applyValidation(action, multiWrapper);
// bind allowed Files
@@ -245,7 +247,7 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor i
}
} else {
for (UploadedFile uploadedFile : uploadedFiles) {
- if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) {
+ if (acceptFile(policy, action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) {
acceptedFiles.add(uploadedFile);
}
}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java b/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
new file mode 100644
index 000000000..a9e444157
--- /dev/null
+++ b/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
@@ -0,0 +1,95 @@
+/*
+ * 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.util.TextParseUtil;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * Per-invocation upload validation policy for {@link ActionFileUploadInterceptor}.
+ * <p>
+ * A configured instance is held by the interceptor and copied for each invocation, so lazily
+ * resolved values never reach the shared interceptor.
+ *
+ * @since 7.3.0
+ */
+public class UploadPolicy extends DisableParams {
+
+ private Long maximumSize;
+ private Set<String> allowedTypes = Collections.emptySet();
+ private Set<String> allowedExtensions = Collections.emptySet();
+
+ public UploadPolicy() {
+ }
+
+ private UploadPolicy(UploadPolicy other) {
+ super(other);
+ this.maximumSize = other.maximumSize;
+ this.allowedTypes = other.allowedTypes;
+ this.allowedExtensions = other.allowedExtensions;
+ }
+
+ /**
+ * @param allowedTypes a comma-delimited list of content types, or null for no restriction
+ */
+ public void setAllowedTypes(String allowedTypes) {
+ this.allowedTypes = toSet(allowedTypes);
+ }
+
+ /**
+ * @param allowedExtensions a comma-delimited list of extensions, or null for no restriction
+ */
+ public void setAllowedExtensions(String allowedExtensions) {
+ this.allowedExtensions = toSet(allowedExtensions);
+ }
+
+ /**
+ * @param maximumSize the maximum size in bytes, or null for no limit
+ */
+ public void setMaximumSize(Long maximumSize) {
+ this.maximumSize = maximumSize;
+ }
+
+ public Long getMaximumSize() {
+ return maximumSize;
+ }
+
+ public Set<String> getAllowedTypes() {
+ return allowedTypes;
+ }
+
+ public Set<String> getAllowedExtensions() {
+ return allowedExtensions;
+ }
+
+ /**
+ * @return an independent copy, used to seed a per-invocation policy from the configured one
+ */
+ public UploadPolicy copy() {
+ return new UploadPolicy(this);
+ }
+
+ private static Set<String> toSet(String commaDelimited) {
+ return commaDelimited == null
+ ? Collections.emptySet()
+ : TextParseUtil.commaDelimitedStringToSet(commaDelimited);
+ }
+}
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index 058dabe11..c45091d77 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -63,7 +63,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
public void testAcceptFileWithEmptyAllowedTypesAndExtensions() {
// when allowed type is empty
ValidationAwareSupport validation = new ValidationAwareSupport();
- boolean ok = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename", "text/plain", "inputName");
+ boolean ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename", "text/plain", "inputName");
assertThat(ok).isTrue();
assertThat(validation.getFieldErrors()).isEmpty();
@@ -75,7 +75,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
// when file is of allowed types
ValidationAwareSupport validation = new ValidationAwareSupport();
- boolean ok = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.txt", "text/plain", "inputName");
+ boolean ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.txt", "text/plain", "inputName");
assertThat(ok).isTrue();
assertThat(validation.getFieldErrors()).isEmpty();
@@ -83,7 +83,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
// when file is not of allowed types
validation = new ValidationAwareSupport();
- boolean notOk = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.html", "text/html", "inputName");
+ boolean notOk = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.html", "text/html", "inputName");
assertThat(notOk).isFalse();
assertThat(validation.getFieldErrors()).isNotEmpty();
@@ -94,7 +94,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.setAllowedTypes("text/*");
ValidationAwareSupport validation = new ValidationAwareSupport();
- boolean ok = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.txt", "text/plain", "inputName");
+ boolean ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.txt", "text/plain", "inputName");
assertThat(ok).isTrue();
assertThat(validation.getFieldErrors()).isEmpty();
@@ -102,7 +102,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.setAllowedTypes("text/h*");
validation = new ValidationAwareSupport();
- boolean notOk = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.html", "text/plain", "inputName");
+ boolean notOk = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.html", "text/plain", "inputName");
assertThat(notOk).isFalse();
assertThat(validation.getFieldErrors()).isNotEmpty();
@@ -114,7 +114,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
// when file is of allowed extensions
ValidationAwareSupport validation = new ValidationAwareSupport();
- boolean ok = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.txt", "text/plain", "inputName");
+ boolean ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.txt", "text/plain", "inputName");
assertThat(ok).isTrue();
assertThat(validation.getFieldErrors()).isEmpty();
@@ -122,7 +122,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
// when file is not of allowed extensions
validation = new ValidationAwareSupport();
- boolean notOk = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.html", "text/html", "inputName");
+ boolean notOk = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.html", "text/html", "inputName");
assertThat(notOk).isFalse();
assertThat(validation.getFieldErrors()).isNotEmpty();
@@ -130,7 +130,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.setAllowedExtensions(".txt,.lol");
validation = new ValidationAwareSupport();
- ok = interceptor.acceptFile(validation, createTestFile(Files.newTemporaryFile()), "filename.lol", "text/plain", "inputName");
+ ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(Files.newTemporaryFile()), "filename.lol", "text/plain", "inputName");
assertThat(ok).isTrue();
assertThat(validation.getFieldErrors()).isEmpty();
@@ -142,7 +142,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
// when file is not of allowed types
ValidationAwareSupport validation = new ValidationAwareSupport();
- boolean notOk = interceptor.acceptFile(validation, null, "filename.html", "text/html", "inputName");
+ boolean notOk = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, null, "filename.html", "text/html", "inputName");
assertThat(notOk).isFalse();
assertThat(validation.getFieldErrors()).isNotEmpty();
@@ -159,7 +159,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.setAllowedTypes("text/plain");
ValidationAwareSupport validation = new ValidationAwareSupport();
- boolean notOk = interceptor.acceptFile(validation, createTestFile(null), "filename.html", "text/plain", "inputName");
+ boolean notOk = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, createTestFile(null), "filename.html", "text/plain", "inputName");
assertThat(notOk).isFalse();
assertThat(validation.getFieldErrors()).isNotEmpty();
@@ -183,7 +183,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
.withInputName("inputName")
.build();
- boolean ok = interceptor.acceptFile(validation, file, "f.txt", "text/plain", "inputName");
+ boolean ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, file, "f.txt", "text/plain", "inputName");
assertThat(ok).isTrue();
assertThat(validation.hasErrors()).isFalse();
@@ -203,7 +203,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
.build();
// wrong content type -> rejected
- boolean ok = interceptor.acceptFile(validation, file, "f.html", "text/html", "inputName");
+ boolean ok = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, file, "f.html", "text/html", "inputName");
assertThat(ok).isFalse();
assertThat(validation.hasErrors()).isTrue();
@@ -221,7 +221,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
File file = new File(new URI(url.toString()));
assertThat(file).exists();
UploadedFile uploadedFile = StrutsUploadedFile.Builder.create(file).withContentType("text/html").withOriginalName("filename").build();
- boolean notOk = interceptor.acceptFile(validation, uploadedFile, "filename", "text/html", "inputName");
+ boolean notOk = interceptor.acceptFile(interceptor.copyConfiguredPolicy(), validation, uploadedFile, "filename", "text/html", "inputName");
assertThat(notOk).isFalse();
assertThat(validation.getFieldErrors()).isNotEmpty();
@@ -920,4 +920,39 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
}
}
+ public void testUploadPolicyParsesAndCopies() {
+ UploadPolicy policy = new UploadPolicy();
+ policy.setAllowedTypes("text/plain, text/html");
+ policy.setAllowedExtensions(".txt,.html");
+ policy.setMaximumSize(1024L);
+ policy.setDisabled("true");
+
+ UploadPolicy copy = policy.copy();
+
+ assertThat(copy.getAllowedTypes()).containsExactlyInAnyOrder("text/plain", "text/html");
+ assertThat(copy.getAllowedExtensions()).containsExactlyInAnyOrder(".txt", ".html");
+ assertThat(copy.getMaximumSize()).isEqualTo(1024L);
+ assertThat(copy.isDisabled()).isTrue();
+ }
+
+ public void testUploadPolicyCopyIsIndependentOfTheOriginal() {
+ UploadPolicy policy = new UploadPolicy();
+ policy.setAllowedTypes("text/plain");
+
+ UploadPolicy copy = policy.copy();
+ copy.setAllowedTypes("text/html");
+
+ assertThat(policy.getAllowedTypes()).containsExactly("text/plain");
+ assertThat(copy.getAllowedTypes()).containsExactly("text/html");
+ }
+
+ public void testUploadPolicyTreatsNullAsNoRestriction() {
+ UploadPolicy policy = new UploadPolicy();
+ policy.setAllowedTypes(null);
+ policy.setAllowedExtensions(null);
+
+ assertThat(policy.getAllowedTypes()).isEmpty();
+ assertThat(policy.getAllowedExtensions()).isEmpty();
+ }
+
}
(struts) 05/13: WW-5659 feat(core): resolve lazy params into a holder instead of the interceptor
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 687963abae8c79e708f5a62b06ca70c32649839c
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:23:20 2026 +0200
WW-5659 feat(core): resolve lazy params into a holder instead of the interceptor
---
.../apache/struts2/interceptor/WithLazyParams.java | 49 ++++++++
.../struts2/interceptor/LazyParamInjectorTest.java | 134 +++++++++++++++++++++
2 files changed, 183 insertions(+)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
index cdfb3a8ad..8c74b398d 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
@@ -18,12 +18,15 @@
*/
package org.apache.struts2.interceptor;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
import org.apache.struts2.ActionContext;
import org.apache.struts2.inject.Inject;
import org.apache.struts2.ognl.OgnlUtil;
import org.apache.struts2.util.TextParseUtil;
import org.apache.struts2.util.TextParser;
import org.apache.struts2.util.ValueStack;
+import org.apache.struts2.util.reflection.ReflectionException;
import org.apache.struts2.util.reflection.ReflectionProvider;
import java.util.Map;
@@ -49,6 +52,8 @@ public interface WithLazyParams {
class LazyParamInjector {
+ private static final Logger LOG = LogManager.getLogger(LazyParamInjector.class);
+
protected OgnlUtil ognlUtil;
protected TextParser textParser;
protected ReflectionProvider reflectionProvider;
@@ -82,5 +87,49 @@ public interface WithLazyParams {
}
return interceptor;
}
+
+ /**
+ * Resolves configured params into a per-invocation holder, leaving the interceptor untouched.
+ * <p>
+ * A {@code ${...}} expression that cannot be resolved is not written: the holder keeps its
+ * seeded configuration value and is notified via {@link InterceptorParams#unresolved(String)},
+ * so a broken expression cannot silently relax a validation policy.
+ *
+ * @since 7.3.0
+ */
+ public <P extends InterceptorParams> P resolveInto(P target, Map<String, String> params, ActionContext invocationContext) {
+ for (Map.Entry<String, String> entry : params.entrySet()) {
+ String paramName = entry.getKey();
+ String rawValue = entry.getValue();
+ Object paramValue = textParser.evaluate(new char[]{'$'}, rawValue, valueEvaluator, TextParser.DEFAULT_LOOP_COUNT);
+
+ if (isUnresolved(rawValue, paramValue)) {
+ LOG.warn("Param [{}] of [{}] could not be resolved from expression [{}]; keeping the configured value",
+ paramName, target.getClass().getName(), rawValue);
+ target.unresolved(paramName);
+ continue;
+ }
+ try {
+ // throwPropertyExceptions=true so a param with no matching property on the holder is
+ // reported rather than silently ignored; OgnlUtil only warns in devMode otherwise
+ ognlUtil.setProperty(paramName, paramValue, target, invocationContext.getContextMap(), true);
+ } catch (ReflectionException e) {
+ LOG.warn("Param [{}] cannot be applied to [{}]; check the interceptor configuration",
+ paramName, target.getClass().getName(), e);
+ }
+ }
+ return target;
+ }
+
+ /**
+ * {@link org.apache.struts2.util.OgnlTextParser} yields an empty string for an expression that
+ * does not resolve and gives no other signal, so the raw template is needed to tell that apart
+ * from a legitimately empty value.
+ */
+ private boolean isUnresolved(String rawValue, Object paramValue) {
+ return rawValue != null
+ && rawValue.contains("${")
+ && (paramValue == null || paramValue.toString().isEmpty());
+ }
}
}
diff --git a/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java b/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
new file mode 100644
index 000000000..8939c1313
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
@@ -0,0 +1,134 @@
+/*
+ * 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.ActionContext;
+import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.util.ValueStack;
+import org.apache.struts2.util.ValueStackFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class LazyParamInjectorTest extends StrutsInternalTestCase {
+
+ public static class Holder extends DisableParams {
+ private String name;
+ private Long size;
+ private final List<String> unresolvedCalls = new ArrayList<>();
+
+ public void setName(String name) { this.name = name; }
+ public void setSize(Long size) { this.size = size; }
+ public String getName() { return name; }
+ public Long getSize() { return size; }
+ public List<String> getUnresolvedCalls() { return unresolvedCalls; }
+
+ @Override
+ public void unresolved(String paramName) { unresolvedCalls.add(paramName); }
+ }
+
+ public static class Bean {
+ public String getLabel() { return "resolved-label"; }
+ public Long getLimit() { return 4096L; }
+ }
+
+ private ActionContext context;
+ private WithLazyParams.LazyParamInjector injector;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ ValueStack stack = container.getInstance(ValueStackFactory.class).createValueStack();
+ stack.push(new Bean());
+ context = ActionContext.of(stack.getContext()).withContainer(container).withValueStack(stack).bind();
+ injector = new WithLazyParams.LazyParamInjector(stack);
+ container.inject(injector);
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ ActionContext.clear();
+ super.tearDown();
+ }
+
+ public void testResolvesExpressionsIntoTheHolder() {
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "${label}");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getName()).isEqualTo("resolved-label");
+ assertThat(holder.getUnresolvedCalls()).isEmpty();
+ }
+
+ public void testAppliesOgnlTypeConversion() {
+ Map<String, String> params = new HashMap<>();
+ params.put("size", "${limit}");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getSize()).isEqualTo(4096L);
+ }
+
+ public void testPassesLiteralValuesThrough() {
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "plain-text");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getName()).isEqualTo("plain-text");
+ assertThat(holder.getUnresolvedCalls()).isEmpty();
+ }
+
+ public void testUnresolvableExpressionSkipsWriteAndNotifiesHolder() {
+ Holder seeded = new Holder();
+ seeded.setName("seeded-value");
+
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "${noSuchProperty}");
+
+ Holder holder = injector.resolveInto(seeded, params, context);
+
+ assertThat(holder.getName()).isEqualTo("seeded-value");
+ assertThat(holder.getUnresolvedCalls()).containsExactly("name");
+ }
+
+ public void testResolvesDisabledOntoDisableParams() {
+ Map<String, String> params = new HashMap<>();
+ params.put("disabled", "true");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.isDisabled()).isTrue();
+ }
+
+ public void testUnknownParamIsIgnoredWithoutFailingTheInvocation() {
+ Map<String, String> params = new HashMap<>();
+ params.put("noSuchParam", "whatever");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getName()).isNull();
+ assertThat(holder.getSize()).isNull();
+ }
+}
(struts) 04/13: WW-5659 feat(core): add InterceptorParams contract and DisableParams holder
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit c35483c65eab332e0635f508399804732bf36d98
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:18:42 2026 +0200
WW-5659 feat(core): add InterceptorParams contract and DisableParams holder
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../apache/struts2/interceptor/DisableParams.java | 51 ++++++++++++++++++
.../struts2/interceptor/InterceptorParams.java | 43 ++++++++++++++++
.../struts2/interceptor/DisableParamsTest.java | 60 ++++++++++++++++++++++
3 files changed, 154 insertions(+)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/DisableParams.java b/core/src/main/java/org/apache/struts2/interceptor/DisableParams.java
new file mode 100644
index 000000000..aa3446787
--- /dev/null
+++ b/core/src/main/java/org/apache/struts2/interceptor/DisableParams.java
@@ -0,0 +1,51 @@
+/*
+ * 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;
+
+/**
+ * Opt-in support for the {@code disabled} interceptor parameter.
+ * <p>
+ * Interceptors implementing {@link WithLazyParams} must have their params holder extend this
+ * class to support {@code <param name="disabled">...</param>}; there is deliberately no
+ * fallback to the interceptor instance, which would reintroduce shared mutable state.
+ *
+ * @since 7.3.0
+ */
+public class DisableParams implements InterceptorParams {
+
+ private boolean disabled;
+
+ public DisableParams() {
+ }
+
+ protected DisableParams(DisableParams other) {
+ this.disabled = other.disabled;
+ }
+
+ /**
+ * @param disable if {@code true}, execution of the interceptor is skipped for this invocation
+ */
+ public void setDisabled(String disable) {
+ this.disabled = Boolean.parseBoolean(disable);
+ }
+
+ public boolean isDisabled() {
+ return disabled;
+ }
+}
diff --git a/core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.java b/core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.java
new file mode 100644
index 000000000..68cf3e2ba
--- /dev/null
+++ b/core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.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;
+
+/**
+ * Contract for an object holding the parameters of a single interceptor.
+ * <p>
+ * Implementations are per-invocation value objects: the framework resolves configured
+ * parameters into a fresh instance for each action invocation, so nothing is written back
+ * onto the interceptor, which stays immutable after {@link Interceptor#init()}.
+ *
+ * @since 7.3.0
+ */
+public interface InterceptorParams {
+
+ /**
+ * Called when a {@code ${...}} parameter could not be resolved for the current invocation.
+ * The framework skips the write, leaving the seeded configuration value in place, and
+ * notifies the holder so it can decide how to degrade.
+ * <p>
+ * The default implementation does nothing.
+ *
+ * @param paramName name of the parameter that could not be resolved
+ */
+ default void unresolved(String paramName) {
+ }
+}
diff --git a/core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java b/core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java
new file mode 100644
index 000000000..3a41b0b84
--- /dev/null
+++ b/core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java
@@ -0,0 +1,60 @@
+/*
+ * 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 junit.framework.TestCase;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DisableParamsTest extends TestCase {
+
+ public void testDisabledDefaultsToFalse() {
+ assertThat(new DisableParams().isDisabled()).isFalse();
+ }
+
+ public void testSetDisabledParsesStringValue() {
+ DisableParams params = new DisableParams();
+ params.setDisabled("true");
+ assertThat(params.isDisabled()).isTrue();
+
+ params.setDisabled("false");
+ assertThat(params.isDisabled()).isFalse();
+ }
+
+ public void testSetDisabledTreatsNonBooleanTextAsFalse() {
+ DisableParams params = new DisableParams();
+ params.setDisabled("yes");
+ assertThat(params.isDisabled()).isFalse();
+ }
+
+ public void testCopyConstructorCarriesDisabledFlag() {
+ DisableParams original = new DisableParams();
+ original.setDisabled("true");
+
+ DisableParams copy = new DisableParams(original);
+
+ assertThat(copy.isDisabled()).isTrue();
+ }
+
+ public void testUnresolvedDefaultsToNoOp() {
+ DisableParams params = new DisableParams();
+ params.unresolved("someParam"); // must not throw
+ assertThat(params.isDisabled()).isFalse();
+ }
+}
(struts) 02/13: WW-5659 docs: implementation plan for request-scoped lazy params
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit e51a5ce36dc33f1a784a971e3249afddf62b5367
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:10:01 2026 +0200
WW-5659 docs: implementation plan for request-scoped lazy params
Six tasks, each independently testable and compiling: new InterceptorParams
and DisableParams types, LazyParamInjector.resolveInto alongside the old
path, a pure refactor onto a single UploadPolicy value object, the contract
switch plus DefaultActionInvocation wiring, fail-closed handling, and test
migration.
Records one deviation from the spec: the unresolved-param rule is applied
unconditionally rather than by introspecting the seeded value, which is not
implementable deterministically for the Long-typed maximumSize. Task 6
updates the spec to match.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...26-07-27-WW-5659-lazy-params-request-scoping.md | 1323 ++++++++++++++++++++
1 file changed, 1323 insertions(+)
diff --git a/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md b/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md
new file mode 100644
index 000000000..f1af340e8
--- /dev/null
+++ b/docs/superpowers/plans/2026-07-27-WW-5659-lazy-params-request-scoping.md
@@ -0,0 +1,1323 @@
+# WW-5659 Request-Scoped Lazy Interceptor Params Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Stop `WithLazyParams` from writing per-request resolved `${...}` params onto the shared interceptor singleton, so concurrent requests can no longer read one another's upload validation policy.
+
+**Architecture:** Resolved params are written into a per-invocation holder object that the interceptor supplies (`newLazyParams()`) and receives back (`intercept(invocation, params)`). The interceptor singleton becomes immutable after `init()`. A new `InterceptorParams` marker is the general contract; `DisableParams` is opt-in support for the `disabled` param; `UploadPolicy` is the file-upload holder. Unresolvable expressions fail closed instead of silently disabling validation.
+
+**Tech Stack:** Java 17, Maven, JUnit 3-style tests (`junit.framework.TestCase` via `XWorkTestCase`), AssertJ assertions, Log4j2.
+
+**Spec:** `docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md`
+
+## Global Constraints
+
+- **Ticket prefix:** every commit message starts with `WW-5659`.
+- **Target version:** 7.3.0. This is a deliberate public API break; no backport to 7.2.x.
+- **Test style:** `core` tests are JUnit 3 style — extend `StrutsInternalTestCase` (which extends `XWorkTestCase`, which extends `junit.framework.TestCase`), use `protected void setUp()`, and name tests `public void testXxx()`. **A JUnit 5 `@Test` annotation compiles and silently never runs.** Never add one.
+- **Assertions:** use AssertJ (`import static org.assertj.core.api.Assertions.assertThat;`), matching the existing `ActionFileUploadInterceptorTest`.
+- **Package:** all new production types go in `org.apache.struts2.interceptor`.
+- **License header:** every new `.java` file starts with the ASF header, copied verbatim from `core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java:1-18`.
+- **Message bundles:** new keys go into `core/src/main/resources/org/apache/struts2/struts-messages.properties` **only**. The five locale bundles (`_da`, `_de`, `_en`, `_pl`, `_pt`) are partial (12–14 keys vs 20 in the base) and fall back to the base bundle. Do not fabricate translations.
+- **Build command:** `mvn test -DskipAssembly -pl core -Dtest=ClassName#methodName`
+
+## Deviation from the spec — read before Task 5
+
+The spec's Error Handling section says `unresolved(param)` should mark a dimension unusable **only if** the seeded config-time value still contains `${`. That rule is not implementable deterministically: it relies on introspecting the seed, and for `maximumSize` the seed is a `Long`, so a `${...}` literal cannot survive there — whether config-time OGNL conversion of `"${maxFileSize}"` to `Long` yields `null`, throws, or skips the setter is not knowable without testing, and the rule would [...]
+
+**This plan implements the simpler, deterministic rule instead: on unresolved, the dimension is marked unusable unconditionally.** A static fallback then applies only when the param is absent from the lazy map entirely (pure static config), which never triggers `unresolved`. This is stricter, type-independent, and consistent with the chosen fail-closed policy.
+
+Task 6 includes a step to update the spec to match. **If the reviewer prefers the spec's original rule, stop and re-plan Task 5** rather than implementing both.
+
+## File Structure
+
+**Created:**
+- `core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.java` — general params-holder contract; generic bound for `WithLazyParams`.
+- `core/src/main/java/org/apache/struts2/interceptor/DisableParams.java` — opt-in holder state for the `disabled` param.
+- `core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java` — file-upload holder: sizes, types, extensions, unresolved tracking.
+- `core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java`
+- `core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java`
+
+**Modified:**
+- `core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java` — new generic contract; `injectParams` → `resolveInto`.
+- `core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java` — single config-time `UploadPolicy`; `acceptFile` takes the policy.
+- `core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java` — implements `WithLazyParams<UploadPolicy>`.
+- `core/src/main/java/org/apache/struts2/DefaultActionInvocation.java:258-276` — merged lazy/conditional path; non-mutating `mergedParams`.
+- `core/src/main/resources/org/apache/struts2/struts-messages.properties` — new fail-closed message key.
+- `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java` — regression + migration.
+
+---
+
+### Task 1: `InterceptorParams` and `DisableParams`
+
+Purely additive — nothing references these yet. Establishes the vocabulary the rest of the plan builds on.
+
+**Files:**
+- Create: `core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.java`
+- Create: `core/src/main/java/org/apache/struts2/interceptor/DisableParams.java`
+- Test: `core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java`
+
+**Interfaces:**
+- Consumes: nothing.
+- Produces: `interface InterceptorParams { default void unresolved(String paramName) {} }`; `class DisableParams implements InterceptorParams` with `void setDisabled(String)`, `boolean isDisabled()`, and `protected DisableParams(DisableParams other)` copy constructor.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java` (ASF header first):
+
+```java
+package org.apache.struts2.interceptor;
+
+import junit.framework.TestCase;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class DisableParamsTest extends TestCase {
+
+ public void testDisabledDefaultsToFalse() {
+ assertThat(new DisableParams().isDisabled()).isFalse();
+ }
+
+ public void testSetDisabledParsesStringValue() {
+ DisableParams params = new DisableParams();
+ params.setDisabled("true");
+ assertThat(params.isDisabled()).isTrue();
+
+ params.setDisabled("false");
+ assertThat(params.isDisabled()).isFalse();
+ }
+
+ public void testSetDisabledTreatsNonBooleanTextAsFalse() {
+ DisableParams params = new DisableParams();
+ params.setDisabled("yes");
+ assertThat(params.isDisabled()).isFalse();
+ }
+
+ public void testCopyConstructorCarriesDisabledFlag() {
+ DisableParams original = new DisableParams();
+ original.setDisabled("true");
+
+ DisableParams copy = new DisableParams(original);
+
+ assertThat(copy.isDisabled()).isTrue();
+ }
+
+ public void testUnresolvedDefaultsToNoOp() {
+ DisableParams params = new DisableParams();
+ params.unresolved("someParam"); // must not throw
+ assertThat(params.isDisabled()).isFalse();
+ }
+}
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=DisableParamsTest`
+Expected: compilation failure — `DisableParams` does not exist.
+
+- [ ] **Step 3: Write minimal implementation**
+
+Create `core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.java` (ASF header first):
+
+```java
+package org.apache.struts2.interceptor;
+
+/**
+ * Contract for an object holding the parameters of a single interceptor.
+ * <p>
+ * Implementations are per-invocation value objects: the framework resolves configured
+ * parameters into a fresh instance for each action invocation, so nothing is written back
+ * onto the interceptor, which stays immutable after {@link Interceptor#init()}.
+ *
+ * @since 7.3.0
+ */
+public interface InterceptorParams {
+
+ /**
+ * Called when a {@code ${...}} parameter could not be resolved for the current invocation.
+ * The framework skips the write, leaving the seeded configuration value in place, and
+ * notifies the holder so it can decide how to degrade.
+ * <p>
+ * The default implementation does nothing.
+ *
+ * @param paramName name of the parameter that could not be resolved
+ */
+ default void unresolved(String paramName) {
+ }
+}
+```
+
+Create `core/src/main/java/org/apache/struts2/interceptor/DisableParams.java` (ASF header first):
+
+```java
+package org.apache.struts2.interceptor;
+
+/**
+ * Opt-in support for the {@code disabled} interceptor parameter.
+ * <p>
+ * Interceptors implementing {@link WithLazyParams} must have their params holder extend this
+ * class to support {@code <param name="disabled">...</param>}; there is deliberately no
+ * fallback to the interceptor instance, which would reintroduce shared mutable state.
+ *
+ * @since 7.3.0
+ */
+public class DisableParams implements InterceptorParams {
+
+ private boolean disabled;
+
+ public DisableParams() {
+ }
+
+ protected DisableParams(DisableParams other) {
+ this.disabled = other.disabled;
+ }
+
+ /**
+ * @param disable if {@code true}, execution of the interceptor is skipped for this invocation
+ */
+ public void setDisabled(String disable) {
+ this.disabled = Boolean.parseBoolean(disable);
+ }
+
+ public boolean isDisabled() {
+ return disabled;
+ }
+}
+```
+
+- [ ] **Step 4: Run test to verify it passes**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=DisableParamsTest`
+Expected: PASS, 5 tests.
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/interceptor/InterceptorParams.java \
+ core/src/main/java/org/apache/struts2/interceptor/DisableParams.java \
+ core/src/test/java/org/apache/struts2/interceptor/DisableParamsTest.java
+git commit -m "WW-5659 feat(core): add InterceptorParams contract and DisableParams holder"
+```
+
+---
+
+### Task 2: `LazyParamInjector.resolveInto`
+
+Adds the new resolution method **alongside** the existing `injectParams`, so the tree keeps compiling. `injectParams` is removed in Task 4 when the call site moves.
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java:78-84`
+- Test: `core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java`
+
+**Interfaces:**
+- Consumes: `InterceptorParams` from Task 1.
+- Produces: `public <P extends InterceptorParams> P resolveInto(P target, Map<String, String> params, ActionContext invocationContext)` on `WithLazyParams.LazyParamInjector`.
+
+- [ ] **Step 1: Write the failing test**
+
+Create `core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java` (ASF header first):
+
+```java
+package org.apache.struts2.interceptor;
+
+import org.apache.struts2.ActionContext;
+import org.apache.struts2.StrutsInternalTestCase;
+import org.apache.struts2.util.ValueStack;
+import org.apache.struts2.util.ValueStackFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class LazyParamInjectorTest extends StrutsInternalTestCase {
+
+ public static class Holder extends DisableParams {
+ private String name;
+ private Long size;
+ private final List<String> unresolvedCalls = new ArrayList<>();
+
+ public void setName(String name) { this.name = name; }
+ public void setSize(Long size) { this.size = size; }
+ public String getName() { return name; }
+ public Long getSize() { return size; }
+ public List<String> getUnresolvedCalls() { return unresolvedCalls; }
+
+ @Override
+ public void unresolved(String paramName) { unresolvedCalls.add(paramName); }
+ }
+
+ public static class Bean {
+ public String getLabel() { return "resolved-label"; }
+ public Long getLimit() { return 4096L; }
+ }
+
+ private ActionContext context;
+ private WithLazyParams.LazyParamInjector injector;
+
+ @Override
+ protected void setUp() throws Exception {
+ super.setUp();
+ ValueStack stack = container.getInstance(ValueStackFactory.class).createValueStack();
+ stack.push(new Bean());
+ context = ActionContext.of(stack.getContext()).withContainer(container).withValueStack(stack).bind();
+ injector = new WithLazyParams.LazyParamInjector(stack);
+ container.inject(injector);
+ }
+
+ @Override
+ protected void tearDown() throws Exception {
+ ActionContext.clear();
+ super.tearDown();
+ }
+
+ public void testResolvesExpressionsIntoTheHolder() {
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "${label}");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getName()).isEqualTo("resolved-label");
+ assertThat(holder.getUnresolvedCalls()).isEmpty();
+ }
+
+ public void testAppliesOgnlTypeConversion() {
+ Map<String, String> params = new HashMap<>();
+ params.put("size", "${limit}");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getSize()).isEqualTo(4096L);
+ }
+
+ public void testPassesLiteralValuesThrough() {
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "plain-text");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getName()).isEqualTo("plain-text");
+ assertThat(holder.getUnresolvedCalls()).isEmpty();
+ }
+
+ public void testUnresolvableExpressionSkipsWriteAndNotifiesHolder() {
+ Holder seeded = new Holder();
+ seeded.setName("seeded-value");
+
+ Map<String, String> params = new HashMap<>();
+ params.put("name", "${noSuchProperty}");
+
+ Holder holder = injector.resolveInto(seeded, params, context);
+
+ assertThat(holder.getName()).isEqualTo("seeded-value");
+ assertThat(holder.getUnresolvedCalls()).containsExactly("name");
+ }
+
+ public void testResolvesDisabledOntoDisableParams() {
+ Map<String, String> params = new HashMap<>();
+ params.put("disabled", "true");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.isDisabled()).isTrue();
+ }
+
+ public void testUnknownParamIsIgnoredWithoutFailingTheInvocation() {
+ Map<String, String> params = new HashMap<>();
+ params.put("noSuchParam", "whatever");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getName()).isNull();
+ assertThat(holder.getSize()).isNull();
+ }
+}
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=LazyParamInjectorTest`
+Expected: compilation failure — `resolveInto` does not exist.
+
+- [ ] **Step 3: Write minimal implementation**
+
+In `WithLazyParams.java`, add these imports:
+
+```java
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.struts2.util.reflection.ReflectionException;
+```
+
+Add a logger as the first member of `class LazyParamInjector`:
+
+```java
+private static final Logger LOG = LogManager.getLogger(LazyParamInjector.class);
+```
+
+Add the new method **below** the existing `injectParams` (leave `injectParams` in place for now):
+
+```java
+/**
+ * Resolves configured params into a per-invocation holder, leaving the interceptor untouched.
+ * <p>
+ * A {@code ${...}} expression that cannot be resolved is not written: the holder keeps its
+ * seeded configuration value and is notified via {@link InterceptorParams#unresolved(String)},
+ * so a broken expression cannot silently relax a validation policy.
+ *
+ * @since 7.3.0
+ */
+public <P extends InterceptorParams> P resolveInto(P target, Map<String, String> params, ActionContext invocationContext) {
+ for (Map.Entry<String, String> entry : params.entrySet()) {
+ String paramName = entry.getKey();
+ String rawValue = entry.getValue();
+ Object paramValue = textParser.evaluate(new char[]{'$'}, rawValue, valueEvaluator, TextParser.DEFAULT_LOOP_COUNT);
+
+ if (isUnresolved(rawValue, paramValue)) {
+ LOG.warn("Param [{}] of [{}] could not be resolved from expression [{}]; keeping the configured value",
+ paramName, target.getClass().getName(), rawValue);
+ target.unresolved(paramName);
+ continue;
+ }
+ try {
+ // throwPropertyExceptions=true so a param with no matching property on the holder is
+ // reported rather than silently ignored; OgnlUtil only warns in devMode otherwise
+ ognlUtil.setProperty(paramName, paramValue, target, invocationContext.getContextMap(), true);
+ } catch (ReflectionException e) {
+ LOG.warn("Param [{}] cannot be applied to [{}]; check the interceptor configuration",
+ paramName, target.getClass().getName(), e);
+ }
+ }
+ return target;
+}
+
+/**
+ * {@link org.apache.struts2.util.OgnlTextParser} yields an empty string for an expression that
+ * does not resolve and gives no other signal, so the raw template is needed to tell that apart
+ * from a legitimately empty value.
+ */
+private boolean isUnresolved(String rawValue, Object paramValue) {
+ return rawValue != null
+ && rawValue.contains("${")
+ && (paramValue == null || paramValue.toString().isEmpty());
+}
+```
+
+- [ ] **Step 4: Run test to verify it passes**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=LazyParamInjectorTest`
+Expected: PASS, 6 tests.
+
+- [ ] **Step 5: Confirm nothing else broke**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest,DefaultActionInvocationTest`
+Expected: PASS — `injectParams` is untouched, so existing behaviour is unchanged.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java \
+ core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
+git commit -m "WW-5659 feat(core): resolve lazy params into a holder instead of the interceptor"
+```
+
+---
+
+### Task 3: `UploadPolicy` and the `acceptFile` signature
+
+Pure refactor: the interceptor stops holding three loose fields and holds one config-time `UploadPolicy`; `acceptFile` receives the policy explicitly. **No behaviour change** — the old `injectParams` path still mutates the singleton via the setters, which now delegate into `configuredPolicy`. Task 4 removes that.
+
+**Files:**
+- Create: `core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java`
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java:62-64, 84-104, 116-166`
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java:210-264`
+
+**Interfaces:**
+- Consumes: `DisableParams` from Task 1.
+- Produces: `class UploadPolicy extends DisableParams` with `setMaximumSize(Long)`, `setAllowedTypes(String)`, `setAllowedExtensions(String)`, `getMaximumSize()`, `getAllowedTypes()`, `getAllowedExtensions()`, `copy()`; and `protected boolean acceptFile(UploadPolicy policy, Object action, UploadedFile file, String originalFilename, String contentType, String inputName)`.
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java`, before the closing brace:
+
+```java
+ public void testUploadPolicyParsesAndCopies() {
+ UploadPolicy policy = new UploadPolicy();
+ policy.setAllowedTypes("text/plain, text/html");
+ policy.setAllowedExtensions(".txt,.html");
+ policy.setMaximumSize(1024L);
+ policy.setDisabled("true");
+
+ UploadPolicy copy = policy.copy();
+
+ assertThat(copy.getAllowedTypes()).containsExactlyInAnyOrder("text/plain", "text/html");
+ assertThat(copy.getAllowedExtensions()).containsExactlyInAnyOrder(".txt", ".html");
+ assertThat(copy.getMaximumSize()).isEqualTo(1024L);
+ assertThat(copy.isDisabled()).isTrue();
+ }
+
+ public void testUploadPolicyCopyIsIndependentOfTheOriginal() {
+ UploadPolicy policy = new UploadPolicy();
+ policy.setAllowedTypes("text/plain");
+
+ UploadPolicy copy = policy.copy();
+ copy.setAllowedTypes("text/html");
+
+ assertThat(policy.getAllowedTypes()).containsExactly("text/plain");
+ assertThat(copy.getAllowedTypes()).containsExactly("text/html");
+ }
+
+ public void testUploadPolicyTreatsNullAsNoRestriction() {
+ UploadPolicy policy = new UploadPolicy();
+ policy.setAllowedTypes(null);
+ policy.setAllowedExtensions(null);
+
+ assertThat(policy.getAllowedTypes()).isEmpty();
+ assertThat(policy.getAllowedExtensions()).isEmpty();
+ }
+```
+
+- [ ] **Step 2: Run test to verify it fails**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest#testUploadPolicyParsesAndCopies`
+Expected: compilation failure — `UploadPolicy` does not exist.
+
+- [ ] **Step 3: Create `UploadPolicy`**
+
+Create `core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java` (ASF header first):
+
+```java
+package org.apache.struts2.interceptor;
+
+import org.apache.struts2.util.TextParseUtil;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * Per-invocation upload validation policy for {@link ActionFileUploadInterceptor}.
+ * <p>
+ * A configured instance is held by the interceptor and copied for each invocation, so lazily
+ * resolved values never reach the shared interceptor.
+ *
+ * @since 7.3.0
+ */
+public class UploadPolicy extends DisableParams {
+
+ private Long maximumSize;
+ private Set<String> allowedTypes = Collections.emptySet();
+ private Set<String> allowedExtensions = Collections.emptySet();
+
+ public UploadPolicy() {
+ }
+
+ private UploadPolicy(UploadPolicy other) {
+ super(other);
+ this.maximumSize = other.maximumSize;
+ this.allowedTypes = other.allowedTypes;
+ this.allowedExtensions = other.allowedExtensions;
+ }
+
+ /**
+ * @param allowedTypes a comma-delimited list of content types, or null for no restriction
+ */
+ public void setAllowedTypes(String allowedTypes) {
+ this.allowedTypes = toSet(allowedTypes);
+ }
+
+ /**
+ * @param allowedExtensions a comma-delimited list of extensions, or null for no restriction
+ */
+ public void setAllowedExtensions(String allowedExtensions) {
+ this.allowedExtensions = toSet(allowedExtensions);
+ }
+
+ /**
+ * @param maximumSize the maximum size in bytes, or null for no limit
+ */
+ public void setMaximumSize(Long maximumSize) {
+ this.maximumSize = maximumSize;
+ }
+
+ public Long getMaximumSize() {
+ return maximumSize;
+ }
+
+ public Set<String> getAllowedTypes() {
+ return allowedTypes;
+ }
+
+ public Set<String> getAllowedExtensions() {
+ return allowedExtensions;
+ }
+
+ /**
+ * @return an independent copy, used to seed a per-invocation policy from the configured one
+ */
+ public UploadPolicy copy() {
+ return new UploadPolicy(this);
+ }
+
+ private static Set<String> toSet(String commaDelimited) {
+ return commaDelimited == null
+ ? Collections.emptySet()
+ : TextParseUtil.commaDelimitedStringToSet(commaDelimited);
+ }
+}
+```
+
+Note the null guard: `TextParseUtil.commaDelimitedStringToSet` throws `NullPointerException` on null (`TextParseUtil.java:257` calls `s.split(",")` unguarded), and the old setters had the same defect. Treating null as "no restriction" matches how an empty value already behaves.
+
+- [ ] **Step 4: Rewrite the policy state in `AbstractFileUploadInterceptor`**
+
+Replace the three fields at `:62-64`:
+
+```java
+ private Long maximumSize;
+ private Set<String> allowedTypesSet = Collections.emptySet();
+ private Set<String> allowedExtensionsSet = Collections.emptySet();
+```
+
+with:
+
+```java
+ private final UploadPolicy configuredPolicy = new UploadPolicy();
+```
+
+Replace the three setters at `:84-104` with delegating versions:
+
+```java
+ /**
+ * Sets the allowed extensions. Applied at configuration time only; the effective policy for
+ * an invocation is a copy, see {@link ActionFileUploadInterceptor#newLazyParams()}.
+ *
+ * @param allowedExtensions A comma-delimited list of extensions
+ */
+ public void setAllowedExtensions(String allowedExtensions) {
+ configuredPolicy.setAllowedExtensions(allowedExtensions);
+ }
+
+ /**
+ * Sets the allowed mimetypes. Applied at configuration time only; the effective policy for
+ * an invocation is a copy, see {@link ActionFileUploadInterceptor#newLazyParams()}.
+ *
+ * @param allowedTypes A comma-delimited list of types
+ */
+ public void setAllowedTypes(String allowedTypes) {
+ configuredPolicy.setAllowedTypes(allowedTypes);
+ }
+
+ /**
+ * Sets the maximum size of an uploaded file. Applied at configuration time only; the
+ * effective policy for an invocation is a copy, see
+ * {@link ActionFileUploadInterceptor#newLazyParams()}.
+ *
+ * @param maximumSize The maximum size in bytes
+ */
+ public void setMaximumSize(Long maximumSize) {
+ configuredPolicy.setMaximumSize(maximumSize);
+ }
+
+ /**
+ * @return an independent copy of the configured policy, to be resolved for one invocation
+ * @since 7.3.0
+ */
+ protected UploadPolicy copyConfiguredPolicy() {
+ return configuredPolicy.copy();
+ }
+```
+
+- [ ] **Step 5: Change `acceptFile` to take the policy**
+
+Replace the signature and the three checks at `:116` and `:134-154`:
+
+```java
+ protected boolean acceptFile(UploadPolicy policy, Object action, UploadedFile file, String originalFilename, String contentType, String inputName) {
+```
+
+and inside it:
+
+```java
+ if (policy.getMaximumSize() != null && policy.getMaximumSize() < file.length()) {
+ String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY, new String[]{
+ inputName, originalFilename, file.getName(), "" + file.length(), getMaximumSizeStr(action, policy.getMaximumSize())
+ });
+ errorMessages.add(errMsg);
+ LOG.warn(errMsg);
+ }
+ if ((!policy.getAllowedTypes().isEmpty()) && (!containsItem(policy.getAllowedTypes(), contentType))) {
+ String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY, new String[]{
+ inputName, originalFilename, file.getName(), contentType
+ });
+ errorMessages.add(errMsg);
+ LOG.warn(errMsg);
+ }
+ if ((!policy.getAllowedExtensions().isEmpty()) && (!hasAllowedExtension(policy.getAllowedExtensions(), originalFilename))) {
+ String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY, new String[]{
+ inputName, originalFilename, file.getName(), contentType
+ });
+ errorMessages.add(errMsg);
+ LOG.warn(errMsg);
+ }
+```
+
+Replace `getMaximumSizeStr` at `:164-166`:
+
+```java
+ private String getMaximumSizeStr(Object action, Long maximumSize) {
+ return NumberFormat.getNumberInstance(getLocaleProvider(action).getLocale()).format(maximumSize);
+ }
+```
+
+Remove the now-unused `java.util.Collections` import if the compiler flags it.
+
+- [ ] **Step 6: Update the single caller**
+
+In `ActionFileUploadInterceptor.java`, at the top of `intercept` (`:210`), after the `UploadedFilesAware` check at `:229`, add:
+
+```java
+ UploadPolicy policy = copyConfiguredPolicy();
+```
+
+and change the call at `:248`:
+
+```java
+ if (acceptFile(policy, action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) {
+```
+
+- [ ] **Step 7: Run tests to verify they pass**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest`
+Expected: PASS — the three new `UploadPolicy` tests plus every pre-existing test, unchanged. If a pre-existing test fails, the refactor changed behaviour and must be corrected, not the test.
+
+- [ ] **Step 8: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java \
+ core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java \
+ core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java \
+ core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+git commit -m "WW-5659 refactor(core): hold upload policy in one value object"
+```
+
+---
+
+### Task 4: New `WithLazyParams` contract and invocation wiring
+
+The behaviour change. After this task the interceptor singleton is never written per request.
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java`
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java`
+- Modify: `core/src/main/java/org/apache/struts2/DefaultActionInvocation.java:258-276`
+- Test: `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java`
+
+**Interfaces:**
+- Consumes: `resolveInto` (Task 2), `UploadPolicy` and `copyConfiguredPolicy()` (Task 3).
+- Produces: `interface WithLazyParams<P extends InterceptorParams> { P newLazyParams(); String intercept(ActionInvocation, P) throws Exception; }`; `ActionFileUploadInterceptor implements WithLazyParams<UploadPolicy>`.
+
+- [ ] **Step 1: Write the failing tests**
+
+Append to `ActionFileUploadInterceptorTest`, before the closing brace. The first test is carried over from PR #1815 by @deprrous, adapted to the new contract — keep the attribution comment.
+
+```java
+ /**
+ * Regression for WW-5659: two concurrent invocations resolving different policies must not
+ * see each other's values. Scenario contributed by @deprrous in GitHub PR #1815.
+ */
+ public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Exception {
+ CoordinatedActionFileUploadInterceptor sharedInterceptor = new CoordinatedActionFileUploadInterceptor();
+ container.inject(sharedInterceptor);
+
+ MyDynamicFileUploadAction plainPolicyAction = new MyDynamicFileUploadAction();
+ plainPolicyAction.setAllowedMimeTypes("text/plain");
+ container.inject(plainPolicyAction);
+
+ MyDynamicFileUploadAction htmlPolicyAction = new MyDynamicFileUploadAction();
+ htmlPolicyAction.setAllowedMimeTypes("text/html");
+ container.inject(htmlPolicyAction);
+
+ ExecutorService executor = Executors.newFixedThreadPool(2);
+ try {
+ Future<String> plainResult = executor.submit(() -> runUploadAttempt(
+ sharedInterceptor, plainPolicyAction, createUploadRequest("plain-policy.html", "text/html", htmlContent)));
+
+ assertThat(sharedInterceptor.awaitFirstValidation()).isTrue();
+
+ Future<String> htmlResult = executor.submit(() -> runUploadAttempt(
+ sharedInterceptor, htmlPolicyAction, createUploadRequest("html-policy.html", "text/html", htmlContent)));
+
+ assertThat(htmlResult.get(10, TimeUnit.SECONDS)).isEqualTo("success");
+ sharedInterceptor.releaseFirstValidation();
+ assertThat(plainResult.get(10, TimeUnit.SECONDS)).isEqualTo("success");
+ } finally {
+ sharedInterceptor.releaseFirstValidation();
+ executor.shutdownNow();
+ sharedInterceptor.destroy();
+ }
+
+ // the text/plain policy must have rejected the text/html upload despite the concurrent
+ // text/html invocation resolving a more permissive policy on the same interceptor
+ assertThat(plainPolicyAction.getUploadFiles()).isNull();
+ assertThat(plainPolicyAction.getFieldErrors()).containsKey("file");
+
+ assertThat(htmlPolicyAction.hasFieldErrors()).isFalse();
+ assertThat(htmlPolicyAction.getUploadFiles()).isNotNull().hasSize(1);
+ assertThat(htmlPolicyAction.getUploadFiles().get(0).getOriginalName()).isEqualTo("html-policy.html");
+ }
+
+ /**
+ * Regression for WW-5659: resolving an invocation's params must leave the interceptor
+ * singleton exactly as configured.
+ */
+ public void testResolutionDoesNotMutateTheInterceptor() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+ interceptor.setAllowedTypes("text/plain");
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes("text/html");
+ container.inject(action);
+
+ runUploadAttempt(interceptor, action, createUploadRequest("f.html", "text/html", htmlContent));
+
+ assertThat(interceptor.newLazyParams().getAllowedTypes()).containsExactly("text/plain");
+ }
+
+ /**
+ * Regression for WW-5659: a lazily resolved {@code disabled} must apply to one invocation only.
+ */
+ public void testDisabledIsResolvedPerInvocation() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes("text/plain");
+ container.inject(action);
+
+ UploadPolicy policy = interceptor.newLazyParams();
+ policy.setDisabled("true");
+
+ assertThat(policy.isDisabled()).isTrue();
+ assertThat(interceptor.newLazyParams().isDisabled()).isFalse();
+ }
+```
+
+Add these helpers — none of them exist in the test class yet. `runUploadAttempt`, `createUploadRequest`, the `MockHttpServletRequest` overload of `createMultipartRequest` and `CoordinatedActionFileUploadInterceptor` all originate in PR #1815 by @deprrous and are reproduced here, adapted to the new contract, so this task is self-contained:
+
+```java
+ private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInterceptor,
+ MyDynamicFileUploadAction action,
+ MockHttpServletRequest uploadRequest) throws Exception {
+ MultiPartRequestWrapper multiPartRequest = createMultipartRequest(uploadRequest, -1, -1, 3, -1);
+ ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack();
+ valueStack.push(action);
+
+ ActionContext context = ActionContext.of(valueStack.getContext())
+ .withContainer(container)
+ .withValueStack(valueStack)
+ .withServletRequest(multiPartRequest)
+ .bind();
+ try {
+ MockActionInvocation invocation = new MockActionInvocation();
+ invocation.setAction(action);
+ invocation.setResultCode("success");
+ invocation.setInvocationContext(context);
+
+ Map<String, String> params = new HashMap<>();
+ params.put("allowedTypes", "${allowedMimeTypes}");
+
+ WithLazyParams.LazyParamInjector injector = new WithLazyParams.LazyParamInjector(valueStack);
+ container.inject(injector);
+ UploadPolicy policy = injector.resolveInto(actionFileUploadInterceptor.newLazyParams(), params, context);
+
+ return actionFileUploadInterceptor.intercept(invocation, policy);
+ } finally {
+ ActionContext.clear();
+ }
+ }
+
+ private MockHttpServletRequest createUploadRequest(String filename, String contentType, String content) {
+ MockHttpServletRequest uploadRequest = new MockHttpServletRequest();
+ uploadRequest.setCharacterEncoding(StandardCharsets.UTF_8.name());
+ uploadRequest.setMethod("POST");
+ uploadRequest.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\"");
+ uploadRequest.setContent((encodeTextFile(filename, contentType, content) + endLine + "--" + boundary + "--")
+ .getBytes(StandardCharsets.UTF_8));
+ return uploadRequest;
+ }
+
+ private MultiPartRequestWrapper createMultipartRequest(MockHttpServletRequest multipartRequest, int maxsize, int maxfilesize, int maxfiles, int maxStringLength) {
+ JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
+ jak.setMaxSize(String.valueOf(maxsize));
+ jak.setMaxFileSize(String.valueOf(maxfilesize));
+ jak.setMaxFiles(String.valueOf(maxfiles));
+ jak.setMaxStringLength(String.valueOf(maxStringLength));
+ jak.setDefaultEncoding(StandardCharsets.UTF_8.name());
+ return new MultiPartRequestWrapper(jak, multipartRequest, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
+ }
+
+ /** Pauses the first validation so a second invocation can overlap it. From PR #1815 by @deprrous. */
+ private static final class CoordinatedActionFileUploadInterceptor extends ActionFileUploadInterceptor {
+ private final AtomicBoolean pauseFirstValidation = new AtomicBoolean(true);
+ private final CountDownLatch firstValidationEntered = new CountDownLatch(1);
+ private final CountDownLatch allowFirstValidationToContinue = new CountDownLatch(1);
+
+ @Override
+ protected boolean acceptFile(UploadPolicy policy, Object action, UploadedFile file, String originalFilename, String contentType, String inputName) {
+ if (pauseFirstValidation.compareAndSet(true, false)) {
+ firstValidationEntered.countDown();
+ awaitUnchecked(allowFirstValidationToContinue);
+ }
+ return super.acceptFile(policy, action, file, originalFilename, contentType, inputName);
+ }
+
+ private boolean awaitFirstValidation() throws InterruptedException {
+ return firstValidationEntered.await(10, TimeUnit.SECONDS);
+ }
+
+ private void releaseFirstValidation() {
+ allowFirstValidationToContinue.countDown();
+ }
+
+ private void awaitUnchecked(CountDownLatch latch) {
+ try {
+ if (!latch.await(10, TimeUnit.SECONDS)) {
+ throw new AssertionError("Timed out waiting for concurrent validation release");
+ }
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new AssertionError("Interrupted while waiting for concurrent validation release", e);
+ }
+ }
+ }
+```
+
+To avoid duplicating the body, change the existing no-request `createMultipartRequest(int, int, int, int)` to delegate to the new overload:
+
+```java
+ private MultiPartRequestWrapper createMultipartRequest(int maxsize, int maxfilesize, int maxfiles, int maxStringLength) {
+ return createMultipartRequest(request, maxsize, maxfilesize, maxfiles, maxStringLength);
+ }
+```
+
+Required test imports:
+
+```java
+import org.apache.struts2.util.ValueStack;
+import org.apache.struts2.util.ValueStackFactory;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+```
+
+- [ ] **Step 2: Run tests to verify they fail**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest#testResolutionDoesNotMutateTheInterceptor`
+Expected: compilation failure — `newLazyParams()` and the two-arg `intercept` do not exist.
+
+- [ ] **Step 3: Change the `WithLazyParams` interface**
+
+Replace the interface declaration in `WithLazyParams.java` (keep `LazyParamInjector` nested inside it unchanged apart from the deletion below):
+
+```java
+public interface WithLazyParams<P extends InterceptorParams> {
+
+ /**
+ * @return a fresh holder for one invocation, seeded from the configured values
+ * @since 7.3.0
+ */
+ P newLazyParams();
+
+ /**
+ * Invoked in place of {@link Interceptor#intercept(ActionInvocation)} when lazy params apply.
+ *
+ * @param lazyParams params resolved for this invocation only
+ * @since 7.3.0
+ */
+ String intercept(ActionInvocation invocation, P lazyParams) throws Exception;
+```
+
+Add the import `org.apache.struts2.ActionInvocation` and delete the old `injectParams` method entirely.
+
+- [ ] **Step 4: Implement the contract on the interceptor**
+
+In `ActionFileUploadInterceptor.java`, change the class declaration:
+
+```java
+public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor implements WithLazyParams<UploadPolicy> {
+```
+
+Add, above `intercept`:
+
+```java
+ @Override
+ public UploadPolicy newLazyParams() {
+ return copyConfiguredPolicy();
+ }
+
+ @Override
+ public String intercept(ActionInvocation invocation) throws Exception {
+ return intercept(invocation, newLazyParams());
+ }
+```
+
+Change the existing `intercept` to the two-arg form, replacing the `UploadPolicy policy = copyConfiguredPolicy();` line added in Task 3:
+
+```java
+ @Override
+ public String intercept(ActionInvocation invocation, UploadPolicy policy) throws Exception {
+```
+
+- [ ] **Step 5: Wire `DefaultActionInvocation`**
+
+Replace `:258-276` with:
+
+```java
+ if (interceptors.hasNext()) {
+ final InterceptorMapping interceptorMapping = interceptors.next();
+ Interceptor interceptor = interceptorMapping.getInterceptor();
+ if (interceptor instanceof WithLazyParams<?> lazyInterceptor) {
+ resultCode = invokeWithLazyParams(lazyInterceptor, interceptorMapping);
+ } else if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) {
+ resultCode = executeConditional(conditionalInterceptor);
+ } else {
+ LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName());
+ resultCode = interceptor.intercept(this);
+ }
+ } else {
+ resultCode = invokeActionOnly();
+ }
+```
+
+Add these two methods next to `executeConditional`:
+
+```java
+ /**
+ * Resolves lazy params into a per-invocation holder and dispatches to the interceptor.
+ * <p>
+ * {@link org.apache.struts2.interceptor.AbstractInterceptor} implements
+ * {@link ConditionalInterceptor}, so a lazy interceptor is normally conditional too; both the
+ * lazily resolved {@code disabled} flag and any custom {@code shouldIntercept} must be honoured
+ * here, because the single-argument {@code intercept} is not the entry point on this path.
+ */
+ private <P extends org.apache.struts2.interceptor.InterceptorParams> String invokeWithLazyParams(
+ WithLazyParams<P> lazyInterceptor, InterceptorMapping interceptorMapping) throws Exception {
+ P lazyParams = lazyParamInjector.resolveInto(
+ lazyInterceptor.newLazyParams(), mergedParams(interceptorMapping), invocationContext);
+
+ if (lazyParams instanceof org.apache.struts2.interceptor.DisableParams disableParams && disableParams.isDisabled()) {
+ LOG.debug("Interceptor: {} is disabled for this invocation, skipping to next", interceptorMapping.getName());
+ return this.invoke();
+ }
+ if (lazyInterceptor instanceof ConditionalInterceptor conditionalInterceptor
+ && !conditionalInterceptor.shouldIntercept(this)) {
+ LOG.debug("Interceptor: {} is disabled, skipping to next", interceptorMapping.getName());
+ return this.invoke();
+ }
+ LOG.debug("Executing lazy params interceptor: {}", interceptorMapping.getName());
+ return lazyInterceptor.intercept(this, lazyParams);
+ }
+
+ /**
+ * @return a fresh map; the mapping's own param map is shared across requests and must not be mutated
+ */
+ private Map<String, String> mergedParams(InterceptorMapping interceptorMapping) {
+ Map<String, String> merged = new HashMap<>(interceptorMapping.getParams());
+ proxy.getConfig().getInterceptors().stream()
+ .filter(im -> im.getName().equals(interceptorMapping.getName()))
+ .findFirst()
+ .ifPresent(im -> merged.putAll(im.getParams()));
+ return merged;
+ }
+```
+
+Add `import java.util.HashMap;` if not present.
+
+- [ ] **Step 6: Run the new tests**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest`
+Expected: PASS, including the three new regressions.
+
+- [ ] **Step 7: Run the wider suite**
+
+Run: `mvn test -DskipAssembly -pl core`
+Expected: PASS. `DefaultActionInvocationTest` exercises the rewritten `invoke()` branch and must stay green.
+
+- [ ] **Step 8: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java \
+ core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java \
+ core/src/main/java/org/apache/struts2/DefaultActionInvocation.java \
+ core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+git commit -m "WW-5659 fix(core): resolve lazy interceptor params per invocation
+
+Co-Authored-By: deprrous <su...@gmail.com>"
+```
+
+---
+
+### Task 5: Fail closed on unresolvable expressions
+
+**Files:**
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java`
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java`
+- Modify: `core/src/main/resources/org/apache/struts2/struts-messages.properties`
+- Test: `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java`
+
+**Interfaces:**
+- Consumes: `InterceptorParams.unresolved(String)` (Task 1), `resolveInto` (Task 2), `UploadPolicy` (Task 3).
+- Produces: `UploadPolicy.isUnresolved()`, `UploadPolicy.getUnresolvedParams()`; constant `STRUTS_MESSAGES_ERROR_UPLOAD_POLICY_UNRESOLVED_KEY`.
+
+Implements the deterministic rule from the *Deviation from the spec* section: any param reported unresolved marks the policy unusable, regardless of the seeded value.
+
+- [ ] **Step 1: Write the failing test**
+
+Append to `ActionFileUploadInterceptorTest`:
+
+```java
+ public void testUnresolvedPolicyRejectsTheUpload() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes(null); // ${allowedMimeTypes} will not resolve
+ container.inject(action);
+
+ runUploadAttempt(interceptor, action, createUploadRequest("f.txt", "text/plain", plainContent));
+
+ assertThat(action.getUploadFiles()).isNull();
+ assertThat(action.getFieldErrors()).containsKey("file");
+ }
+
+ public void testResolvedPolicyStillAcceptsTheUpload() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes("text/plain");
+ container.inject(action);
+
+ runUploadAttempt(interceptor, action, createUploadRequest("f.txt", "text/plain", plainContent));
+
+ assertThat(action.hasFieldErrors()).isFalse();
+ assertThat(action.getUploadFiles()).isNotNull().hasSize(1);
+ }
+
+ public void testUploadPolicyTracksUnresolvedParams() {
+ UploadPolicy policy = new UploadPolicy();
+ assertThat(policy.isUnresolved()).isFalse();
+
+ policy.unresolved("allowedTypes");
+
+ assertThat(policy.isUnresolved()).isTrue();
+ assertThat(policy.getUnresolvedParams()).containsExactly("allowedTypes");
+ }
+```
+
+- [ ] **Step 2: Run tests to verify they fail**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest#testUploadPolicyTracksUnresolvedParams`
+Expected: compilation failure — `isUnresolved()` does not exist.
+
+- [ ] **Step 3: Track unresolved params on the policy**
+
+In `UploadPolicy.java` add the field, copy it, and override `unresolved`:
+
+```java
+ private final Set<String> unresolvedParams = new LinkedHashSet<>();
+```
+
+In the copy constructor add:
+
+```java
+ this.unresolvedParams.addAll(other.unresolvedParams);
+```
+
+and add:
+
+```java
+ /**
+ * A parameter that could not be resolved makes this policy unusable: the upload is rejected
+ * rather than validated against a partially-resolved policy, so a broken expression cannot
+ * silently relax validation.
+ */
+ @Override
+ public void unresolved(String paramName) {
+ unresolvedParams.add(paramName);
+ }
+
+ public boolean isUnresolved() {
+ return !unresolvedParams.isEmpty();
+ }
+
+ public Set<String> getUnresolvedParams() {
+ return Collections.unmodifiableSet(unresolvedParams);
+ }
+```
+
+Add imports `java.util.LinkedHashSet`.
+
+- [ ] **Step 4: Reject in `acceptFile`**
+
+In `AbstractFileUploadInterceptor.java` add the key constant next to the others at `:47-60`:
+
+```java
+ public static final String STRUTS_MESSAGES_ERROR_UPLOAD_POLICY_UNRESOLVED_KEY = "struts.messages.error.upload.policy.unresolved";
+```
+
+In `acceptFile`, immediately after the existing missing-file check that returns false at `:125-132`, add:
+
+```java
+ if (policy.isUnresolved()) {
+ String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOAD_POLICY_UNRESOLVED_KEY, new String[]{
+ inputName, originalFilename, String.join(", ", policy.getUnresolvedParams())
+ });
+ if (validation != null) {
+ validation.addFieldError(inputName, errMsg);
+ }
+ LOG.warn(errMsg);
+ return false;
+ }
+```
+
+- [ ] **Step 5: Add the message**
+
+Append to `core/src/main/resources/org/apache/struts2/struts-messages.properties`:
+
+```properties
+struts.messages.error.upload.policy.unresolved=The upload validation policy could not be resolved, rejecting the file: {0} "{1}"; unresolved parameters: {2}
+```
+
+Base bundle only — the locale bundles are partial and fall back to it.
+
+- [ ] **Step 6: Run tests to verify they pass**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest`
+Expected: PASS.
+
+- [ ] **Step 7: Commit**
+
+```bash
+git add core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java \
+ core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java \
+ core/src/main/resources/org/apache/struts2/struts-messages.properties \
+ core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+git commit -m "WW-5659 fix(core): reject uploads when the policy cannot be resolved"
+```
+
+---
+
+### Task 6: Migrate legacy tests, javadoc, and spec alignment
+
+**Files:**
+- Modify: `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java`
+- Modify: `core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java:70-204` (class javadoc)
+- Modify: `docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md`
+
+**Interfaces:**
+- Consumes: everything from Tasks 1–5.
+- Produces: nothing new.
+
+- [ ] **Step 1: Migrate the setter-shortcut tests**
+
+Five tests simulate injection by calling a setter directly — `testDynamicParameterEvaluation`, `testDynamicParametersChangePerRequest`, `testDynamicExtensionValidation`, `testDynamicMaximumSizeValidation`, `testSecurityValidationWithDynamicParameters`, `testWildcardMatchingWithDynamicParameters`. Each contains a line of the form `interceptor.setAllowedTypes(action.getAllowedMimeTypes());` followed by `interceptor.intercept(mai);`.
+
+Replace each such pair with a real resolution through the injector, e.g.:
+
+```java
+ UploadPolicy policy = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false);
+ interceptor.intercept(mai, policy);
+```
+
+and add the helper:
+
+```java
+ private UploadPolicy injectDynamicUploadPolicy(ActionFileUploadInterceptor actionFileUploadInterceptor,
+ ActionContext context,
+ boolean includeAllowedTypes,
+ boolean includeAllowedExtensions,
+ boolean includeMaximumSize) {
+ Map<String, String> params = new HashMap<>();
+ if (includeAllowedTypes) {
+ params.put("allowedTypes", "${allowedMimeTypes}");
+ }
+ if (includeAllowedExtensions) {
+ params.put("allowedExtensions", "${allowedExtensions}");
+ }
+ if (includeMaximumSize) {
+ params.put("maximumSize", "${maxFileSize}");
+ }
+
+ WithLazyParams.LazyParamInjector injector = new WithLazyParams.LazyParamInjector(context.getValueStack());
+ container.inject(injector);
+ return injector.resolveInto(actionFileUploadInterceptor.newLazyParams(), params, context);
+ }
+```
+
+Set the flags per test to match the param that test previously set by hand: `allowedTypes` for the four type/wildcard tests, `allowedExtensions` for `testDynamicExtensionValidation`, `maximumSize` for `testDynamicMaximumSizeValidation`, and both types and extensions for `testSecurityValidationWithDynamicParameters`.
+
+- [ ] **Step 2: Run the suite**
+
+Run: `mvn test -DskipAssembly -pl core -Dtest=ActionFileUploadInterceptorTest`
+Expected: PASS. These tests now exercise the real resolution path rather than a hand-called setter.
+
+- [ ] **Step 3: Update the interceptor javadoc**
+
+`ActionFileUploadInterceptor`'s class javadoc at `:70-204` documents the dynamic-parameter feature with a `uploadConfig.setAllowedExtensions(".jpg,.png")` example. Add a paragraph directly above `@see WithLazyParams`:
+
+```java
+ * <p>
+ * Dynamic parameters are resolved into a fresh {@link UploadPolicy} for each invocation, so the
+ * interceptor itself is never modified per request and concurrent uploads cannot observe each
+ * other's policy. An expression that cannot be resolved does not relax validation: the policy is
+ * marked unresolved and affected uploads are rejected.
+```
+
+- [ ] **Step 4: Align the spec with the implemented rule**
+
+In the spec's *Error handling* section, replace the two-case "genuine static fallback / no fallback" rule and the sentence beginning "The rule: `unresolved(param)` marks the dimension unusable **only if**..." with:
+
+```markdown
+`UploadPolicy.unresolved(param)` records the parameter and marks the whole policy unusable,
+regardless of the seeded value. A static fallback therefore applies only when the param is absent
+from the lazy map entirely, i.e. pure static configuration, which never triggers `unresolved`.
+
+The seed-introspection alternative — honouring a seeded value that does not itself contain
+`${` — was rejected during planning: it is not implementable deterministically, because
+`maximumSize` is seeded as a `Long` and cannot carry a `${...}` literal, so the rule would behave
+differently per parameter type.
+```
+
+Update the corresponding Testing bullet to drop the static-fallback case.
+
+- [ ] **Step 5: Full build**
+
+Run: `mvn test -DskipAssembly`
+Expected: PASS across all modules. Plugins do not implement `WithLazyParams` (verified: `ActionFileUploadInterceptor` is the only implementer), but the interface change is compile-visible, so a full build is the check.
+
+- [ ] **Step 6: Commit**
+
+```bash
+git add core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java \
+ core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java \
+ docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
+git commit -m "WW-5659 test(core): exercise real lazy param resolution in dynamic upload tests"
+```
+
+---
+
+## Done criteria
+
+- `mvn test -DskipAssembly` passes across all modules.
+- `ActionFileUploadInterceptor` has no mutable state written after `init()`; `testResolutionDoesNotMutateTheInterceptor` proves it.
+- No `ThreadLocal`, no `finally` cleanup, and no per-request write to any shared map remains on the lazy path.
+- The spec and the implementation agree on the unresolved-param rule.
(struts) 10/13: WW-5659 fix(core): reject uploads when the policy cannot be resolved
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 9d66a7b9f44953c2adadee4d8f892fa486ecb507
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 11:31:30 2026 +0200
WW-5659 fix(core): reject uploads when the policy cannot be resolved
---
.../interceptor/AbstractFileUploadInterceptor.java | 12 +++++++
.../apache/struts2/interceptor/UploadPolicy.java | 21 ++++++++++++
.../org/apache/struts2/struts-messages.properties | 4 +++
.../ActionFileUploadInterceptorTest.java | 38 ++++++++++++++++++++++
4 files changed, 75 insertions(+)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
index f8529732b..dac450b45 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java
@@ -56,6 +56,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
public static final String STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY = "struts.messages.invalid.content.type";
public static final String STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY = "struts.messages.error.content.type.not.allowed";
public static final String STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY = "struts.messages.error.file.extension.not.allowed";
+ public static final String STRUTS_MESSAGES_ERROR_UPLOAD_POLICY_UNRESOLVED_KEY = "struts.messages.error.upload.policy.unresolved";
private final UploadPolicy configuredPolicy = new UploadPolicy();
@@ -140,6 +141,17 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor
return false;
}
+ if (policy.isUnresolved()) {
+ String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOAD_POLICY_UNRESOLVED_KEY, new String[]{
+ inputName, originalFilename, String.join(", ", policy.getUnresolvedParams())
+ });
+ if (validation != null) {
+ validation.addFieldError(inputName, errMsg);
+ }
+ LOG.warn(errMsg);
+ return false;
+ }
+
if (policy.getMaximumSize() != null && policy.getMaximumSize() < file.length()) {
String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY, new String[]{
inputName, originalFilename, file.getName(), "" + file.length(), getMaximumSizeStr(action, policy.getMaximumSize())
diff --git a/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java b/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
index a9e444157..08702dee4 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/UploadPolicy.java
@@ -21,6 +21,7 @@ package org.apache.struts2.interceptor;
import org.apache.struts2.util.TextParseUtil;
import java.util.Collections;
+import java.util.LinkedHashSet;
import java.util.Set;
/**
@@ -36,6 +37,7 @@ public class UploadPolicy extends DisableParams {
private Long maximumSize;
private Set<String> allowedTypes = Collections.emptySet();
private Set<String> allowedExtensions = Collections.emptySet();
+ private final Set<String> unresolvedParams = new LinkedHashSet<>();
public UploadPolicy() {
}
@@ -45,6 +47,7 @@ public class UploadPolicy extends DisableParams {
this.maximumSize = other.maximumSize;
this.allowedTypes = other.allowedTypes;
this.allowedExtensions = other.allowedExtensions;
+ this.unresolvedParams.addAll(other.unresolvedParams);
}
/**
@@ -80,6 +83,24 @@ public class UploadPolicy extends DisableParams {
return allowedExtensions;
}
+ /**
+ * A parameter that could not be resolved makes this policy unusable: the upload is rejected
+ * rather than validated against a partially-resolved policy, so a broken expression cannot
+ * silently relax validation.
+ */
+ @Override
+ public void unresolved(String paramName) {
+ unresolvedParams.add(paramName);
+ }
+
+ public boolean isUnresolved() {
+ return !unresolvedParams.isEmpty();
+ }
+
+ public Set<String> getUnresolvedParams() {
+ return Collections.unmodifiableSet(unresolvedParams);
+ }
+
/**
* @return an independent copy, used to seed a per-invocation policy from the configured one
*/
diff --git a/core/src/main/resources/org/apache/struts2/struts-messages.properties b/core/src/main/resources/org/apache/struts2/struts-messages.properties
index 63514b000..ecd99000b 100644
--- a/core/src/main/resources/org/apache/struts2/struts-messages.properties
+++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties
@@ -50,6 +50,10 @@ struts.messages.error.content.type.not.allowed=Content-Type not allowed: {0} "{1
# 2 - file name after uploading the file
# 3 - content type of the file
struts.messages.error.file.extension.not.allowed=File extension not allowed: {0} "{1}" "{2}" {3}
+# 0 - input name
+# 1 - original filename
+# 2 - comma-delimited list of unresolved parameter names
+struts.messages.error.upload.policy.unresolved=The upload validation policy could not be resolved, rejecting the file: {0} "{1}"; unresolved parameters: {2}
# dedicated messages used to handle various problems with file upload - check {@link JakartaMultiPartRequest#parse(HttpServletRequest, String)}
# params depend on exception being handled
# FileUploadByteCountLimitException
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index e9bca76cc..ab31c3d18 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -1160,4 +1160,42 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
}
}
+ public void testUnresolvedPolicyRejectsTheUpload() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes(null); // ${allowedMimeTypes} will not resolve
+ container.inject(action);
+
+ runUploadAttempt(interceptor, action, createUploadRequest("f.txt", "text/plain", plainContent));
+
+ assertThat(action.getUploadFiles()).isNull();
+ assertThat(action.getFieldErrors()).containsKey("file");
+ }
+
+ public void testResolvedPolicyStillAcceptsTheUpload() throws Exception {
+ ActionFileUploadInterceptor interceptor = new ActionFileUploadInterceptor();
+ container.inject(interceptor);
+
+ MyDynamicFileUploadAction action = new MyDynamicFileUploadAction();
+ action.setAllowedMimeTypes("text/plain");
+ container.inject(action);
+
+ runUploadAttempt(interceptor, action, createUploadRequest("f.txt", "text/plain", plainContent));
+
+ assertThat(action.hasFieldErrors()).isFalse();
+ assertThat(action.getUploadFiles()).isNotNull().hasSize(1);
+ }
+
+ public void testUploadPolicyTracksUnresolvedParams() {
+ UploadPolicy policy = new UploadPolicy();
+ assertThat(policy.isUnresolved()).isFalse();
+
+ policy.unresolved("allowedTypes");
+
+ assertThat(policy.isUnresolved()).isTrue();
+ assertThat(policy.getUnresolvedParams()).containsExactly("allowedTypes");
+ }
+
}
(struts) 01/13: WW-5659 docs: design for request-scoped lazy interceptor params
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 08c9ac40c4ebd80c261c90bf81d439c10d0053d5
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 10:01:09 2026 +0200
WW-5659 docs: design for request-scoped lazy interceptor params
WithLazyParams#injectParams resolves ${...} params onto the interceptor
singleton, so concurrent requests can read one another's resolved values.
For ActionFileUploadInterceptor that means allowedTypes, allowedExtensions,
maximumSize and disabled can cross between requests.
Design fixes the contract rather than the one implementer: resolved params
go into a per-invocation holder the interceptor supplies and receives back,
leaving the singleton immutable after init(). Adds InterceptorParams as the
general contract with DisableParams as opt-in support for the disabled param,
and makes unresolvable expressions fail closed instead of silently disabling
validation.
Reported via GitHub PR #1815; that approach (ThreadLocal on the interceptor)
is not adopted.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
...7-WW-5659-lazy-params-request-scoping-design.md | 343 +++++++++++++++++++++
1 file changed, 343 insertions(+)
diff --git a/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md b/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
new file mode 100644
index 000000000..20e8a7c96
--- /dev/null
+++ b/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
@@ -0,0 +1,343 @@
+# WW-5659: Request-scoped resolution of lazy interceptor params
+
+**Jira:** [WW-5659](https://issues.apache.org/jira/browse/WW-5659)
+**Type:** Bug
+**Affects:** 7.2.0, 7.2.1
+**Fix version:** 7.3.0
+**Date:** 2026-07-27
+**Reported via:** GitHub PR [#1815](https://github.com/apache/struts/pull/1815) (approach not adopted)
+
+## Problem
+
+`WithLazyParams.LazyParamInjector#injectParams` resolves `${...}` interceptor params
+once per request and writes the resolved values straight onto the interceptor:
+
+```java
+// WithLazyParams.java:78-84
+public Interceptor injectParams(Interceptor interceptor, Map<String, String> params, ActionContext invocationContext) {
+ for (Map.Entry<String, String> entry : params.entrySet()) {
+ Object paramValue = textParser.evaluate(new char[]{'$'}, entry.getValue(), valueEvaluator, TextParser.DEFAULT_LOOP_COUNT);
+ ognlUtil.setProperty(entry.getKey(), paramValue, interceptor, invocationContext.getContextMap());
+ }
+ return interceptor;
+}
+```
+
+That interceptor is a singleton, built once at configuration-parse time
+(`InterceptorBuilder.java:73-74`, `:175-177`) and reused for every request. When an
+action references a stack without overriding params, `InterceptorBuilder.java:79`
+(`result.addAll(stackConfig.getInterceptors())`) hands the same `InterceptorMapping`
+objects to every such action, so the instance is shared across actions too.
+
+Request-scoped state is therefore written to process-wide state with no synchronisation.
+
+### Consequence for file upload validation
+
+`ActionFileUploadInterceptor` is the only `WithLazyParams` implementer today. Its
+resolved policy lands in plain instance fields (`AbstractFileUploadInterceptor.java:62-64`,
+written at `:85`, `:94`, `:103`) and is read back by `acceptFile` (`:134`, `:141`, `:148`).
+
+Per request:
+
+- `DefaultActionInvocation.java:269` — resolve, write the shared fields
+- `DefaultActionInvocation.java:275` — `intercept()` → `acceptFile()` reads the shared fields
+
+Nothing guards the interval. Two concurrent requests resolving different policies can have
+their `allowedTypes`, `allowedExtensions` and `maximumSize` cross over, so a request can be
+validated against another request's upload policy.
+
+### `disabled` is affected too
+
+`AbstractInterceptor.java:28` holds `disabled` as a shared `boolean`, written by
+`setDisabled` (`:56-58`) and read by `shouldIntercept` (`:61-63`). It travels through
+`injectParams` like any other param, so `<param name="disabled">${...}</param>` has the
+same cross-request bleed — and it decides whether the interceptor runs at all.
+
+Scope note: for interceptors that are *not* `WithLazyParams`, `setDisabled` is only ever
+called at config time by `buildInterceptor`, never per request. `disabled` is racy only on
+the lazy path.
+
+### Secondary defect
+
+`DefaultActionInvocation.java:262-267` calls `params.putAll(...)` on the map returned by
+`InterceptorMapping#getParams` (`:60-62`), which is the live shared map — an unsynchronised
+write to a shared `HashMap` on every request.
+
+### Not affected
+
+`struts-default.xml:60` declares `actionFileUpload` with no params, so a default
+configuration resolves nothing and writes nothing. Static (non-expression) params resolve
+to the same value on every request. Only applications opting into the dynamic `${...}`
+form added in WW-5585 are affected.
+
+`actionFileUpload` sits at position 15 of `defaultStack`, ahead of `staticParams` (19),
+`actionMappingParams` (20) and `params` (21), so no request parameter has been bound to the
+action at resolution time — the expression's *value* is not attacker-supplied. Which policy
+is selected can still depend on request-derived state populated by `prepare` (11),
+`scopedModelDriven` (13) or `modelDriven` (14); the shipped showcase does exactly that in
+`DynamicFileUploadAction.java:136-139`.
+
+This was assessed as a thread-safety defect rather than a framework vulnerability. Released
+7.2.0 and 7.2.1 are not being backported.
+
+## Decision
+
+Fix the `WithLazyParams` contract rather than patching its one implementer, so no future
+implementer can reintroduce the bug. Resolved params are written into a per-invocation
+object the interceptor supplies and receives back; the interceptor singleton stays
+immutable after `init()`.
+
+Rejected alternatives:
+
+- **Per-invocation copy of the interceptor** (`copyForInvocation()`): smallest diff, but a
+ copy constructor rots silently when a field is added, it copies injected container
+ dependencies, and it creates transient instances that never receive `destroy()`. It hides
+ the mutable snapshot rather than separating config-time from request-time state.
+- **Interceptor-owned resolution** (framework stops injecting, interceptor resolves raw
+ templates itself): loses OGNL type conversion, duplicates resolution in every
+ implementer, and silently changes what the setters mean.
+- **`ThreadLocal` on the interceptor** (PR #1815): makes setter behaviour depend on hidden
+ thread state, requires a cleanup call that any implementer can forget, and leaves the
+ unsafe write path in place, merely bypassed.
+
+This is a clean interface break in the next minor. `WithLazyParams` is public API since
+2.5.9, but `ActionFileUploadInterceptor` is its only implementer in the repo; third-party
+implementers get a compile error rather than silent breakage, noted in the migration guide.
+
+## Components
+
+### `InterceptorParams` (new)
+
+`org.apache.struts2.interceptor.InterceptorParams` — the general contract for a params
+holder, and the generic bound for `WithLazyParams`.
+
+```java
+public interface InterceptorParams {
+ /** Notified when a {@code ${...}} param could not be resolved for this invocation. */
+ default void unresolved(String paramName) { }
+}
+```
+
+The default no-op keeps the interface free for implementers that do not care. `UploadPolicy`
+overrides it to support fail-closed validation.
+
+### `DisableParams` (new)
+
+`org.apache.struts2.interceptor.DisableParams` — opt-in support for the `disabled` param.
+A class, not an interface, because it holds state and needs a setter for OGNL.
+
+```java
+public class DisableParams implements InterceptorParams {
+ private boolean disabled;
+ public void setDisabled(String disable) { this.disabled = Boolean.parseBoolean(disable); }
+ public boolean isDisabled() { return disabled; }
+}
+```
+
+`disabled` is universal across interceptors while lazy resolution is rare, so `DisableParams`
+is *not* a subtype of any lazy-specific type; both sit under `InterceptorParams`.
+
+A holder that does not extend `DisableParams` has no `disabled` property, so a `disabled`
+param on such an interceptor resolves to nothing and is reported by the unknown-param
+warning below. There is deliberately no fallback to the singleton — that is the racy path
+being removed. Interceptors supporting `disabled` must extend `DisableParams`.
+
+### `WithLazyParams` (changed)
+
+```java
+public interface WithLazyParams<P extends InterceptorParams> {
+ P newLazyParams();
+ String intercept(ActionInvocation invocation, P lazyParams) throws Exception;
+}
+```
+
+`AbstractInterceptor.java:48` declares `intercept(ActionInvocation)` abstract, and a
+class-declared abstract method takes precedence over an interface default, so a `default`
+single-arg implementation on this interface would not satisfy it. Implementers write the
+one-line delegation themselves.
+
+### `LazyParamInjector` (changed)
+
+`injectParams(Interceptor, ...)` becomes `resolveInto(InterceptorParams target, ...)`. It
+still uses `textParser.evaluate` followed by `ognlUtil.setProperty`, so OGNL type conversion
+against the holder's typed setters is preserved (`maximumSize` still converts `String` →
+`Long`).
+
+Two behaviours are added:
+
+- **Unresolved detection.** `OgnlTextParser.java:85-88` yields `""` for an expression that
+ does not resolve and gives no signal distinguishing that from a legitimate empty value.
+ The injector holds the raw template, so `raw.contains("${") && resolved.isEmpty()`
+ recovers it. On unresolved: skip the write (the holder keeps its seeded config-time
+ value), call `target.unresolved(paramName)`, and log a WARN naming interceptor, param and
+ expression.
+- **Unknown-param warning.** A param with no matching property on the holder currently
+ no-ops silently, because `ognlUtil.setProperty` swallows the `OgnlException`
+ (`OgnlUtil.java:297-299`). Log a WARN. Not a regression — a typo no-ops today too — but
+ this design makes it more likely to matter.
+
+### `DefaultActionInvocation` (changed)
+
+`AbstractInterceptor.java:26` implements `ConditionalInterceptor`, so every interceptor
+extending it — `ActionFileUploadInterceptor` included — takes the `instanceof
+ConditionalInterceptor` branch at `:271` and is invoked through `executeConditional` →
+`conditionalInterceptor.intercept(this)` at `:318`. That works today only because
+`injectParams` had already mutated the singleton. Under the new contract the lazy and
+conditional paths must be merged, or the single-arg `intercept` would run with unresolved
+values and the dynamic policy would silently vanish.
+
+```java
+private <P extends InterceptorParams> String invokeWithLazyParams(WithLazyParams<P> lazy,
+ InterceptorMapping mapping) throws Exception {
+ P params = lazy.newLazyParams();
+ lazyParamInjector.resolveInto(params, mergedParams(mapping), invocationContext);
+ if (params instanceof DisableParams dp && dp.isDisabled()) {
+ return invoke();
+ }
+ if (lazy instanceof ConditionalInterceptor ci && !ci.shouldIntercept(this)) {
+ return invoke();
+ }
+ return lazy.intercept(this, params);
+}
+```
+
+Checking both `isDisabled()` and `shouldIntercept` keeps custom `shouldIntercept` overrides
+working while making the lazily-resolved `disabled` request-scoped. The singleton's
+`disabled` field is never written on this path, so `AbstractInterceptor.shouldIntercept`
+returns `true` and the holder is authoritative.
+
+`mergedParams(mapping)` returns a fresh map instead of mutating the shared one, retiring the
+`putAll` at `:262-267`.
+
+### `UploadPolicy` (new)
+
+`org.apache.struts2.interceptor.UploadPolicy` — top-level, not nested, because it appears in
+the interceptor's public signature.
+
+```java
+public class UploadPolicy extends DisableParams {
+ private Long maximumSize;
+ private Set<String> allowedTypes = Collections.emptySet();
+ private Set<String> allowedExtensions = Collections.emptySet();
+ // typed setters (OGNL converts String -> Long for maximumSize), getters, copy()
+ // overrides unresolved(String) to record dimensions that could not be resolved
+}
+```
+
+### `AbstractFileUploadInterceptor` / `ActionFileUploadInterceptor` (changed)
+
+The interceptor holds a single config-time `UploadPolicy` rather than keeping the three
+fields at `:62-64` *and* adding a holder:
+
+```java
+private final UploadPolicy configuredPolicy = new UploadPolicy();
+
+public void setAllowedTypes(String csv) { configuredPolicy.setAllowedTypes(csv); } // config-time only
+// ...
+@Override public UploadPolicy newLazyParams() { return configuredPolicy.copy(); }
+```
+
+Public setter signatures are unchanged, so static `struts.xml` config and `buildInterceptor`
+reflection are untouched. They are documented as config-time only, which is what they
+already are — no framework code calls them outside OGNL injection.
+
+`acceptFile` takes the policy explicitly, and `getMaximumSizeStr` (`:164-166`) takes the
+value rather than reading a field:
+
+```java
+protected boolean acceptFile(UploadPolicy policy, Object action, UploadedFile file,
+ String originalFilename, String contentType, String inputName)
+```
+
+This is a `protected` break for any subclass overriding `acceptFile`.
+
+`ActionFileUploadInterceptor implements WithLazyParams<UploadPolicy>`; its single-arg
+`intercept` delegates to `intercept(invocation, newLazyParams())` so direct use outside the
+lazy path still works.
+
+## Data flow
+
+**Config time (once at startup)** — unchanged. `InterceptorBuilder` →
+`objectFactory.buildInterceptor(config, params)` reflects params onto the singleton,
+populating `configuredPolicy` with literal values including any raw `${...}` text.
+`InterceptorMapping` stores the raw param map.
+
+**Request time (per invocation)**
+
+1. `DefaultActionInvocation.invoke()` takes the next `InterceptorMapping`;
+ `instanceof WithLazyParams<?>` routes to `invokeWithLazyParams`.
+2. `mergedParams(mapping)` builds a fresh map.
+3. `lazy.newLazyParams()` → `configuredPolicy.copy()`.
+4. `lazyParamInjector.resolveInto(holder, merged, invocationContext)`.
+5. `disabled` check, then `shouldIntercept`.
+6. `lazy.intercept(invocation, holder)`.
+7. The holder becomes garbage when the invocation ends.
+
+Step 7 is the design's own check: **there is no cleanup step.** No `finally`, nothing to
+clear, no `ThreadLocal` to leak. If a future change makes cleanup necessary, the separation
+has regressed.
+
+## Error handling
+
+Unresolvable `${...}` is **fail-closed**. Today it produces `""`
+(`OgnlTextParser.java:85-88`) → empty set (`TextParseUtil.java:257`) → treated as "no
+restriction" by `acceptFile` (`:141`, `:148` both guard on `!isEmpty()`), so a typo or a
+null intermediate silently switches off an upload restriction.
+
+Under this design:
+
+- The write is skipped, so the holder keeps its seeded config-time value, and
+ `unresolved(paramName)` is called.
+- A WARN is logged naming interceptor, param and expression.
+
+`UploadPolicy.unresolved(param)` then has to decide whether the seeded value is usable. Two
+cases exist, and they are distinguishable:
+
+- **A genuine static fallback.** The interceptor's own `<interceptor>` definition carried a
+ literal value for that param and the `<interceptor-ref>` overrode it with an expression,
+ so the seed is a real value (e.g. `image/png`). That value applies and validation
+ proceeds normally.
+- **No fallback.** The only configuration for that param is the expression itself, so
+ `buildInterceptor` seeded the holder with the literal `${...}` text — as a set containing
+ the string `"${uploadConfig.allowedMimeTypes}"`, which matches no content type.
+
+The rule: `unresolved(param)` marks the dimension unusable **only if** the seeded value for
+that dimension still contains `${`. Otherwise a genuine static fallback exists and is used.
+A dimension marked unusable causes `acceptFile` to reject the file with a dedicated message
+rather than the opaque one produced by matching content types against literal `${...}` text.
+This needs a new bundle key — `struts.messages.error.upload.policy.unresolved` — added to
+the shipped properties.
+
+This is a behaviour change for 7.2.x applications with a broken expression. Those
+applications are currently running with that validation silently disabled, which is the
+reason to surface it.
+
+## Testing
+
+`ActionFileUploadInterceptorTest` is JUnit 3 style — `protected void setUp()`, plain
+`public void testX()` methods, no annotations. A JUnit 5 `@Test` added there compiles and
+silently never runs. New tests must follow the existing style.
+
+- Carry over PR #1815's concurrency regression, adapted to the new signature, retaining the
+ contributor's attribution. Its scenario is the acceptance criterion.
+- Direct guard for the defect: after an invocation resolves a policy, assert
+ `newLazyParams()` still returns the configured values — the singleton was never written.
+- `disabled` request-scoping: concurrent invocations resolving different `disabled` values;
+ assert only the intended one is skipped.
+- Fail-closed: unresolved expression with no static fallback → rejected with the new
+ message; unresolved *with* a static fallback (literal on the `<interceptor>` definition,
+ expression on the `<interceptor-ref>`) → the static value applies and validation proceeds.
+- `ConditionalInterceptor` interaction: a `WithLazyParams` interceptor that is also
+ conditional still honours a custom `shouldIntercept`.
+- Migrate existing dynamic tests (`testDynamicParameterEvaluation` and friends) off the
+ "simulate injection by calling the setter directly" shortcut onto real `LazyParamInjector`
+ calls. PR #1815 already started this.
+
+## Out of scope
+
+Defining *all* interceptor params via dedicated params classes, rather than loose setters on
+each interceptor, is the natural extension of `InterceptorParams`. It would touch 44
+interceptor implementations across core and plugins plus every third-party interceptor, so
+it is an ecosystem-wide breaking change belonging to 8.0.0 alongside the struts2-api
+extraction (WW-4759). It is not required for correctness here: `disabled` is racy only on
+the lazy path, which this change already covers. A separate ticket will be filed.
(struts) 12/13: WW-5659 fix(core): mark params unusable when a lazy value cannot be applied
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 0cafec4d24cc751e554ef58c9850fa671ab2ab73
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 11:59:33 2026 +0200
WW-5659 fix(core): mark params unusable when a lazy value cannot be applied
resolveInto had two failure branches behaving oppositely. An unresolvable
${...} skipped the write and notified the holder, so a fail-closed holder such
as UploadPolicy could reject the upload. A value the holder's setter could not
accept — a non-numeric String for the Long maximumSize, say — also skipped the
write but notified nothing, leaving the policy reporting isUnresolved() == false
and maximumSize null, which acceptFile reads as "no size limit". The cap was
silently off and the file accepted: fail-open, through a branch the design never
enumerated.
Notify the holder from the catch block too, and record in the spec the general
rule that every path skipping a write must notify, so a future failure mode gets
checked against it.
Co-Authored-By: Claude Opus 5 <no...@anthropic.com>
---
.../apache/struts2/interceptor/WithLazyParams.java | 32 ++++++++++++++++------
.../struts2/interceptor/LazyParamInjectorTest.java | 14 +++++++++-
...7-WW-5659-lazy-params-request-scoping-design.md | 24 ++++++++++++++--
3 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
index 85d8e6d71..64cea0499 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java
@@ -98,13 +98,22 @@ public interface WithLazyParams<P extends InterceptorParams> {
/**
* Resolves configured params into a per-invocation holder, leaving the interceptor untouched.
* <p>
- * A {@code ${...}} expression that resolves to null or an empty value is not written: the
- * holder keeps its seeded configuration value and is notified via
- * {@link InterceptorParams#unresolved(String)}. This also catches an expression that
- * legitimately evaluates to an empty string, which is indistinguishable from a failed
- * resolution (see {@link #isUnresolved}); for a fail-closed policy such as an allowlist,
- * treating both as unusable is the safe reading, so a broken expression cannot silently
- * relax a validation policy.
+ * <strong>Every path that skips a write notifies the holder</strong> via
+ * {@link InterceptorParams#unresolved(String)}, so the holder can fail closed rather than
+ * silently validating against a dimension that was dropped. Two such paths exist:
+ * <ul>
+ * <li>a {@code ${...}} expression that resolves to null or an empty value (see
+ * {@link #isUnresolved})</li>
+ * <li>a resolved value the holder's setter cannot accept, e.g. a non-numeric string for a
+ * {@code Long} property, which OGNL reports as a
+ * {@link ReflectionException} during conversion</li>
+ * </ul>
+ * In both cases the holder keeps its seeded configuration value and a WARN is logged.
+ * <p>
+ * The empty-value rule also catches an expression that legitimately evaluates to an empty
+ * string, which is indistinguishable from a failed resolution; for a fail-closed policy such
+ * as an allowlist, treating both as unusable is the safe reading, so a broken expression
+ * cannot silently relax a validation policy.
*
* @since 7.3.0
*/
@@ -125,8 +134,9 @@ public interface WithLazyParams<P extends InterceptorParams> {
// reported rather than silently ignored; OgnlUtil only warns in devMode otherwise
ognlUtil.setProperty(paramName, paramValue, target, invocationContext.getContextMap(), true);
} catch (ReflectionException e) {
- LOG.warn("Param [{}] cannot be applied to [{}]; check the interceptor configuration",
+ LOG.warn("Param [{}] cannot be applied to [{}] - the value was not written and the params are marked unusable; check the interceptor configuration",
paramName, target.getClass().getName(), e);
+ target.unresolved(paramName);
}
}
return target;
@@ -141,6 +151,12 @@ public interface WithLazyParams<P extends InterceptorParams> {
* to; a param that legitimately evaluates to an empty string is therefore also reported as
* unresolved. That is a deliberate fail-closed choice: for a security-sensitive param (e.g.
* an allowlist), silently accepting an unintended empty value is worse than refusing it.
+ * <p>
+ * <strong>Partial resolution is not detected.</strong> A value mixing several expressions,
+ * e.g. {@code ${a},${b}}, still parses to a non-empty string when only one of them resolves,
+ * so it is reported as resolved and the truncated value is written. For an allowlist that
+ * narrows the accepted set rather than widening it, so it does not relax validation, but the
+ * holder receives fewer entries than configured and gets no {@code unresolved} notification.
*/
private boolean isUnresolved(String rawValue, Object paramValue) {
return rawValue != null
diff --git a/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java b/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
index 46f5282f0..48ef39c56 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/LazyParamInjectorTest.java
@@ -51,6 +51,7 @@ public class LazyParamInjectorTest extends StrutsInternalTestCase {
public String getLabel() { return "resolved-label"; }
public Long getLimit() { return 4096L; }
public String getBlank() { return ""; }
+ public String getNotANumber() { return "5MB"; }
}
private ActionContext context;
@@ -127,6 +128,16 @@ public class LazyParamInjectorTest extends StrutsInternalTestCase {
assertThat(holder.getUnresolvedCalls()).containsExactly("name");
}
+ public void testValueThatCannotBeConvertedSkipsWriteAndNotifiesHolder() {
+ Map<String, String> params = new HashMap<>();
+ params.put("size", "${notANumber}");
+
+ Holder holder = injector.resolveInto(new Holder(), params, context);
+
+ assertThat(holder.getSize()).isNull();
+ assertThat(holder.getUnresolvedCalls()).containsExactly("size");
+ }
+
public void testResolvesDisabledOntoDisableParams() {
Map<String, String> params = new HashMap<>();
params.put("disabled", "true");
@@ -136,7 +147,7 @@ public class LazyParamInjectorTest extends StrutsInternalTestCase {
assertThat(holder.isDisabled()).isTrue();
}
- public void testUnknownParamIsIgnoredWithoutFailingTheInvocation() {
+ public void testUnknownParamDoesNotFailTheInvocationButNotifiesHolder() {
Map<String, String> params = new HashMap<>();
params.put("noSuchParam", "whatever");
@@ -144,5 +155,6 @@ public class LazyParamInjectorTest extends StrutsInternalTestCase {
assertThat(holder.getName()).isNull();
assertThat(holder.getSize()).isNull();
+ assertThat(holder.getUnresolvedCalls()).containsExactly("noSuchParam");
}
}
diff --git a/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md b/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
index 397b79be3..221dff23f 100644
--- a/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
+++ b/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
@@ -173,8 +173,9 @@ Two behaviours are added:
expression.
- **Unknown-param warning.** A param with no matching property on the holder currently
no-ops silently, because `ognlUtil.setProperty` swallows the `OgnlException`
- (`OgnlUtil.java:297-299`). Log a WARN. Not a regression — a typo no-ops today too — but
- this design makes it more likely to matter.
+ (`OgnlUtil.java:297-299`). Set `throwPropertyExceptions=true`, log a WARN, and call
+ `unresolved(paramName)` — see the general rule under *Error handling*. Not a regression in
+ detection — a typo no-ops today too — but this design makes it more likely to matter.
### `DefaultActionInvocation` (changed)
@@ -290,6 +291,25 @@ Under this design:
`unresolved(paramName)` is called.
- A WARN is logged naming interceptor, param and expression.
+**The rule is general: every path in `resolveInto` that skips a write must notify the holder
+via `unresolved(paramName)`.** Any skipped write leaves the holder reporting a value the
+configuration did not ask for, and a holder that is not told cannot fail closed — it validates
+against a dimension that was silently dropped, which is precisely the failure this ticket
+exists to remove. There are three such paths:
+
+1. **Unresolvable or empty `${...}`** — detected by `isUnresolved`, as above.
+2. **A resolved value the holder's setter cannot accept** — e.g. `${uploadConfig.maxFileSize}`
+ returning a non-numeric `String` for the `Long` `maximumSize`. OGNL conversion throws
+ `ReflectionException` (`resolveInto` sets `throwPropertyExceptions=true`), the write is
+ skipped, and `maximumSize` stays `null` — which `acceptFile` reads as "no size limit". Left
+ unnotified this branch is fail-*open*, so it must call `unresolved` too.
+3. **A param with no matching property on the holder** — the same `ReflectionException`
+ (`NoSuchPropertyException`) from a typo'd param name. Config-time reflection does not reject
+ it (`DefaultInterceptorFactory` calls `setProperties` without `throwPropertyExceptions`), so
+ the lazy path is where it first surfaces; it is notified for the same reason.
+
+A new failure mode added to `resolveInto` later must be checked against this rule.
+
`UploadPolicy.unresolved(param)` records the parameter and marks the whole policy unusable,
regardless of the seeded value. A static fallback therefore applies only when the param is absent
from the lazy map entirely, i.e. pure static configuration, which never triggers `unresolved`.
(struts) 11/13: WW-5659 test(core): exercise real lazy param resolution in dynamic upload tests
Posted by lu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5659-lazy-params-request-scoping
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 83c52c9f077519d7f65ed0f7260568b9e31b6cfc
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Mon Jul 27 11:39:55 2026 +0200
WW-5659 test(core): exercise real lazy param resolution in dynamic upload tests
---
.../interceptor/ActionFileUploadInterceptor.java | 13 +++++
.../ActionFileUploadInterceptorTest.java | 62 +++++++++++++++-------
...7-WW-5659-lazy-params-request-scoping-design.md | 26 ++++-----
3 files changed, 67 insertions(+), 34 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
index 82ff798e3..c898275f0 100644
--- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
+++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java
@@ -198,6 +198,19 @@ import java.util.List;
* }
* </pre>
*
+ * <p>
+ * Dynamic parameters are resolved into a fresh {@link UploadPolicy} for each invocation, so the
+ * interceptor itself is never modified per request and concurrent uploads cannot observe each
+ * other's policy. An expression that cannot be resolved does not relax validation: the policy is
+ * marked unresolved and affected uploads are rejected. A lazily resolved {@code disabled} param
+ * only takes effect if the interceptor's params holder extends {@link DisableParams} — there is
+ * deliberately no fallback to the interceptor instance. Likewise, an interceptor that overrides
+ * {@link ConditionalInterceptor#shouldIntercept(ActionInvocation) shouldIntercept} to read its own
+ * lazily-injected fields would see only config-time values, since resolution never touches the
+ * interceptor; {@code ActionFileUploadInterceptor} does not override {@code shouldIntercept}, so
+ * this does not affect it.
+ * </p>
+ *
* @see WithLazyParams
* @see UploadedFilesAware
* @see AbstractFileUploadInterceptor
diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index ab31c3d18..abdc12763 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -648,11 +648,10 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().getValueStack().push(action);
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
- // Simulate WithLazyParams injection by manually setting the parameters
- // In real execution, DefaultActionInvocation.invoke() would call LazyParamInjector
- interceptor.setAllowedTypes(action.getAllowedMimeTypes());
-
- interceptor.intercept(mai);
+ // Exercise the real resolution path: LazyParamInjector resolves ${allowedMimeTypes}
+ // into a fresh UploadPolicy instead of mutating the shared interceptor.
+ UploadPolicy policy = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false);
+ interceptor.intercept(mai, policy);
List<UploadedFile> files = action.getUploadFiles();
@@ -686,8 +685,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().getValueStack().push(action1);
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
- interceptor.setAllowedTypes(action1.getAllowedMimeTypes());
- interceptor.intercept(mai1);
+ UploadPolicy policy1 = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false);
+ interceptor.intercept(mai1, policy1);
assertThat(action1.getUploadFiles()).isNotNull().hasSize(1);
assertThat(action1.getUploadFiles().get(0).getContentType()).isEqualTo("text/plain");
@@ -715,8 +714,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
// Simulate new parameter evaluation for second request
- interceptor.setAllowedTypes(action2.getAllowedMimeTypes());
- interceptor.intercept(mai2);
+ UploadPolicy policy2 = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false);
+ interceptor.intercept(mai2, policy2);
assertThat(action2.getUploadFiles()).isNotNull().hasSize(1);
assertThat(action2.getUploadFiles().get(0).getContentType()).isEqualTo("text/html");
@@ -746,8 +745,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().getValueStack().push(action);
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
- interceptor.setAllowedExtensions(action.getAllowedExtensions());
- interceptor.intercept(mai);
+ UploadPolicy policy = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, true, false);
+ interceptor.intercept(mai, policy);
List<UploadedFile> files = action.getUploadFiles();
@@ -785,8 +784,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().getValueStack().push(action);
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
- interceptor.setMaximumSize(action.getMaxFileSize());
- interceptor.intercept(mai);
+ UploadPolicy policy = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, false, true);
+ interceptor.intercept(mai, policy);
// File should be rejected due to size
assertThat(action.hasFieldErrors()).isTrue();
@@ -819,9 +818,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().getValueStack().push(action);
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
- interceptor.setAllowedTypes(action.getAllowedMimeTypes());
- interceptor.setAllowedExtensions(action.getAllowedExtensions());
- interceptor.intercept(mai);
+ UploadPolicy policy = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, true, false);
+ interceptor.intercept(mai, policy);
List<UploadedFile> files = action.getUploadFiles();
@@ -856,8 +854,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
ActionContext.getContext().getValueStack().push(action);
ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles());
- interceptor.setAllowedTypes(action.getAllowedMimeTypes());
- interceptor.intercept(mai);
+ UploadPolicy policy = injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false);
+ interceptor.intercept(mai, policy);
List<UploadedFile> files = action.getUploadFiles();
@@ -1074,6 +1072,34 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
}
}
+ /**
+ * Resolves the given flags into a fresh {@link UploadPolicy} via the real
+ * {@link WithLazyParams.LazyParamInjector} path, using the already-bound {@code context}
+ * (and its ValueStack, with the action already pushed by the caller) rather than fabricating
+ * a new one. This mirrors what {@code DefaultActionInvocation} does at request time, so tests
+ * exercise the actual resolution instead of hand-calling a setter on the shared interceptor.
+ */
+ private UploadPolicy injectDynamicUploadPolicy(ActionFileUploadInterceptor actionFileUploadInterceptor,
+ ActionContext context,
+ boolean includeAllowedTypes,
+ boolean includeAllowedExtensions,
+ boolean includeMaximumSize) {
+ Map<String, String> params = new HashMap<>();
+ if (includeAllowedTypes) {
+ params.put("allowedTypes", "${allowedMimeTypes}");
+ }
+ if (includeAllowedExtensions) {
+ params.put("allowedExtensions", "${allowedExtensions}");
+ }
+ if (includeMaximumSize) {
+ params.put("maximumSize", "${maxFileSize}");
+ }
+
+ WithLazyParams.LazyParamInjector injector = new WithLazyParams.LazyParamInjector(context.getValueStack());
+ container.inject(injector);
+ return injector.resolveInto(actionFileUploadInterceptor.newLazyParams(), params, context);
+ }
+
private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInterceptor,
MyDynamicFileUploadAction action,
MockHttpServletRequest uploadRequest) throws Exception {
diff --git a/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md b/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
index 20e8a7c96..397b79be3 100644
--- a/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
+++ b/docs/superpowers/specs/2026-07-27-WW-5659-lazy-params-request-scoping-design.md
@@ -290,19 +290,15 @@ Under this design:
`unresolved(paramName)` is called.
- A WARN is logged naming interceptor, param and expression.
-`UploadPolicy.unresolved(param)` then has to decide whether the seeded value is usable. Two
-cases exist, and they are distinguishable:
-
-- **A genuine static fallback.** The interceptor's own `<interceptor>` definition carried a
- literal value for that param and the `<interceptor-ref>` overrode it with an expression,
- so the seed is a real value (e.g. `image/png`). That value applies and validation
- proceeds normally.
-- **No fallback.** The only configuration for that param is the expression itself, so
- `buildInterceptor` seeded the holder with the literal `${...}` text — as a set containing
- the string `"${uploadConfig.allowedMimeTypes}"`, which matches no content type.
-
-The rule: `unresolved(param)` marks the dimension unusable **only if** the seeded value for
-that dimension still contains `${`. Otherwise a genuine static fallback exists and is used.
+`UploadPolicy.unresolved(param)` records the parameter and marks the whole policy unusable,
+regardless of the seeded value. A static fallback therefore applies only when the param is absent
+from the lazy map entirely, i.e. pure static configuration, which never triggers `unresolved`.
+
+The seed-introspection alternative — honouring a seeded value that does not itself contain
+`${` — was rejected during planning: it is not implementable deterministically, because
+`maximumSize` is seeded as a `Long` and cannot carry a `${...}` literal, so the rule would behave
+differently per parameter type.
+
A dimension marked unusable causes `acceptFile` to reject the file with a dedicated message
rather than the opaque one produced by matching content types against literal `${...}` text.
This needs a new bundle key — `struts.messages.error.upload.policy.unresolved` — added to
@@ -324,9 +320,7 @@ silently never runs. New tests must follow the existing style.
`newLazyParams()` still returns the configured values — the singleton was never written.
- `disabled` request-scoping: concurrent invocations resolving different `disabled` values;
assert only the intended one is skipped.
-- Fail-closed: unresolved expression with no static fallback → rejected with the new
- message; unresolved *with* a static fallback (literal on the `<interceptor>` definition,
- expression on the `<interceptor-ref>`) → the static value applies and validation proceeds.
+- Fail-closed: unresolved expression → rejected with the new message.
- `ConditionalInterceptor` interaction: a `WithLazyParams` interceptor that is also
conditional still honours a custom `shouldIntercept`.
- Migrate existing dynamic tests (`testDynamicParameterEvaluation` and friends) off the