WW-5585 Keep dynamic upload validation request-scoped

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,11 @@ public String invoke() throws Exception {
             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.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 {
                     LOG.debug("Executing normal interceptor: {}", interceptorMapping.getName());
@@ -312,6 +307,56 @@ 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 <P> String executeWithLazyParams(InterceptorMapping interceptorMapping,
+                                               Interceptor interceptor,
+                                               WithLazyParams.InvocationScoped<P> 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 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());

core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java

@@ -68,10 +68,16 @@ public Interceptor buildInterceptor(InterceptorConfig interceptorConfig, Map<Str
                 throw new ConfigurationException("Class [" + interceptorClassName + "] does not implement Interceptor", interceptorConfig);
             }
 
-            reflectionProvider.setProperties(params, interceptor);
-            if (interceptor instanceof WithLazyParams) {
-                LOG.debug("Interceptor {} implements {} - expression parameters will be re-evaluated during action invocation",
+            if (interceptor instanceof WithLazyParams.InvocationScoped) {
+                reflectionProvider.setProperties(filterFactoryTimeParams(params), interceptor);
+                LOG.debug("Interceptor {} implements {} - expression parameters will be re-evaluated into invocation-scoped lazy params",
                         interceptorClassName, WithLazyParams.class.getName());
+            } else {
+                reflectionProvider.setProperties(params, interceptor);
+                if (interceptor instanceof WithLazyParams) {
+                    LOG.debug("Interceptor {} implements {} - expression parameters will be re-evaluated during action invocation",
+                            interceptorClassName, WithLazyParams.class.getName());
+                }
             }
 
             interceptor.init();
@@ -96,4 +102,18 @@ public Interceptor buildInterceptor(InterceptorConfig interceptorConfig, Map<Str
         throw new ConfigurationException(message, cause, interceptorConfig);
     }
 
+    private Map<String, String> filterFactoryTimeParams(Map<String, String> params) {
+        Map<String, String> eagerParams = new HashMap<>();
+        for (Map.Entry<String, String> 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("}");
+    }
+
 }

core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java

@@ -82,7 +82,7 @@ public void setContainer(Container container) {
      * @param allowedExtensions A comma-delimited list of extensions
      */
     public void setAllowedExtensions(String allowedExtensions) {
-        allowedExtensionsSet = TextParseUtil.commaDelimitedStringToSet(allowedExtensions);
+        allowedExtensionsSet = parseCommaDelimitedSet(allowedExtensions);
     }
 
     /**
@@ -91,7 +91,7 @@ public void setAllowedExtensions(String allowedExtensions) {
      * @param allowedTypes A comma-delimited list of types
      */
     public void setAllowedTypes(String allowedTypes) {
-        allowedTypesSet = TextParseUtil.commaDelimitedStringToSet(allowedTypes);
+        allowedTypesSet = parseCommaDelimitedSet(allowedTypes);
     }
 
     /**
@@ -114,6 +114,15 @@ 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) {
+        return acceptFile(action, file, originalFilename, contentType, inputName, createUploadValidationPolicy());
+    }
+
+    protected boolean acceptFile(Object action,
+                                 UploadedFile file,
+                                 String originalFilename,
+                                 String contentType,
+                                 String inputName,
+                                 UploadValidationPolicy validationPolicy) {
         Set<String> errorMessages = new HashSet<>();
 
         ValidationAware validation = null;
@@ -131,21 +140,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 +170,21 @@ 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);
     }
 
+    protected UploadValidationPolicy createUploadValidationPolicy() {
+        return new UploadValidationPolicy(maximumSize, allowedTypesSet, allowedExtensionsSet);
+    }
+
+    private Set<String> parseCommaDelimitedSet(String value) {
+        if (value == null || value.isBlank()) {
+            return Collections.emptySet();
+        }
+        return TextParseUtil.commaDelimitedStringToSet(value);
+    }
+
     /**
      * @param extensionCollection - Collection of extensions (all lowercase).
      * @param filename            - filename to check.
@@ -249,4 +269,32 @@ protected void applyValidation(Object action, MultiPartRequestWrapper multiWrapp
         }
     }
 
+    protected static class UploadValidationPolicy {
+        private Long maximumSize;
+        private Set<String> allowedTypesSet = Collections.emptySet();
+        private Set<String> allowedExtensionsSet = Collections.emptySet();
+
+        private UploadValidationPolicy(Long maximumSize, Set<String> allowedTypesSet, Set<String> 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);
+        }
+    }
+
 }

core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java

@@ -202,12 +202,23 @@
  * @see UploadedFilesAware
  * @see AbstractFileUploadInterceptor
  */
-public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor implements WithLazyParams {
+public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor
+        implements WithLazyParams.InvocationScoped<AbstractFileUploadInterceptor.UploadValidationPolicy> {
 
     protected static final Logger LOG = LogManager.getLogger(ActionFileUploadInterceptor.class);
 
     @Override
     public String intercept(ActionInvocation invocation) throws Exception {
+        return WithLazyParams.InvocationScoped.super.intercept(invocation);
+    }
+
+    @Override
+    public UploadValidationPolicy newLazyParams() {
+        return createUploadValidationPolicy();
+    }
+
+    @Override
+    public String intercept(ActionInvocation invocation, UploadValidationPolicy validationPolicy) throws Exception {
         HttpServletRequest request = invocation.getInvocationContext().getServletRequest();
         MultiPartRequestWrapper multiWrapper = request instanceof HttpServletRequestWrapper wrapper
                 ? findMultipartRequestWrapper(wrapper)
@@ -245,7 +256,7 @@ public String intercept(ActionInvocation invocation) throws Exception {
                 }
             } else {
                 for (UploadedFile uploadedFile : uploadedFiles) {
-                    if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) {
+                    if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName, validationPolicy)) {
                         acceptedFiles.add(uploadedFile);
                     }
                 }
@@ -285,4 +296,3 @@ protected MultiPartRequestWrapper findMultipartRequestWrapper(HttpServletRequest
     }
 
 }
-

core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java

@@ -18,20 +18,19 @@
  */
 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.ognl.ThreadAllowlist;
 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}.
  * <p>
  * This enables both:
  * <ul>
@@ -41,18 +40,40 @@
  * <p>
  * 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 {
 
+    /**
+     * Optional extension for interceptors that need lazy params isolated per invocation instead of
+     * being written back into the long-lived interceptor instance.
+     */
+    interface InvocationScoped<P> extends WithLazyParams, Interceptor {
+
+        /**
+         * 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());
+        }
+    }
+
     class LazyParamInjector {
 
         protected OgnlUtil ognlUtil;
         protected TextParser textParser;
-        protected ReflectionProvider reflectionProvider;
-
+        protected ThreadAllowlist threadAllowlist;
         private final TextParseUtil.ParsedValueEvaluator valueEvaluator;
 
         public LazyParamInjector(final ValueStack valueStack) {
@@ -66,21 +87,34 @@ public void setTextParser(TextParser textParser) {
         }
 
         @Inject
-        public void setReflectionProvider(ReflectionProvider reflectionProvider) {
-            this.reflectionProvider = reflectionProvider;
+        public void setOgnlUtil(OgnlUtil ognlUtil) {
+            this.ognlUtil = ognlUtil;
         }
 
         @Inject
-        public void setOgnlUtil(OgnlUtil ognlUtil) {
-            this.ognlUtil = ognlUtil;
+        public void setThreadAllowlist(ThreadAllowlist threadAllowlist) {
+            this.threadAllowlist = threadAllowlist;
+        }
+
+        public <P> P injectParams(InvocationScoped<P> interceptor, Map<String, String> params, ActionContext invocationContext) {
+            P lazyParams = interceptor.newLazyParams();
+            injectParams(lazyParams, params, invocationContext);
+            return lazyParams;
         }
 
         public Interceptor injectParams(Interceptor interceptor, Map<String, String> params, ActionContext invocationContext) {
+            injectParams((Object) interceptor, params, invocationContext);
+            return interceptor;
+        }
+
+        private void injectParams(Object target, Map<String, String> params, ActionContext invocationContext) {
+            if (threadAllowlist != null) {
+                threadAllowlist.allowClassHierarchy(target.getClass());
+            }
             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());
+                ognlUtil.setProperty(entry.getKey(), paramValue, target, invocationContext.getContextMap());
             }
-            return interceptor;
         }
     }
 }