From 345f7030ce3cb4e00de6bf9b7438592db642f705 Mon Sep 17 00:00:00 2001 From: deprrous Date: Mon, 27 Jul 2026 10:56:00 +0800 Subject: [PATCH 1/5] WW-5585 Keep dynamic upload validation request-scoped Follow-up to the dynamic upload-policy support added in WW-5585. When lazy upload parameters are resolved per request, keep the effective policy on request-local state instead of writing it back to the shared interceptor instance. Add a regression that covers the concurrent overwrite case. --- .../AbstractFileUploadInterceptor.java | 66 +++++- .../ActionFileUploadInterceptor.java | 87 ++++---- .../struts2/interceptor/WithLazyParams.java | 19 +- .../ActionFileUploadInterceptorTest.java | 194 +++++++++++++++++- 4 files changed, 303 insertions(+), 63 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 c7cbb65c8d..de0bf8cf62 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.interceptor; +import org.apache.struts2.ActionContext; import org.apache.struts2.locale.LocaleProvider; import org.apache.struts2.locale.LocaleProviderFactory; import org.apache.struts2.text.TextProvider; @@ -62,6 +63,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor private Long maximumSize; private Set allowedTypesSet = Collections.emptySet(); private Set allowedExtensionsSet = Collections.emptySet(); + private final ThreadLocal requestScopedUploadValidationPolicy = new ThreadLocal<>(); private ContentTypeMatcher matcher; private Container container; @@ -82,7 +84,12 @@ public void setContainer(Container container) { * @param allowedExtensions A comma-delimited list of extensions */ public void setAllowedExtensions(String allowedExtensions) { - allowedExtensionsSet = TextParseUtil.commaDelimitedStringToSet(allowedExtensions); + Set parsedAllowedExtensions = TextParseUtil.commaDelimitedStringToSet(allowedExtensions); + if (WithLazyParams.LazyParamInjector.isParamInjectionInProgress() && ActionContext.getContext() != null) { + getOrCreateRequestScopedUploadValidationPolicy().allowedExtensionsSet = parsedAllowedExtensions; + } else { + allowedExtensionsSet = parsedAllowedExtensions; + } } /** @@ -91,7 +98,12 @@ public void setAllowedExtensions(String allowedExtensions) { * @param allowedTypes A comma-delimited list of types */ public void setAllowedTypes(String allowedTypes) { - allowedTypesSet = TextParseUtil.commaDelimitedStringToSet(allowedTypes); + Set parsedAllowedTypes = TextParseUtil.commaDelimitedStringToSet(allowedTypes); + if (WithLazyParams.LazyParamInjector.isParamInjectionInProgress() && ActionContext.getContext() != null) { + getOrCreateRequestScopedUploadValidationPolicy().allowedTypesSet = parsedAllowedTypes; + } else { + allowedTypesSet = parsedAllowedTypes; + } } /** @@ -100,7 +112,15 @@ public void setAllowedTypes(String allowedTypes) { * @param maximumSize The maximum size in bytes */ public void setMaximumSize(Long maximumSize) { - this.maximumSize = maximumSize; + if (WithLazyParams.LazyParamInjector.isParamInjectionInProgress() && ActionContext.getContext() != null) { + getOrCreateRequestScopedUploadValidationPolicy().maximumSize = maximumSize; + } else { + this.maximumSize = maximumSize; + } + } + + protected void clearRequestScopedUploadValidationPolicy() { + requestScopedUploadValidationPolicy.remove(); } /** @@ -114,6 +134,7 @@ public void setMaximumSize(Long maximumSize) { * @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) { + UploadValidationPolicy validationPolicy = getUploadValidationPolicy(); Set errorMessages = new HashSet<>(); ValidationAware validation = null; @@ -131,21 +152,21 @@ protected boolean acceptFile(Object action, UploadedFile file, String originalFi return false; } - if (maximumSize != null && maximumSize < file.length()) { + if (validationPolicy.maximumSize != null && validationPolicy.maximumSize < 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, validationPolicy.maximumSize) }); errorMessages.add(errMsg); LOG.warn(errMsg); } - if ((!allowedTypesSet.isEmpty()) && (!containsItem(allowedTypesSet, contentType))) { + if ((!validationPolicy.allowedTypesSet.isEmpty()) && (!containsItem(validationPolicy.allowedTypesSet, 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 ((!validationPolicy.allowedExtensionsSet.isEmpty()) && (!hasAllowedExtension(validationPolicy.allowedExtensionsSet, originalFilename))) { String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY, new String[]{ inputName, originalFilename, file.getName(), contentType }); @@ -161,10 +182,27 @@ protected boolean acceptFile(Object action, UploadedFile file, String originalFi return errorMessages.isEmpty(); } - private String getMaximumSizeStr(Object action) { + private String getMaximumSizeStr(Object action, Long maximumSize) { return NumberFormat.getNumberInstance(getLocaleProvider(action).getLocale()).format(maximumSize); } + private UploadValidationPolicy getUploadValidationPolicy() { + UploadValidationPolicy requestScopedPolicy = requestScopedUploadValidationPolicy.get(); + if (requestScopedPolicy != null) { + return requestScopedPolicy; + } + return new UploadValidationPolicy(maximumSize, allowedTypesSet, allowedExtensionsSet); + } + + private UploadValidationPolicy getOrCreateRequestScopedUploadValidationPolicy() { + UploadValidationPolicy requestScopedPolicy = requestScopedUploadValidationPolicy.get(); + if (requestScopedPolicy == null) { + requestScopedPolicy = new UploadValidationPolicy(maximumSize, allowedTypesSet, allowedExtensionsSet); + requestScopedUploadValidationPolicy.set(requestScopedPolicy); + } + return requestScopedPolicy; + } + /** * @param extensionCollection - Collection of extensions (all lowercase). * @param filename - filename to check. @@ -249,4 +287,16 @@ protected void applyValidation(Object action, MultiPartRequestWrapper multiWrapp } } + private static final class UploadValidationPolicy { + private Long maximumSize; + private Set allowedTypesSet; + private Set allowedExtensionsSet; + + private UploadValidationPolicy(Long maximumSize, Set allowedTypesSet, Set allowedExtensionsSet) { + this.maximumSize = maximumSize; + this.allowedTypesSet = allowedTypesSet; + this.allowedExtensionsSet = allowedExtensionsSet; + } + } + } 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 79d020a345..4286d1bbfb 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java @@ -208,59 +208,63 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor i @Override public String intercept(ActionInvocation invocation) throws Exception { - HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); - MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper - ? findMultipartRequestWrapper(wrapper) - : null; + try { + HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); + MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper + ? findMultipartRequestWrapper(wrapper) + : null; - if (multiWrapper == null) { - if (LOG.isDebugEnabled()) { - ActionProxy proxy = invocation.getProxy(); - LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); + if (multiWrapper == null) { + if (LOG.isDebugEnabled()) { + ActionProxy proxy = invocation.getProxy(); + LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); + } + return invocation.invoke(); } - return invocation.invoke(); - } - if (!(invocation.getAction() instanceof UploadedFilesAware action)) { - LOG.debug("Action: {} doesn't implement: {}, ignoring file upload", - invocation.getProxy().getActionName(), - UploadedFilesAware.class.getSimpleName()); - return invocation.invoke(); - } + if (!(invocation.getAction() instanceof UploadedFilesAware action)) { + LOG.debug("Action: {} doesn't implement: {}, ignoring file upload", + invocation.getProxy().getActionName(), + UploadedFilesAware.class.getSimpleName()); + return invocation.invoke(); + } - applyValidation(action, multiWrapper); + applyValidation(action, multiWrapper); - // bind allowed Files - Enumeration fileParameterNames = multiWrapper.getFileParameterNames(); - List acceptedFiles = new ArrayList<>(); + // bind allowed Files + Enumeration fileParameterNames = multiWrapper.getFileParameterNames(); + List acceptedFiles = new ArrayList<>(); - while (fileParameterNames != null && fileParameterNames.hasMoreElements()) { - // get the value of this input tag - String inputName = fileParameterNames.nextElement(); - UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); + while (fileParameterNames != null && fileParameterNames.hasMoreElements()) { + // get the value of this input tag + String inputName = fileParameterNames.nextElement(); + UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); - if (uploadedFiles == null || uploadedFiles.length == 0) { - if (LOG.isWarnEnabled()) { - LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); - } - } else { - for (UploadedFile uploadedFile : uploadedFiles) { - if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) { - acceptedFiles.add(uploadedFile); + if (uploadedFiles == null || uploadedFiles.length == 0) { + if (LOG.isWarnEnabled()) { + LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); + } + } else { + for (UploadedFile uploadedFile : uploadedFiles) { + if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) { + acceptedFiles.add(uploadedFile); + } } } } - } - if (acceptedFiles.isEmpty()) { - LOG.debug("No files have been uploaded/accepted"); - } else { - LOG.debug("Passing: {} uploaded file(s) to action", acceptedFiles.size()); - action.withUploadedFiles(acceptedFiles); - } + if (acceptedFiles.isEmpty()) { + LOG.debug("No files have been uploaded/accepted"); + } else { + LOG.debug("Passing: {} uploaded file(s) to action", acceptedFiles.size()); + action.withUploadedFiles(acceptedFiles); + } - // invoke action - return invocation.invoke(); + // invoke action + return invocation.invoke(); + } finally { + clearRequestScopedUploadValidationPolicy(); + } } /** @@ -285,4 +289,3 @@ protected MultiPartRequestWrapper findMultipartRequestWrapper(HttpServletRequest } } - 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 cdfb3a8adb..34855837e9 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java +++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java @@ -49,6 +49,8 @@ public interface WithLazyParams { class LazyParamInjector { + private static final ThreadLocal PARAM_INJECTION_IN_PROGRESS = new ThreadLocal<>(); + protected OgnlUtil ognlUtil; protected TextParser textParser; protected ReflectionProvider reflectionProvider; @@ -75,12 +77,21 @@ public void setOgnlUtil(OgnlUtil ognlUtil) { this.ognlUtil = ognlUtil; } + public static boolean isParamInjectionInProgress() { + return Boolean.TRUE.equals(PARAM_INJECTION_IN_PROGRESS.get()); + } + public Interceptor injectParams(Interceptor interceptor, Map params, ActionContext invocationContext) { - for (Map.Entry entry : params.entrySet()) { - Object paramValue = textParser.evaluate(new char[]{'$'}, entry.getValue(), valueEvaluator, TextParser.DEFAULT_LOOP_COUNT); - ognlUtil.setProperty(entry.getKey(), paramValue, interceptor, invocationContext.getContextMap()); + PARAM_INJECTION_IN_PROGRESS.set(Boolean.TRUE); + try { + for (Map.Entry 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; + } finally { + PARAM_INJECTION_IN_PROGRESS.remove(); } - return interceptor; } } } 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 058dabe115..7eb41540b5 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.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.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,6 +546,10 @@ private MultiPartRequestWrapper createMultipartRequestMaxStringLength() { } private MultiPartRequestWrapper createMultipartRequest(int maxsize, int maxfilesize, int maxfiles, int maxStringLength) { + return createMultipartRequest(request, maxsize, maxfilesize, maxfiles, maxStringLength); + } + + 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)); @@ -543,7 +557,17 @@ private MultiPartRequestWrapper createMultipartRequest(int maxsize, int maxfiles jak.setMaxStringLength(String.valueOf(maxStringLength)); jak.setDefaultEncoding(StandardCharsets.UTF_8.name()); - return new MultiPartRequestWrapper(jak, request, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); + return new MultiPartRequestWrapper(jak, multipartRequest, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); + } + + 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; } protected void setUp() throws Exception { @@ -647,7 +671,7 @@ public void testDynamicParameterEvaluation() throws Exception { // Simulate WithLazyParams injection by manually setting the parameters // In real execution, DefaultActionInvocation.invoke() would call LazyParamInjector - interceptor.setAllowedTypes(action.getAllowedMimeTypes()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); interceptor.intercept(mai); @@ -683,7 +707,7 @@ public void testDynamicParametersChangePerRequest() throws Exception { ActionContext.getContext().getValueStack().push(action1); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - interceptor.setAllowedTypes(action1.getAllowedMimeTypes()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); interceptor.intercept(mai1); assertThat(action1.getUploadFiles()).isNotNull().hasSize(1); @@ -712,7 +736,7 @@ public void testDynamicParametersChangePerRequest() throws Exception { ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); // Simulate new parameter evaluation for second request - interceptor.setAllowedTypes(action2.getAllowedMimeTypes()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); interceptor.intercept(mai2); assertThat(action2.getUploadFiles()).isNotNull().hasSize(1); @@ -743,7 +767,7 @@ public void testDynamicExtensionValidation() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - interceptor.setAllowedExtensions(action.getAllowedExtensions()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, true, false); interceptor.intercept(mai); List files = action.getUploadFiles(); @@ -782,7 +806,7 @@ public void testDynamicMaximumSizeValidation() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - interceptor.setMaximumSize(action.getMaxFileSize()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, false, true); interceptor.intercept(mai); // File should be rejected due to size @@ -816,8 +840,7 @@ public void testSecurityValidationWithDynamicParameters() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - interceptor.setAllowedTypes(action.getAllowedMimeTypes()); - interceptor.setAllowedExtensions(action.getAllowedExtensions()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, true, false); interceptor.intercept(mai); List files = action.getUploadFiles(); @@ -853,7 +876,7 @@ public void testWildcardMatchingWithDynamicParameters() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - interceptor.setAllowedTypes(action.getAllowedMimeTypes()); + injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); interceptor.intercept(mai); List files = action.getUploadFiles(); @@ -864,6 +887,125 @@ public void testWildcardMatchingWithDynamicParameters() throws Exception { assertThat(files.get(1).getContentType()).startsWith("image/"); } + public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Exception { + ActionFileUploadInterceptor controlInterceptor = new ActionFileUploadInterceptor(); + container.inject(controlInterceptor); + + MyDynamicFileUploadAction controlAction = new MyDynamicFileUploadAction(); + controlAction.setAllowedMimeTypes("text/plain"); + container.inject(controlAction); + + try { + runUploadAttempt( + controlInterceptor, + controlAction, + createUploadRequest("control.html", "text/html", htmlContent), + controlAction.getAllowedMimeTypes() + ); + + assertThat(controlAction.getUploadFiles()).isNull(); + assertThat(controlAction.getFieldErrors()).containsKey("file"); + } finally { + controlInterceptor.destroy(); + } + + 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 plainResult = executor.submit(() -> runUploadAttempt( + sharedInterceptor, + plainPolicyAction, + createUploadRequest("plain-policy.html", "text/html", htmlContent), + plainPolicyAction.getAllowedMimeTypes() + )); + + assertThat(sharedInterceptor.awaitFirstValidation()).isTrue(); + + Future htmlResult = executor.submit(() -> runUploadAttempt( + sharedInterceptor, + htmlPolicyAction, + createUploadRequest("html-policy.html", "text/html", htmlContent), + htmlPolicyAction.getAllowedMimeTypes() + )); + + 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(); + } + + assertThat(plainPolicyAction.getAllowedMimeTypes()).isEqualTo("text/plain"); + 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"); + assertThat(htmlPolicyAction.getUploadFiles().get(0).getContentType()).isEqualTo("text/html"); + } + + private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInterceptor, + MyDynamicFileUploadAction action, + MockHttpServletRequest uploadRequest, + String allowedTypes) 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); + + injectDynamicUploadPolicy(actionFileUploadInterceptor, context, true, false, false); + return actionFileUploadInterceptor.intercept(invocation); + } finally { + ActionContext.clear(); + } + } + + private void injectDynamicUploadPolicy(ActionFileUploadInterceptor actionFileUploadInterceptor, + ActionContext context, + boolean includeAllowedTypes, + boolean includeAllowedExtensions, + boolean includeMaximumSize) { + Map params = new HashMap<>(); + if (includeAllowedTypes) { + params.put("allowedTypes", "${allowedMimeTypes}"); + } + if (includeAllowedExtensions) { + params.put("allowedExtensions", "${allowedExtensions}"); + } + if (includeMaximumSize) { + params.put("maximumSize", "${maxFileSize}"); + } + + WithLazyParams.LazyParamInjector lazyParamInjector = + new WithLazyParams.LazyParamInjector(context.getValueStack()); + container.inject(lazyParamInjector); + lazyParamInjector.injectParams(actionFileUploadInterceptor, params, context); + } + public static class MyFileUploadAction extends ActionSupport implements UploadedFilesAware { private List uploadedFiles; @@ -920,4 +1062,38 @@ public void setMaxFileSize(Long maxFileSize) { } } + 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(Object action, UploadedFile file, String originalFilename, String contentType, String inputName) { + if (pauseFirstValidation.compareAndSet(true, false)) { + firstValidationEntered.countDown(); + awaitUnchecked(allowFirstValidationToContinue); + } + return super.acceptFile(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); + } + } + } + } From 1c975b487fde422ea3f361a4128be62cad499784 Mon Sep 17 00:00:00 2001 From: deprrous Date: Mon, 27 Jul 2026 13:53:25 +0800 Subject: [PATCH 2/5] test: remove unused upload helper parameter --- .../interceptor/ActionFileUploadInterceptorTest.java | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) 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 7eb41540b5..231476ebd3 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@ -899,8 +899,7 @@ public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Excepti runUploadAttempt( controlInterceptor, controlAction, - createUploadRequest("control.html", "text/html", htmlContent), - controlAction.getAllowedMimeTypes() + createUploadRequest("control.html", "text/html", htmlContent) ); assertThat(controlAction.getUploadFiles()).isNull(); @@ -925,8 +924,7 @@ public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Excepti Future plainResult = executor.submit(() -> runUploadAttempt( sharedInterceptor, plainPolicyAction, - createUploadRequest("plain-policy.html", "text/html", htmlContent), - plainPolicyAction.getAllowedMimeTypes() + createUploadRequest("plain-policy.html", "text/html", htmlContent) )); assertThat(sharedInterceptor.awaitFirstValidation()).isTrue(); @@ -934,8 +932,7 @@ public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Excepti Future htmlResult = executor.submit(() -> runUploadAttempt( sharedInterceptor, htmlPolicyAction, - createUploadRequest("html-policy.html", "text/html", htmlContent), - htmlPolicyAction.getAllowedMimeTypes() + createUploadRequest("html-policy.html", "text/html", htmlContent) )); assertThat(htmlResult.get(10, TimeUnit.SECONDS)).isEqualTo("success"); @@ -959,8 +956,7 @@ public void testConcurrentDynamicPoliciesStayIsolatedPerRequest() throws Excepti private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInterceptor, MyDynamicFileUploadAction action, - MockHttpServletRequest uploadRequest, - String allowedTypes) throws Exception { + MockHttpServletRequest uploadRequest) throws Exception { MultiPartRequestWrapper multiPartRequest = createMultipartRequest(uploadRequest, -1, -1, 3, -1); ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack(); valueStack.push(action); From 286966cd6f99005504ba39fa15221ca6327ed611 Mon Sep 17 00:00:00 2001 From: deprrous Date: Mon, 27 Jul 2026 16:38:47 +0800 Subject: [PATCH 3/5] WW-5659 Make lazy interceptor params invocation-scoped --- .../struts2/DefaultActionInvocation.java | 38 +++-- .../factory/DefaultInterceptorFactory.java | 18 ++- .../AbstractFileUploadInterceptor.java | 74 +++++---- .../ActionFileUploadInterceptor.java | 99 ++++++------ .../struts2/interceptor/WithLazyParams.java | 53 ++++--- .../DefaultInterceptorFactoryTest.java | 146 ++++++++++++++++++ .../ActionFileUploadInterceptorTest.java | 49 +++--- .../struts2/mock/MockLazyInterceptor.java | 45 +++++- 8 files changed, 370 insertions(+), 152 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java diff --git a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java index 6885dd50d1..fd20926477 100644 --- a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java +++ b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java @@ -41,6 +41,7 @@ import java.util.ArrayList; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; @@ -258,17 +259,9 @@ public String invoke() throws Exception { if (interceptors.hasNext()) { final InterceptorMapping interceptorMapping = interceptors.next(); Interceptor interceptor = interceptorMapping.getInterceptor(); - if (interceptor instanceof WithLazyParams) { - Map 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 withLazyParamsInterceptor) { + resultCode = executeWithLazyParams(interceptorMapping, interceptor, withLazyParamsInterceptor); + } else if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) { resultCode = executeConditional(conditionalInterceptor); } else { LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName()); @@ -312,6 +305,29 @@ public String invoke() throws Exception { return resultCode; } + protected

String executeWithLazyParams(InterceptorMapping interceptorMapping, + Interceptor interceptor, + WithLazyParams

withLazyParamsInterceptor) throws Exception { + P lazyParams = lazyParamInjector.injectParams( + withLazyParamsInterceptor, + new LinkedHashMap<>(interceptorMapping.getParams()), + invocationContext + ); + + if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) { + if (conditionalInterceptor.shouldIntercept(this)) { + LOG.debug("Executing conditional interceptor: {}", conditionalInterceptor.getClass().getSimpleName()); + return withLazyParamsInterceptor.intercept(this, lazyParams); + } else { + LOG.debug("Interceptor: {} is disabled, skipping to next", conditionalInterceptor.getClass().getSimpleName()); + return this.invoke(); + } + } + + LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName()); + return withLazyParamsInterceptor.intercept(this, lazyParams); + } + 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/factory/DefaultInterceptorFactory.java b/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java index cd5bc09bd2..13e20e30f1 100644 --- a/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java +++ b/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java @@ -68,10 +68,12 @@ public Interceptor buildInterceptor(InterceptorConfig interceptorConfig, Map filterFactoryTimeParams(Map params) { + Map eagerParams = new HashMap<>(); + for (Map.Entry entry : params.entrySet()) { + if (!containsLazyParamExpression(entry.getValue())) { + eagerParams.put(entry.getKey(), entry.getValue()); + } + } + return eagerParams; + } + + private boolean containsLazyParamExpression(String value) { + return value != null && value.contains("${") && value.contains("}"); + } + } 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 de0bf8cf62..23e699a6de 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java @@ -18,7 +18,6 @@ */ package org.apache.struts2.interceptor; -import org.apache.struts2.ActionContext; import org.apache.struts2.locale.LocaleProvider; import org.apache.struts2.locale.LocaleProviderFactory; import org.apache.struts2.text.TextProvider; @@ -63,7 +62,6 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor private Long maximumSize; private Set allowedTypesSet = Collections.emptySet(); private Set allowedExtensionsSet = Collections.emptySet(); - private final ThreadLocal requestScopedUploadValidationPolicy = new ThreadLocal<>(); private ContentTypeMatcher matcher; private Container container; @@ -84,12 +82,7 @@ public void setContainer(Container container) { * @param allowedExtensions A comma-delimited list of extensions */ public void setAllowedExtensions(String allowedExtensions) { - Set parsedAllowedExtensions = TextParseUtil.commaDelimitedStringToSet(allowedExtensions); - if (WithLazyParams.LazyParamInjector.isParamInjectionInProgress() && ActionContext.getContext() != null) { - getOrCreateRequestScopedUploadValidationPolicy().allowedExtensionsSet = parsedAllowedExtensions; - } else { - allowedExtensionsSet = parsedAllowedExtensions; - } + allowedExtensionsSet = parseCommaDelimitedSet(allowedExtensions); } /** @@ -98,12 +91,7 @@ public void setAllowedExtensions(String allowedExtensions) { * @param allowedTypes A comma-delimited list of types */ public void setAllowedTypes(String allowedTypes) { - Set parsedAllowedTypes = TextParseUtil.commaDelimitedStringToSet(allowedTypes); - if (WithLazyParams.LazyParamInjector.isParamInjectionInProgress() && ActionContext.getContext() != null) { - getOrCreateRequestScopedUploadValidationPolicy().allowedTypesSet = parsedAllowedTypes; - } else { - allowedTypesSet = parsedAllowedTypes; - } + allowedTypesSet = parseCommaDelimitedSet(allowedTypes); } /** @@ -112,15 +100,7 @@ public void setAllowedTypes(String allowedTypes) { * @param maximumSize The maximum size in bytes */ public void setMaximumSize(Long maximumSize) { - if (WithLazyParams.LazyParamInjector.isParamInjectionInProgress() && ActionContext.getContext() != null) { - getOrCreateRequestScopedUploadValidationPolicy().maximumSize = maximumSize; - } else { - this.maximumSize = maximumSize; - } - } - - protected void clearRequestScopedUploadValidationPolicy() { - requestScopedUploadValidationPolicy.remove(); + this.maximumSize = maximumSize; } /** @@ -134,7 +114,15 @@ protected void clearRequestScopedUploadValidationPolicy() { * @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) { - UploadValidationPolicy validationPolicy = getUploadValidationPolicy(); + return acceptFile(action, file, originalFilename, contentType, inputName, createUploadValidationPolicy()); + } + + protected boolean acceptFile(Object action, + UploadedFile file, + String originalFilename, + String contentType, + String inputName, + UploadValidationPolicy validationPolicy) { Set errorMessages = new HashSet<>(); ValidationAware validation = null; @@ -186,21 +174,15 @@ private String getMaximumSizeStr(Object action, Long maximumSize) { return NumberFormat.getNumberInstance(getLocaleProvider(action).getLocale()).format(maximumSize); } - private UploadValidationPolicy getUploadValidationPolicy() { - UploadValidationPolicy requestScopedPolicy = requestScopedUploadValidationPolicy.get(); - if (requestScopedPolicy != null) { - return requestScopedPolicy; - } + protected UploadValidationPolicy createUploadValidationPolicy() { return new UploadValidationPolicy(maximumSize, allowedTypesSet, allowedExtensionsSet); } - private UploadValidationPolicy getOrCreateRequestScopedUploadValidationPolicy() { - UploadValidationPolicy requestScopedPolicy = requestScopedUploadValidationPolicy.get(); - if (requestScopedPolicy == null) { - requestScopedPolicy = new UploadValidationPolicy(maximumSize, allowedTypesSet, allowedExtensionsSet); - requestScopedUploadValidationPolicy.set(requestScopedPolicy); + private Set parseCommaDelimitedSet(String value) { + if (value == null || value.isBlank()) { + return Collections.emptySet(); } - return requestScopedPolicy; + return TextParseUtil.commaDelimitedStringToSet(value); } /** @@ -287,16 +269,32 @@ protected void applyValidation(Object action, MultiPartRequestWrapper multiWrapp } } - private static final class UploadValidationPolicy { + protected static class UploadValidationPolicy { private Long maximumSize; - private Set allowedTypesSet; - private Set allowedExtensionsSet; + private Set allowedTypesSet = Collections.emptySet(); + private Set allowedExtensionsSet = Collections.emptySet(); private UploadValidationPolicy(Long maximumSize, Set allowedTypesSet, Set allowedExtensionsSet) { this.maximumSize = maximumSize; this.allowedTypesSet = allowedTypesSet; this.allowedExtensionsSet = allowedExtensionsSet; } + + public void setMaximumSize(Long maximumSize) { + this.maximumSize = maximumSize; + } + + public void setAllowedTypes(String allowedTypes) { + this.allowedTypesSet = allowedTypes == null || allowedTypes.isBlank() + ? Collections.emptySet() + : TextParseUtil.commaDelimitedStringToSet(allowedTypes); + } + + public void setAllowedExtensions(String allowedExtensions) { + this.allowedExtensionsSet = allowedExtensions == null || allowedExtensions.isBlank() + ? Collections.emptySet() + : TextParseUtil.commaDelimitedStringToSet(allowedExtensions); + } } } 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 4286d1bbfb..b89b1a0d0e 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java @@ -202,69 +202,76 @@ * @see UploadedFilesAware * @see AbstractFileUploadInterceptor */ -public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor implements WithLazyParams { +public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor + implements WithLazyParams { protected static final Logger LOG = LogManager.getLogger(ActionFileUploadInterceptor.class); @Override public String intercept(ActionInvocation invocation) throws Exception { - try { - HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); - MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper - ? findMultipartRequestWrapper(wrapper) - : null; + return WithLazyParams.super.intercept(invocation); + } - if (multiWrapper == null) { - if (LOG.isDebugEnabled()) { - ActionProxy proxy = invocation.getProxy(); - LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); - } - return invocation.invoke(); - } + @Override + public UploadValidationPolicy newLazyParams() { + return createUploadValidationPolicy(); + } - if (!(invocation.getAction() instanceof UploadedFilesAware action)) { - LOG.debug("Action: {} doesn't implement: {}, ignoring file upload", - invocation.getProxy().getActionName(), - UploadedFilesAware.class.getSimpleName()); - return invocation.invoke(); + @Override + public String intercept(ActionInvocation invocation, UploadValidationPolicy validationPolicy) throws Exception { + HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); + MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper + ? findMultipartRequestWrapper(wrapper) + : null; + + if (multiWrapper == null) { + if (LOG.isDebugEnabled()) { + ActionProxy proxy = invocation.getProxy(); + LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); } + return invocation.invoke(); + } - applyValidation(action, multiWrapper); + if (!(invocation.getAction() instanceof UploadedFilesAware action)) { + LOG.debug("Action: {} doesn't implement: {}, ignoring file upload", + invocation.getProxy().getActionName(), + UploadedFilesAware.class.getSimpleName()); + return invocation.invoke(); + } - // bind allowed Files - Enumeration fileParameterNames = multiWrapper.getFileParameterNames(); - List acceptedFiles = new ArrayList<>(); + applyValidation(action, multiWrapper); - while (fileParameterNames != null && fileParameterNames.hasMoreElements()) { - // get the value of this input tag - String inputName = fileParameterNames.nextElement(); - UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); + // bind allowed Files + Enumeration fileParameterNames = multiWrapper.getFileParameterNames(); + List acceptedFiles = new ArrayList<>(); - if (uploadedFiles == null || uploadedFiles.length == 0) { - if (LOG.isWarnEnabled()) { - LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); - } - } else { - for (UploadedFile uploadedFile : uploadedFiles) { - if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) { - acceptedFiles.add(uploadedFile); - } - } - } - } + while (fileParameterNames != null && fileParameterNames.hasMoreElements()) { + // get the value of this input tag + String inputName = fileParameterNames.nextElement(); + UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); - if (acceptedFiles.isEmpty()) { - LOG.debug("No files have been uploaded/accepted"); + if (uploadedFiles == null || uploadedFiles.length == 0) { + if (LOG.isWarnEnabled()) { + LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); + } } else { - LOG.debug("Passing: {} uploaded file(s) to action", acceptedFiles.size()); - action.withUploadedFiles(acceptedFiles); + for (UploadedFile uploadedFile : uploadedFiles) { + if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName, validationPolicy)) { + acceptedFiles.add(uploadedFile); + } + } } + } - // invoke action - return invocation.invoke(); - } finally { - clearRequestScopedUploadValidationPolicy(); + if (acceptedFiles.isEmpty()) { + LOG.debug("No files have been uploaded/accepted"); + } else { + LOG.debug("Passing: {} uploaded file(s) to action", acceptedFiles.size()); + action.withUploadedFiles(acceptedFiles); } + + // invoke action + return invocation.invoke(); } /** 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 34855837e9..6be0bab003 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java +++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java @@ -18,20 +18,21 @@ */ package org.apache.struts2.interceptor; +import org.apache.struts2.ActionInvocation; 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.ReflectionProvider; import java.util.Map; /** * Interceptors marked with this interface support dynamic parameter evaluation at action invocation time. - * Parameters are set during interceptor creation (factory time), then re-evaluated during each action - * invocation to resolve expressions like ${someValue}. + * Static parameters are set during interceptor creation (factory time), while lazy expression parameters + * are re-evaluated during each action invocation to resolve expressions like ${someValue} into an + * invocation-scoped parameter holder. *

* This enables both: *

    @@ -45,16 +46,27 @@ * * @since 2.5.9 */ -public interface WithLazyParams { +public interface WithLazyParams

    extends Interceptor { - class LazyParamInjector { + /** + * Creates the per-invocation holder used for lazily resolved interceptor params. + */ + P newLazyParams(); + + /** + * Intercepts using the per-invocation lazy params resolved for the current request. + */ + String intercept(ActionInvocation invocation, P lazyParams) throws Exception; + + @Override + default String intercept(ActionInvocation invocation) throws Exception { + return intercept(invocation, newLazyParams()); + } - private static final ThreadLocal PARAM_INJECTION_IN_PROGRESS = new ThreadLocal<>(); + class LazyParamInjector { protected OgnlUtil ognlUtil; protected TextParser textParser; - protected ReflectionProvider reflectionProvider; - private final TextParseUtil.ParsedValueEvaluator valueEvaluator; public LazyParamInjector(final ValueStack valueStack) { @@ -67,31 +79,18 @@ public void setTextParser(TextParser textParser) { this.textParser = textParser; } - @Inject - public void setReflectionProvider(ReflectionProvider reflectionProvider) { - this.reflectionProvider = reflectionProvider; - } - @Inject public void setOgnlUtil(OgnlUtil ognlUtil) { this.ognlUtil = ognlUtil; } - public static boolean isParamInjectionInProgress() { - return Boolean.TRUE.equals(PARAM_INJECTION_IN_PROGRESS.get()); - } - - public Interceptor injectParams(Interceptor interceptor, Map params, ActionContext invocationContext) { - PARAM_INJECTION_IN_PROGRESS.set(Boolean.TRUE); - try { - for (Map.Entry 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; - } finally { - PARAM_INJECTION_IN_PROGRESS.remove(); + public

    P injectParams(WithLazyParams

    interceptor, Map params, ActionContext invocationContext) { + P lazyParams = interceptor.newLazyParams(); + for (Map.Entry entry : params.entrySet()) { + Object paramValue = textParser.evaluate(new char[]{'$'}, entry.getValue(), valueEvaluator, TextParser.DEFAULT_LOOP_COUNT); + ognlUtil.setProperty(entry.getKey(), paramValue, lazyParams, invocationContext.getContextMap()); } + return lazyParams; } } } diff --git a/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java b/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java new file mode 100644 index 0000000000..ba9e13b29c --- /dev/null +++ b/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java @@ -0,0 +1,146 @@ +/* + * 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.factory; + +import org.apache.struts2.ActionContext; +import org.apache.struts2.ActionInvocation; +import org.apache.struts2.XWorkTestCase; +import org.apache.struts2.config.entities.InterceptorConfig; +import org.apache.struts2.interceptor.AbstractInterceptor; +import org.apache.struts2.interceptor.WithLazyParams; +import org.apache.struts2.util.ValueStack; +import org.apache.struts2.util.ValueStackFactory; + +import java.util.Collections; + +public class DefaultInterceptorFactoryTest extends XWorkTestCase { + + public void testBuildInterceptorDefersTypedLazyParamsUntilInvocationTime() throws Exception { + DefaultInterceptorFactory factory = new DefaultInterceptorFactory(); + container.inject(factory); + + InterceptorConfig config = new InterceptorConfig.Builder("typedLazy", TypedLazyInterceptor.class.getName()) + .addParam("maximumSize", "${maxFileSize}") + .addParam("mode", "strict") + .build(); + + TypedLazyInterceptor interceptor = + (TypedLazyInterceptor) factory.buildInterceptor(config, Collections.emptyMap()); + + assertNull("Dynamic Long param should not be written into the singleton interceptor at factory time", + interceptor.getMaximumSize()); + assertEquals("strict", interceptor.getMode()); + + ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack(); + valueStack.push(new LazyParamAction(42L)); + + ActionContext context = ActionContext.of(valueStack.getContext()) + .withContainer(container) + .withValueStack(valueStack) + .bind(); + + try { + WithLazyParams.LazyParamInjector lazyParamInjector = new WithLazyParams.LazyParamInjector(valueStack); + container.inject(lazyParamInjector); + + TypedLazyInterceptor.LazyParams lazyParams = + lazyParamInjector.injectParams(interceptor, config.getParams(), context); + + assertEquals(Long.valueOf(42L), lazyParams.getMaximumSize()); + assertEquals("strict", lazyParams.getMode()); + } finally { + ActionContext.clear(); + } + } + + public static final class TypedLazyInterceptor extends AbstractInterceptor + implements WithLazyParams { + private Long maximumSize; + private String mode; + + public Long getMaximumSize() { + return maximumSize; + } + + public void setMaximumSize(Long maximumSize) { + this.maximumSize = maximumSize; + } + + public String getMode() { + return mode; + } + + public void setMode(String mode) { + this.mode = mode; + } + + @Override + public LazyParams newLazyParams() { + return new LazyParams(maximumSize, mode); + } + + @Override + public String intercept(ActionInvocation invocation) throws Exception { + return WithLazyParams.super.intercept(invocation); + } + + @Override + public String intercept(ActionInvocation invocation, LazyParams lazyParams) throws Exception { + return invocation.invoke(); + } + + public static final class LazyParams { + private Long maximumSize; + private String mode; + + private LazyParams(Long maximumSize, String mode) { + this.maximumSize = maximumSize; + this.mode = mode; + } + + public Long getMaximumSize() { + return maximumSize; + } + + public void setMaximumSize(Long maximumSize) { + this.maximumSize = maximumSize; + } + + public String getMode() { + return mode; + } + + public void setMode(String mode) { + this.mode = mode; + } + } + } + + public static final class LazyParamAction { + private final Long maxFileSize; + + private LazyParamAction(Long maxFileSize) { + this.maxFileSize = maxFileSize; + } + + public Long getMaxFileSize() { + return maxFileSize; + } + } +} 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 231476ebd3..3e7cc417e9 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@ -671,9 +671,7 @@ public void testDynamicParameterEvaluation() throws Exception { // Simulate WithLazyParams injection by manually setting the parameters // In real execution, DefaultActionInvocation.invoke() would call LazyParamInjector - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); - - interceptor.intercept(mai); + interceptor.intercept(mai, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false)); List files = action.getUploadFiles(); @@ -707,8 +705,7 @@ public void testDynamicParametersChangePerRequest() throws Exception { ActionContext.getContext().getValueStack().push(action1); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); - interceptor.intercept(mai1); + interceptor.intercept(mai1, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false)); assertThat(action1.getUploadFiles()).isNotNull().hasSize(1); assertThat(action1.getUploadFiles().get(0).getContentType()).isEqualTo("text/plain"); @@ -736,8 +733,7 @@ public void testDynamicParametersChangePerRequest() throws Exception { ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); // Simulate new parameter evaluation for second request - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); - interceptor.intercept(mai2); + interceptor.intercept(mai2, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false)); assertThat(action2.getUploadFiles()).isNotNull().hasSize(1); assertThat(action2.getUploadFiles().get(0).getContentType()).isEqualTo("text/html"); @@ -767,8 +763,7 @@ public void testDynamicExtensionValidation() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, true, false); - interceptor.intercept(mai); + interceptor.intercept(mai, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, true, false)); List files = action.getUploadFiles(); @@ -806,8 +801,7 @@ public void testDynamicMaximumSizeValidation() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, false, true); - interceptor.intercept(mai); + interceptor.intercept(mai, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), false, false, true)); // File should be rejected due to size assertThat(action.hasFieldErrors()).isTrue(); @@ -840,8 +834,7 @@ public void testSecurityValidationWithDynamicParameters() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, true, false); - interceptor.intercept(mai); + interceptor.intercept(mai, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, true, false)); List files = action.getUploadFiles(); @@ -876,8 +869,7 @@ public void testWildcardMatchingWithDynamicParameters() throws Exception { ActionContext.getContext().getValueStack().push(action); ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); - injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false); - interceptor.intercept(mai); + interceptor.intercept(mai, injectDynamicUploadPolicy(interceptor, ActionContext.getContext(), true, false, false)); List files = action.getUploadFiles(); @@ -973,18 +965,20 @@ private String runUploadAttempt(ActionFileUploadInterceptor actionFileUploadInte invocation.setResultCode("success"); invocation.setInvocationContext(context); - injectDynamicUploadPolicy(actionFileUploadInterceptor, context, true, false, false); - return actionFileUploadInterceptor.intercept(invocation); + return actionFileUploadInterceptor.intercept( + invocation, + injectDynamicUploadPolicy(actionFileUploadInterceptor, context, true, false, false) + ); } finally { ActionContext.clear(); } } - private void injectDynamicUploadPolicy(ActionFileUploadInterceptor actionFileUploadInterceptor, - ActionContext context, - boolean includeAllowedTypes, - boolean includeAllowedExtensions, - boolean includeMaximumSize) { + private AbstractFileUploadInterceptor.UploadValidationPolicy injectDynamicUploadPolicy(ActionFileUploadInterceptor actionFileUploadInterceptor, + ActionContext context, + boolean includeAllowedTypes, + boolean includeAllowedExtensions, + boolean includeMaximumSize) { Map params = new HashMap<>(); if (includeAllowedTypes) { params.put("allowedTypes", "${allowedMimeTypes}"); @@ -999,7 +993,7 @@ private void injectDynamicUploadPolicy(ActionFileUploadInterceptor actionFileUpl WithLazyParams.LazyParamInjector lazyParamInjector = new WithLazyParams.LazyParamInjector(context.getValueStack()); container.inject(lazyParamInjector); - lazyParamInjector.injectParams(actionFileUploadInterceptor, params, context); + return lazyParamInjector.injectParams(actionFileUploadInterceptor, params, context); } public static class MyFileUploadAction extends ActionSupport implements UploadedFilesAware { @@ -1064,12 +1058,17 @@ private static final class CoordinatedActionFileUploadInterceptor extends Action private final CountDownLatch allowFirstValidationToContinue = new CountDownLatch(1); @Override - protected boolean acceptFile(Object action, UploadedFile file, String originalFilename, String contentType, String inputName) { + protected boolean acceptFile(Object action, + UploadedFile file, + String originalFilename, + String contentType, + String inputName, + UploadValidationPolicy validationPolicy) { if (pauseFirstValidation.compareAndSet(true, false)) { firstValidationEntered.countDown(); awaitUnchecked(allowFirstValidationToContinue); } - return super.acceptFile(action, file, originalFilename, contentType, inputName); + return super.acceptFile(action, file, originalFilename, contentType, inputName, validationPolicy); } private boolean awaitFirstValidation() throws InterruptedException { 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 ae3aea3cc3..c1cab7c546 100644 --- a/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java +++ b/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java @@ -23,7 +23,7 @@ import org.apache.struts2.interceptor.AbstractInterceptor; import org.apache.struts2.interceptor.WithLazyParams; -public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams { +public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams { private String foo = ""; private String bar = ""; @@ -44,14 +44,51 @@ public String getBar() { return bar; } + @Override public String intercept(ActionInvocation invocation) throws Exception { + return WithLazyParams.super.intercept(invocation); + } + + @Override + public LazyParams newLazyParams() { + return new LazyParams(foo, bar); + } + + @Override + public String intercept(ActionInvocation invocation, LazyParams 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(); } + + public static final class LazyParams { + private String foo = ""; + private String bar = ""; + + private LazyParams(String foo, String bar) { + this.foo = foo; + this.bar = bar; + } + + public String getFoo() { + return foo; + } + + public void setFoo(String foo) { + this.foo = foo; + } + + public String getBar() { + return bar; + } + + public void setBar(String bar) { + this.bar = bar; + } + } } From 437ccddfa00ea78fdb768805fc4cc37b528c973a Mon Sep 17 00:00:00 2001 From: deprrous Date: Mon, 27 Jul 2026 18:11:28 +0800 Subject: [PATCH 4/5] WW-5659 Preserve WithLazyParams compatibility --- .../struts2/DefaultActionInvocation.java | 35 +++++++++- .../factory/DefaultInterceptorFactory.java | 8 ++- .../ActionFileUploadInterceptor.java | 4 +- .../struts2/interceptor/WithLazyParams.java | 48 +++++++++----- .../DefaultInterceptorFactoryTest.java | 66 ++++++++++++++++++- .../struts2/mock/MockLazyInterceptor.java | 4 +- 6 files changed, 137 insertions(+), 28 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java index fd20926477..98ee34c919 100644 --- a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java +++ b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java @@ -259,8 +259,10 @@ public String invoke() throws Exception { if (interceptors.hasNext()) { final InterceptorMapping interceptorMapping = interceptors.next(); Interceptor interceptor = interceptorMapping.getInterceptor(); - if (interceptor instanceof WithLazyParams withLazyParamsInterceptor) { - resultCode = executeWithLazyParams(interceptorMapping, interceptor, withLazyParamsInterceptor); + if (interceptor instanceof WithLazyParams.InvocationScoped invocationScopedInterceptor) { + resultCode = executeWithLazyParams(interceptorMapping, interceptor, invocationScopedInterceptor); + } else if (interceptor instanceof WithLazyParams) { + resultCode = executeWithLazyParams(interceptorMapping, interceptor); } else if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) { resultCode = executeConditional(conditionalInterceptor); } else { @@ -305,9 +307,20 @@ public String invoke() throws Exception { return resultCode; } + protected String executeWithLazyParams(InterceptorMapping interceptorMapping, + Interceptor interceptor) throws Exception { + lazyParamInjector.injectParams( + interceptor, + new LinkedHashMap<>(interceptorMapping.getParams()), + invocationContext + ); + + return executeLazyParamsInterceptor(interceptorMapping, interceptor); + } + protected

    String executeWithLazyParams(InterceptorMapping interceptorMapping, Interceptor interceptor, - WithLazyParams

    withLazyParamsInterceptor) throws Exception { + WithLazyParams.InvocationScoped

    withLazyParamsInterceptor) throws Exception { P lazyParams = lazyParamInjector.injectParams( withLazyParamsInterceptor, new LinkedHashMap<>(interceptorMapping.getParams()), @@ -328,6 +341,22 @@ protected

    String executeWithLazyParams(InterceptorMapping interceptorMapping return withLazyParamsInterceptor.intercept(this, lazyParams); } + protected String executeLazyParamsInterceptor(InterceptorMapping interceptorMapping, + Interceptor interceptor) throws Exception { + if (interceptor instanceof ConditionalInterceptor conditionalInterceptor) { + if (conditionalInterceptor.shouldIntercept(this)) { + LOG.debug("Executing conditional interceptor: {}", conditionalInterceptor.getClass().getSimpleName()); + return interceptor.intercept(this); + } else { + LOG.debug("Interceptor: {} is disabled, skipping to next", conditionalInterceptor.getClass().getSimpleName()); + return this.invoke(); + } + } + + LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName()); + return interceptor.intercept(this); + } + 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/factory/DefaultInterceptorFactory.java b/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java index 13e20e30f1..9a279308ce 100644 --- a/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java +++ b/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java @@ -68,12 +68,16 @@ public Interceptor buildInterceptor(InterceptorConfig interceptorConfig, Map { + implements WithLazyParams.InvocationScoped { protected static final Logger LOG = LogManager.getLogger(ActionFileUploadInterceptor.class); @Override public String intercept(ActionInvocation invocation) throws Exception { - return WithLazyParams.super.intercept(invocation); + return WithLazyParams.InvocationScoped.super.intercept(invocation); } @Override 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 6be0bab003..c41e54a32b 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java +++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java @@ -30,9 +30,6 @@ /** * Interceptors marked with this interface support dynamic parameter evaluation at action invocation time. - * Static parameters are set during interceptor creation (factory time), while lazy expression parameters - * are re-evaluated during each action invocation to resolve expressions like ${someValue} into an - * invocation-scoped parameter holder. *

    * This enables both: *

      @@ -42,25 +39,33 @@ *

      * The {@link Interceptor#init()} method is called after initial parameter setting, so interceptors * can rely on configured values during initialization. Expression parameters (containing ${...}) - * are re-evaluated at invocation time via {@link LazyParamInjector}. + * are re-evaluated at invocation time via {@link LazyParamInjector}. Interceptors that need + * invocation-scoped lazy parameter state should additionally implement {@link InvocationScoped}. * * @since 2.5.9 */ -public interface WithLazyParams

      extends Interceptor { +public interface WithLazyParams { /** - * Creates the per-invocation holder used for lazily resolved interceptor params. + * Optional extension for interceptors that need lazy params isolated per invocation instead of + * being written back into the long-lived interceptor instance. */ - P newLazyParams(); + interface InvocationScoped

      extends WithLazyParams, Interceptor { - /** - * Intercepts using the per-invocation lazy params resolved for the current request. - */ - String intercept(ActionInvocation invocation, P lazyParams) throws Exception; + /** + * Creates the per-invocation holder used for lazily resolved interceptor params. + */ + P newLazyParams(); + + /** + * Intercepts using the per-invocation lazy params resolved for the current request. + */ + String intercept(ActionInvocation invocation, P lazyParams) throws Exception; - @Override - default String intercept(ActionInvocation invocation) throws Exception { - return intercept(invocation, newLazyParams()); + @Override + default String intercept(ActionInvocation invocation) throws Exception { + return intercept(invocation, newLazyParams()); + } } class LazyParamInjector { @@ -84,13 +89,22 @@ public void setOgnlUtil(OgnlUtil ognlUtil) { this.ognlUtil = ognlUtil; } - public

      P injectParams(WithLazyParams

      interceptor, Map params, ActionContext invocationContext) { + public

      P injectParams(InvocationScoped

      interceptor, Map params, ActionContext invocationContext) { P lazyParams = interceptor.newLazyParams(); + injectParams(lazyParams, params, invocationContext); + return lazyParams; + } + + public Interceptor injectParams(Interceptor interceptor, Map params, ActionContext invocationContext) { + injectParams((Object) interceptor, params, invocationContext); + return interceptor; + } + + private void injectParams(Object target, Map params, ActionContext invocationContext) { for (Map.Entry entry : params.entrySet()) { Object paramValue = textParser.evaluate(new char[]{'$'}, entry.getValue(), valueEvaluator, TextParser.DEFAULT_LOOP_COUNT); - ognlUtil.setProperty(entry.getKey(), paramValue, lazyParams, invocationContext.getContextMap()); + ognlUtil.setProperty(entry.getKey(), paramValue, target, invocationContext.getContextMap()); } - return lazyParams; } } } diff --git a/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java b/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java index ba9e13b29c..d947dee540 100644 --- a/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java +++ b/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java @@ -69,8 +69,41 @@ public void testBuildInterceptorDefersTypedLazyParamsUntilInvocationTime() throw } } + public void testBuildInterceptorPreservesLegacyWithLazyParamsContract() throws Exception { + DefaultInterceptorFactory factory = new DefaultInterceptorFactory(); + container.inject(factory); + + InterceptorConfig config = new InterceptorConfig.Builder("legacyLazy", LegacyLazyInterceptor.class.getName()) + .addParam("mode", "${lazyMode}") + .build(); + + LegacyLazyInterceptor interceptor = + (LegacyLazyInterceptor) factory.buildInterceptor(config, Collections.emptyMap()); + + assertEquals("${lazyMode}", interceptor.getMode()); + + ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack(); + valueStack.push(new LegacyLazyAction("strict")); + + ActionContext context = ActionContext.of(valueStack.getContext()) + .withContainer(container) + .withValueStack(valueStack) + .bind(); + + try { + WithLazyParams.LazyParamInjector lazyParamInjector = new WithLazyParams.LazyParamInjector(valueStack); + container.inject(lazyParamInjector); + + lazyParamInjector.injectParams(interceptor, config.getParams(), context); + + assertEquals("strict", interceptor.getMode()); + } finally { + ActionContext.clear(); + } + } + public static final class TypedLazyInterceptor extends AbstractInterceptor - implements WithLazyParams { + implements WithLazyParams.InvocationScoped { private Long maximumSize; private String mode; @@ -97,7 +130,7 @@ public LazyParams newLazyParams() { @Override public String intercept(ActionInvocation invocation) throws Exception { - return WithLazyParams.super.intercept(invocation); + return WithLazyParams.InvocationScoped.super.intercept(invocation); } @Override @@ -132,6 +165,23 @@ public void setMode(String mode) { } } + public static final class LegacyLazyInterceptor extends AbstractInterceptor implements WithLazyParams { + private String mode; + + public String getMode() { + return mode; + } + + public void setMode(String mode) { + this.mode = mode; + } + + @Override + public String intercept(ActionInvocation invocation) throws Exception { + return invocation.invoke(); + } + } + public static final class LazyParamAction { private final Long maxFileSize; @@ -143,4 +193,16 @@ public Long getMaxFileSize() { return maxFileSize; } } + + public static final class LegacyLazyAction { + private final String lazyMode; + + private LegacyLazyAction(String lazyMode) { + this.lazyMode = lazyMode; + } + + public String getLazyMode() { + return lazyMode; + } + } } 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 c1cab7c546..d04b1a80bc 100644 --- a/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java +++ b/core/src/test/java/org/apache/struts2/mock/MockLazyInterceptor.java @@ -23,7 +23,7 @@ import org.apache.struts2.interceptor.AbstractInterceptor; import org.apache.struts2.interceptor.WithLazyParams; -public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams { +public class MockLazyInterceptor extends AbstractInterceptor implements WithLazyParams.InvocationScoped { private String foo = ""; private String bar = ""; @@ -46,7 +46,7 @@ public String getBar() { @Override public String intercept(ActionInvocation invocation) throws Exception { - return WithLazyParams.super.intercept(invocation); + return WithLazyParams.InvocationScoped.super.intercept(invocation); } @Override From ce7c3fcaabff39d843494908897a22143cc328be Mon Sep 17 00:00:00 2001 From: deprrous Date: Mon, 27 Jul 2026 19:03:38 +0800 Subject: [PATCH 5/5] WW-5585 Allowlist request-scoped lazy params --- .../struts2/interceptor/WithLazyParams.java | 10 ++++++ .../DefaultInterceptorFactoryTest.java | 36 +++++++++++++++++++ 2 files changed, 46 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 c41e54a32b..13b8d4ddd6 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java +++ b/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java @@ -22,6 +22,7 @@ import org.apache.struts2.ActionContext; import org.apache.struts2.inject.Inject; import org.apache.struts2.ognl.OgnlUtil; +import org.apache.struts2.ognl.ThreadAllowlist; import org.apache.struts2.util.TextParseUtil; import org.apache.struts2.util.TextParser; import org.apache.struts2.util.ValueStack; @@ -72,6 +73,7 @@ class LazyParamInjector { protected OgnlUtil ognlUtil; protected TextParser textParser; + protected ThreadAllowlist threadAllowlist; private final TextParseUtil.ParsedValueEvaluator valueEvaluator; public LazyParamInjector(final ValueStack valueStack) { @@ -89,6 +91,11 @@ public void setOgnlUtil(OgnlUtil ognlUtil) { this.ognlUtil = ognlUtil; } + @Inject + public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { + this.threadAllowlist = threadAllowlist; + } + public

      P injectParams(InvocationScoped

      interceptor, Map params, ActionContext invocationContext) { P lazyParams = interceptor.newLazyParams(); injectParams(lazyParams, params, invocationContext); @@ -101,6 +108,9 @@ public Interceptor injectParams(Interceptor interceptor, Map par } private void injectParams(Object target, Map params, ActionContext invocationContext) { + if (threadAllowlist != null) { + threadAllowlist.allowClassHierarchy(target.getClass()); + } for (Map.Entry entry : params.entrySet()) { Object paramValue = textParser.evaluate(new char[]{'$'}, entry.getValue(), valueEvaluator, TextParser.DEFAULT_LOOP_COUNT); ognlUtil.setProperty(entry.getKey(), paramValue, target, invocationContext.getContextMap()); diff --git a/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java b/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java index d947dee540..76a4e21bdd 100644 --- a/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java +++ b/core/src/test/java/org/apache/struts2/factory/DefaultInterceptorFactoryTest.java @@ -24,6 +24,7 @@ import org.apache.struts2.config.entities.InterceptorConfig; import org.apache.struts2.interceptor.AbstractInterceptor; import org.apache.struts2.interceptor.WithLazyParams; +import org.apache.struts2.ognl.ThreadAllowlist; import org.apache.struts2.util.ValueStack; import org.apache.struts2.util.ValueStackFactory; @@ -69,6 +70,41 @@ public void testBuildInterceptorDefersTypedLazyParamsUntilInvocationTime() throw } } + public void testBuildInterceptorAllowlistsInvocationScopedLazyParamsCarrier() throws Exception { + DefaultInterceptorFactory factory = new DefaultInterceptorFactory(); + container.inject(factory); + + InterceptorConfig config = new InterceptorConfig.Builder("typedLazy", TypedLazyInterceptor.class.getName()) + .addParam("maximumSize", "${maxFileSize}") + .build(); + + TypedLazyInterceptor interceptor = + (TypedLazyInterceptor) factory.buildInterceptor(config, Collections.emptyMap()); + + ValueStack valueStack = container.getInstance(ValueStackFactory.class).createValueStack(); + valueStack.push(new LazyParamAction(42L)); + + ActionContext context = ActionContext.of(valueStack.getContext()) + .withContainer(container) + .withValueStack(valueStack) + .bind(); + + ThreadAllowlist threadAllowlist = container.getInstance(ThreadAllowlist.class); + threadAllowlist.clearAllowlist(); + + try { + WithLazyParams.LazyParamInjector lazyParamInjector = new WithLazyParams.LazyParamInjector(valueStack); + container.inject(lazyParamInjector); + + lazyParamInjector.injectParams(interceptor, config.getParams(), context); + + assertTrue(threadAllowlist.getAllowlist().contains(TypedLazyInterceptor.LazyParams.class)); + } finally { + threadAllowlist.clearAllowlist(); + ActionContext.clear(); + } + } + public void testBuildInterceptorPreservesLegacyWithLazyParamsContract() throws Exception { DefaultInterceptorFactory factory = new DefaultInterceptorFactory(); container.inject(factory);