diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/DynamicFileUploadAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/DynamicFileUploadAction.java new file mode 100644 index 000000000..422dce6c5 --- /dev/null +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/DynamicFileUploadAction.java @@ -0,0 +1,225 @@ +/* + * 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.showcase.fileupload; + +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.ActionSupport; +import org.apache.struts2.Preparable; +import org.apache.struts2.ServletActionContext; +import org.apache.struts2.action.UploadedFilesAware; +import org.apache.struts2.dispatcher.multipart.UploadedFile; +import org.apache.struts2.interceptor.parameter.StrutsParameter; + +import java.util.List; + +/** + *

+ * Demonstrates dynamic file upload validation using WithLazyParams. + * This action shows how file upload validation rules can be determined + * at runtime based on action properties, session data, or other dynamic values. + *

+ * + *

+ * The validation parameters (allowedTypes, allowedExtensions, maximumSize) + * are set dynamically in the prepare() method and then referenced in struts.xml + * using ${...} expressions. This allows the same action to enforce different + * validation rules based on runtime conditions. + *

+ * + *

+ * This example demonstrates two use cases: + *

+ * + * + * @see org.apache.struts2.interceptor.WithLazyParams + * @see org.apache.struts2.interceptor.ActionFileUploadInterceptor + */ +public class DynamicFileUploadAction extends ActionSupport implements Preparable, UploadedFilesAware { + + private static final Logger LOG = LogManager.getLogger(DynamicFileUploadAction.class); + + private UploadedFile uploadedFile; + private String contentType; + private String fileName; + private String originalName; + private String inputName; + private String uploadType = "document"; + + private transient UploadConfig uploadConfig; + + @Override + public String input() { + return INPUT; + } + + public String upload() { + if (uploadedFile == null) { + addActionError("Please select a file to upload"); + return INPUT; + } + + return SUCCESS; + } + + @Override + public void withUploadedFiles(List uploadedFiles) { + if (!uploadedFiles.isEmpty()) { + LOG.info("Uploaded file: {}", uploadedFiles.get(0)); + this.uploadedFile = uploadedFiles.get(0); + this.fileName = uploadedFile.getName(); + this.contentType = uploadedFile.getContentType(); + this.originalName = uploadedFile.getOriginalName(); + this.inputName = uploadedFile.getInputName(); + } + } + + // Getters and Setters + + public String getContentType() { + return contentType; + } + + public String getFileName() { + return fileName; + } + + public String getOriginalName() { + return originalName; + } + + public String getInputName() { + return inputName; + } + + public Object getUploadedFile() { + return uploadedFile != null ? uploadedFile.getContent() : null; + } + + public long getUploadSize() { + return uploadedFile != null ? uploadedFile.length() : 0; + } + + public String getUploadType() { + return uploadType; + } + + @StrutsParameter + public void setUploadType(String uploadType) { + this.uploadType = uploadType; + } + + @Override + public void prepare() throws Exception { + // no-op + } + + public void prepareUpload() { + String type = ServletActionContext.getRequest().getParameter("uploadType"); + prepareUploadConfig(type != null ? type : "document"); + } + + private void prepareUploadConfig(String uploadType) { + uploadConfig = new UploadConfig(); + LOG.debug("Configure validation rules based on upload type: {}", uploadType); + if ("image".equals(uploadType)) { + // Image upload configuration + uploadConfig.setAllowedMimeTypes("image/jpeg,image/png"); + uploadConfig.setAllowedExtensions(".jpg,.jpeg,.png"); + uploadConfig.setMaxFileSize(2097152L); // 2MB + uploadConfig.setDescription("images (JPEG, PNG)"); + } else { + // Document upload configuration (default) + uploadConfig.setAllowedMimeTypes("application/pdf,application/msword,application/vnd.openxmlformats-officedocument.wordprocessingml.document"); + uploadConfig.setAllowedExtensions(".pdf,.doc,.docx"); + uploadConfig.setMaxFileSize(5242880L); // 5MB + uploadConfig.setDescription("documents (PDF, Word)"); + } + } + + /** + * Returns the upload configuration object. + * This is used in struts.xml with ${uploadConfig.allowedMimeTypes} expressions. + */ + public UploadConfig getUploadConfig() { + return uploadConfig; + } + + /** + * Configuration holder for dynamic file upload validation rules. + */ + public static class UploadConfig { + private String allowedMimeTypes; + private String allowedExtensions; + private Long maxFileSize; + private String description; + + public String getAllowedMimeTypes() { + return allowedMimeTypes; + } + + public void setAllowedMimeTypes(String allowedMimeTypes) { + this.allowedMimeTypes = allowedMimeTypes; + } + + public String getAllowedExtensions() { + return allowedExtensions; + } + + public void setAllowedExtensions(String allowedExtensions) { + this.allowedExtensions = allowedExtensions; + } + + public Long getMaxFileSize() { + return maxFileSize; + } + + public void setMaxFileSize(Long maxFileSize) { + this.maxFileSize = maxFileSize; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + /** + * Returns a human-readable string representation of the max file size. + */ + public String getMaxFileSizeFormatted() { + if (maxFileSize == null) { + return "unlimited"; + } + + if (maxFileSize < 1024) { + return maxFileSize + " bytes"; + } else if (maxFileSize < 1024 * 1024) { + return (maxFileSize / 1024) + " KB"; + } else { + return (maxFileSize / (1024 * 1024)) + " MB"; + } + } + } +} diff --git a/apps/showcase/src/main/resources/log4j2.xml b/apps/showcase/src/main/resources/log4j2.xml index d7836c717..e1672a856 100644 --- a/apps/showcase/src/main/resources/log4j2.xml +++ b/apps/showcase/src/main/resources/log4j2.xml @@ -30,5 +30,8 @@ + + + diff --git a/apps/showcase/src/main/resources/struts-fileupload.xml b/apps/showcase/src/main/resources/struts-fileupload.xml index 21ce3fb03..e42a8695a 100644 --- a/apps/showcase/src/main/resources/struts-fileupload.xml +++ b/apps/showcase/src/main/resources/struts-fileupload.xml @@ -20,43 +20,66 @@ */ --> + "-//Apache Software Foundation//DTD Struts Configuration 6.0//EN" + "https://struts.apache.org/dtds/struts-6.0.dtd"> - + - + - /WEB-INF/fileupload/upload.jsp - + /WEB-INF/fileupload/upload.jsp + - /WEB-INF/fileupload/upload.jsp - /WEB-INF/fileupload/upload-success.jsp - + /WEB-INF/fileupload/upload.jsp + /WEB-INF/fileupload/upload-success.jsp + - - /WEB-INF/fileupload/multipleUploadUsingList.jsp - + + /WEB-INF/fileupload/multipleUploadUsingList.jsp + - - /WEB-INF/fileupload/multipleUploadUsingList.jsp - /WEB-INF/fileupload/multiple-success.jsp - + + /WEB-INF/fileupload/multipleUploadUsingList.jsp + /WEB-INF/fileupload/multiple-success.jsp + - - /WEB-INF/fileupload/multipleUploadUsingArray.jsp - + + /WEB-INF/fileupload/multipleUploadUsingArray.jsp + - - /WEB-INF/fileupload/multipleUploadUsingArray.jsp - /WEB-INF/fileupload/multiple-success.jsp - + + /WEB-INF/fileupload/multipleUploadUsingArray.jsp + /WEB-INF/fileupload/multiple-success.jsp + + + + /WEB-INF/fileupload/dynamic-upload.jsp + + + + + + ${uploadConfig.allowedMimeTypes} + ${uploadConfig.allowedExtensions} + ${uploadConfig.maxFileSize} + + /WEB-INF/fileupload/dynamic-upload.jsp + /WEB-INF/fileupload/dynamic-upload-success.jsp + diff --git a/apps/showcase/src/main/resources/struts.xml b/apps/showcase/src/main/resources/struts.xml index 45ab3a5e0..1b57083b3 100644 --- a/apps/showcase/src/main/resources/struts.xml +++ b/apps/showcase/src/main/resources/struts.xml @@ -36,14 +36,7 @@ - - + diff --git a/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp b/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp index a020e926d..b2eeaca62 100644 --- a/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp +++ b/apps/showcase/src/main/webapp/WEB-INF/decorators/main.jsp @@ -192,6 +192,10 @@ Single File Upload +
  • + + Single File Upload - dynamic config +
  • Multiple File Upload (List) diff --git a/apps/showcase/src/main/webapp/WEB-INF/fileupload/dynamic-upload-success.jsp b/apps/showcase/src/main/webapp/WEB-INF/fileupload/dynamic-upload-success.jsp new file mode 100644 index 000000000..78ca7086b --- /dev/null +++ b/apps/showcase/src/main/webapp/WEB-INF/fileupload/dynamic-upload-success.jsp @@ -0,0 +1,109 @@ + + +<%@ page + language="java" + contentType="text/html; charset=UTF-8" + pageEncoding="UTF-8" %> +<%@ taglib prefix="s" uri="/struts-tags" %> + + + Struts2 Showcase - Dynamic File Upload Success + + + + + +
    +
    +
    +
    + Success! Your file passed all validation checks. +
    + +
    +
    +

    Upload Details

    +
    +
    +
    +
    Upload Type:
    +
    + +
    Content Type:
    +
    + +
    File Name:
    +
    + +
    Original Name:
    +
    + +
    File Size:
    +
    bytes
    + +
    Input Name:
    +
    + +
    File Object:
    +
    +
    +
    +
    + +
    +
    +

    Validation Rules Applied

    +
    +
    +
    +
    Allowed MIME Types:
    +
    + +
    Allowed Extensions:
    +
    + +
    Maximum Size:
    +
    +
    +

    + + These validation rules were determined dynamically at runtime + using WithLazyParams and evaluated from the ValueStack. + +

    +
    +
    + +
    + + Upload Another File + +
    +
    +
    +
    + + + diff --git a/apps/showcase/src/main/webapp/WEB-INF/fileupload/dynamic-upload.jsp b/apps/showcase/src/main/webapp/WEB-INF/fileupload/dynamic-upload.jsp new file mode 100644 index 000000000..b1f2b50b5 --- /dev/null +++ b/apps/showcase/src/main/webapp/WEB-INF/fileupload/dynamic-upload.jsp @@ -0,0 +1,101 @@ + + +<%@ taglib prefix="s" uri="/struts-tags" %> + + + Struts2 Showcase - Dynamic File Upload Validation + + + + + +
    +
    +
    +
    +

    About This Example

    +

    + This example demonstrates how to use WithLazyParams to configure + file upload validation rules dynamically at runtime. The validation parameters + (allowedTypes, allowedExtensions, maximumSize) + are evaluated from the ValueStack for each request, allowing different rules + based on action state, user permissions, or other runtime conditions. +

    +
    + +
    +

    Current Configuration

    +
      +
    • Upload Type:
    • +
    • Allowed Types: +
    • +
    • Allowed Extensions:
    • +
    • Maximum Size:
    • +
    • Description:
    • +
    +
    +
    +
    + + + + +
    +
    + + + + + + + + +
    +
    + +
    +
    +
    +

    How It Works

    +

    In struts.xml, the interceptor parameters use expressions:

    +
    <interceptor-ref name="actionFileUpload">
    +    <param name="allowedTypes">${uploadConfig.allowedMimeTypes}</param>
    +    <param name="allowedExtensions">${uploadConfig.allowedExtensions}</param>
    +    <param name="maximumSize">${uploadConfig.maxFileSize}</param>
    +</interceptor-ref>
    +

    + These expressions are evaluated at runtime against the ValueStack, + allowing the action to control validation rules dynamically in its + prepare() method. +

    +
    +
    +
    +
    + + diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DynamicFileUploadTest.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DynamicFileUploadTest.java new file mode 100644 index 000000000..863ba0723 --- /dev/null +++ b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/DynamicFileUploadTest.java @@ -0,0 +1,282 @@ +/* + * 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 it.org.apache.struts2.showcase; + +import org.assertj.core.api.Assertions; +import org.htmlunit.WebClient; +import org.htmlunit.html.HtmlFileInput; +import org.htmlunit.html.HtmlForm; +import org.htmlunit.html.HtmlPage; +import org.htmlunit.html.HtmlRadioButtonInput; +import org.htmlunit.html.HtmlSubmitInput; +import org.junit.Test; + +import java.io.File; +import java.io.FileOutputStream; +import java.io.FileWriter; +import java.security.SecureRandom; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Integration tests for dynamic file upload validation feature. + * Tests the WithLazyParams functionality that allows runtime configuration + * of file upload validation rules based on action state. + */ +public class DynamicFileUploadTest { + + @Test + public void testDynamicUploadValidDocument() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + final HtmlForm form = page.getForms().get(0); + + // Select document upload type + HtmlRadioButtonInput documentRadio = form.getInputByValue("document"); + documentRadio.setChecked(true); + + // Create a small PDF-like file + File pdfFile = createTestFile("test.pdf", 1024); + pdfFile.deleteOnExit(); + + HtmlFileInput uploadInput = form.getInputByName("upload"); + uploadInput.setFiles(pdfFile); + + final HtmlSubmitInput button = form.getInputByValue("Upload File"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).contains( + "File Upload Successful", + "Upload Type:\nDocument", + "Original Name:\n" + pdfFile.getName() + ); + } + } + + @Test + public void testDynamicUploadValidImage() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + final HtmlForm form = page.getForms().get(0); + + // Select image upload type + HtmlRadioButtonInput imageRadio = form.getInputByValue("image"); + imageRadio.setChecked(true); + + assertThat(imageRadio) + .isNotNull() + .hasFieldOrProperty("value") + .isNotNull() + .extracting(HtmlRadioButtonInput::isChecked) + .asInstanceOf(Assertions.BOOLEAN).isTrue(); + + // Create a small image-like file + File imageFile = createTestFile("test.png", 1024); + imageFile.deleteOnExit(); + + HtmlFileInput uploadInput = form.getInputByName("upload"); + uploadInput.setFiles(imageFile); + + final HtmlSubmitInput button = form.getInputByValue("Upload File"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).contains( + "File Upload Successful", + "Upload Type:\nImage", + "Original Name:\n" + imageFile.getName() + ); + } + } + + @Test + public void testDynamicUploadDocumentRejectsImage() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + final HtmlForm form = page.getForms().get(0); + + // Select document upload type + HtmlRadioButtonInput documentRadio = form.getInputByValue("document"); + documentRadio.setChecked(true); + + // Try to upload an image file + File imageFile = createTestFile("test.jpg", 512); + imageFile.deleteOnExit(); + + HtmlFileInput uploadInput = form.getInputByName("upload"); + uploadInput.setFiles(imageFile); + + final HtmlSubmitInput button = form.getInputByValue("Upload File"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).contains( + "Content-Type not allowed", + "image/jpeg", + "File extension not allowed" + + ); + } + } + + @Test + public void testDynamicUploadImageRejectsDocument() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + final HtmlForm form = page.getForms().get(0); + + // Select image upload type + HtmlRadioButtonInput imageRadio = form.getInputByValue("image"); + imageRadio.setChecked(true); + + // Try to upload a PDF file + File pdfFile = createTestFile("test.pdf", 512); + pdfFile.deleteOnExit(); + + HtmlFileInput uploadInput = form.getInputByName("upload"); + uploadInput.setFiles(pdfFile); + + final HtmlSubmitInput button = form.getInputByValue("Upload File"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).contains( + "Content-Type not allowed", + "application/pdf", + "File extension not allowed" + ); + } + } + + @Test + public void testDynamicUploadDocumentExceedsMaxSize() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + final HtmlForm form = page.getForms().get(0); + + // Select document upload type (max 5MB) + HtmlRadioButtonInput documentRadio = form.getInputByValue("document"); + documentRadio.setChecked(true); + + // Create a file larger than 5MB + File largeFile = createLargeTestFile("large.pdf", 5 * 1024 * 1024 + 1024); + largeFile.deleteOnExit(); + + HtmlFileInput uploadInput = form.getInputByName("upload"); + uploadInput.setFiles(largeFile); + + final HtmlSubmitInput button = form.getInputByValue("Upload File"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).contains("Request exceeded allowed size limit! Max size allowed is:"); + } + } + + @Test + public void testDynamicUploadImageExceedsMaxSize() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + final HtmlForm form = page.getForms().get(0); + + // Select image upload type (max 2MB) + HtmlRadioButtonInput imageRadio = form.getInputByValue("image"); + imageRadio.setChecked(true); + + // Create a file larger than 2MB + File largeFile = createLargeTestFile("large.png", 2 * 1024 * 1024 + 1024); + largeFile.deleteOnExit(); + + HtmlFileInput uploadInput = form.getInputByName("upload"); + uploadInput.setFiles(largeFile); + + final HtmlSubmitInput button = form.getInputByValue("Upload File"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).containsAnyOf("size", "Size", "limit", "exceed"); + } + } + + @Test + public void testDynamicUploadSwitchBetweenModes() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/dynamicUpload.action"); + + // Verify initial state shows document mode by default + String initialContent = page.getVisibleText(); + assertThat(initialContent).contains("Document Upload"); + + final HtmlForm form = page.getForms().get(0); + + // Switch to image mode and refresh rules + HtmlRadioButtonInput imageRadio = form.getInputByValue("image"); + imageRadio.setChecked(true); + + final HtmlSubmitInput refreshButton = form.getInputByValue("Refresh Rules"); + final HtmlPage refreshedPage = refreshButton.click(); + + // Verify rules changed to image mode + String refreshedContent = refreshedPage.getVisibleText(); + assertThat(refreshedContent).contains( + "Image Upload", + "image/jpeg", + "2 MB" + ); + } + } + + /** + * Creates a small test file with specified name and extension. + */ + private File createTestFile(String fileName, int sizeInBytes) throws Exception { + File tempFile = File.createTempFile("test_", fileName); + try (FileWriter writer = new FileWriter(tempFile)) { + // Write some content to make it non-empty + for (int i = 0; i < sizeInBytes; i++) { + writer.write('A'); + } + writer.flush(); + } + return tempFile; + } + + /** + * Creates a large test file for size limit testing. + */ + private File createLargeTestFile(String fileName, int sizeInBytes) throws Exception { + File tempFile = File.createTempFile("large_test_", fileName); + SecureRandom rng = new SecureRandom(); + + try (FileOutputStream fos = new FileOutputStream(tempFile)) { + byte[] buffer = new byte[8192]; + int remaining = sizeInBytes; + + while (remaining > 0) { + int toWrite = Math.min(buffer.length, remaining); + rng.nextBytes(buffer); + fos.write(buffer, 0, toWrite); + remaining -= toWrite; + } + fos.flush(); + } + return tempFile; + } +} diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/ParameterUtils.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/ParameterUtils.java index ddfdf4361..0cac990d4 100644 --- a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/ParameterUtils.java +++ b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/ParameterUtils.java @@ -25,7 +25,7 @@ public class ParameterUtils { public static String getBaseUrl() { String port = System.getProperty("http.port"); if (port == null) { - port = "8080"; + port = "8090"; } return "http://localhost:"+port+"/struts2-showcase"; } diff --git a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java index cc477abc2..6885dd50d 100644 --- a/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java +++ b/core/src/main/java/org/apache/struts2/DefaultActionInvocation.java @@ -259,7 +259,14 @@ public class DefaultActionInvocation implements ActionInvocation { final InterceptorMapping interceptorMapping = interceptors.next(); Interceptor interceptor = interceptorMapping.getInterceptor(); if (interceptor instanceof WithLazyParams) { - interceptor = lazyParamInjector.injectParams(interceptor, interceptorMapping.getParams(), invocationContext); + 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) { resultCode = executeConditional(conditionalInterceptor); 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 8d7ef49f1..ee698721b 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java @@ -110,8 +110,8 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor Set errorMessages = new HashSet<>(); ValidationAware validation = null; - if (action instanceof ValidationAware) { - validation = (ValidationAware) action; + if (action instanceof ValidationAware validationAware) { + validation = validationAware; } // If it's null the upload failed @@ -125,7 +125,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor } if (file.getContent() == null) { - String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOADING_KEY, new String[]{originalFilename}); + String errMsg = getTextMessage(action, STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY, new String[]{originalFilename}); errorMessages.add(errMsg); LOG.warn(errMsg); } @@ -200,24 +200,13 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor return matcher.match(new HashMap<>(), text, o); } - protected boolean isNonEmpty(Object[] objArray) { - boolean result = false; - for (Object o : objArray) { - if (o != null) { - result = true; - break; - } - } - return result; - } - protected String getTextMessage(String messageKey, String[] args) { return getTextMessage(this, messageKey, args); } protected String getTextMessage(Object action, String messageKey, String[] args) { - if (action instanceof TextProvider) { - return ((TextProvider) action).getText(messageKey, args); + if (action instanceof TextProvider textProvider) { + return textProvider.getText(messageKey, args); } return getTextProvider(action).getText(messageKey, args); } @@ -229,8 +218,8 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor private LocaleProvider getLocaleProvider(Object action) { LocaleProvider localeProvider; - if (action instanceof LocaleProvider) { - localeProvider = (LocaleProvider) action; + if (action instanceof LocaleProvider lp) { + localeProvider = lp; } else { LocaleProviderFactory localeProviderFactory = container.getInstance(LocaleProviderFactory.class); localeProvider = localeProviderFactory.createLocaleProvider(); @@ -240,8 +229,8 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor protected void applyValidation(Object action, MultiPartRequestWrapper multiWrapper) { ValidationAware validation = null; - if (action instanceof ValidationAware) { - validation = (ValidationAware) action; + if (action instanceof ValidationAware va) { + validation = va; } if (multiWrapper.hasErrors() && validation != null) { 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 911a6e4a9..79d020a34 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java @@ -71,6 +71,79 @@ import java.util.List; * a file reference to be set on the action. If none is specified allow all extensions to be uploaded.
  • * * + *

    Dynamic Parameter Evaluation

    + *

    + * This interceptor implements {@link WithLazyParams}, which enables dynamic parameter evaluation at runtime. + * Parameters can use ${...} expressions that will be evaluated against the ValueStack for each request, + * allowing file upload validation rules to be determined dynamically based on action properties, session data, + * or other runtime values. + *

    + * + *

    Static configuration example:

    + *
    + * <action name="upload" class="com.example.UploadAction">
    + *     <interceptor-ref name="actionFileUpload">
    + *         <param name="allowedTypes">image/jpeg,image/png,application/pdf</param>
    + *         <param name="allowedExtensions">.jpg,.png,.pdf</param>
    + *         <param name="maximumSize">5242880</param>
    + *     </interceptor-ref>
    + *     <interceptor-ref name="basicStack"/>
    + * </action>
    + * 
    + * + *

    Dynamic configuration example:

    + *
    + * <action name="dynamicUpload" class="com.example.DynamicUploadAction">
    + *     <interceptor-ref name="actionFileUpload">
    + *         <param name="allowedTypes">${uploadConfig.allowedMimeTypes}</param>
    + *         <param name="allowedExtensions">${uploadConfig.allowedExtensions}</param>
    + *         <param name="maximumSize">${uploadConfig.maxFileSize}</param>
    + *     </interceptor-ref>
    + *     <interceptor-ref name="basicStack"/>
    + * </action>
    + * 
    + * + *

    Action class with dynamic configuration:

    + *
    + *  package com.example;
    + *
    + *  import org.apache.struts2.ActionSupport;
    + *  import org.apache.struts2.action.UploadedFilesAware;
    + *
    + *  public class DynamicUploadAction extends ActionSupport implements UploadedFilesAware {
    + *    private UploadedFile uploadedFile;
    + *    private UploadConfig uploadConfig;
    + *
    + *    public void prepare() {
    + *        // Load configuration dynamically (from database, properties, etc.)
    + *        uploadConfig = new UploadConfig();
    + *        uploadConfig.setAllowedMimeTypes("image/jpeg,image/png");
    + *        uploadConfig.setAllowedExtensions(".jpg,.png");
    + *        uploadConfig.setMaxFileSize(5242880L);
    + *    }
    + *
    + *    @Override
    + *    public void withUploadedFiles(List<UploadedFile> uploadedFiles) {
    + *        if (!uploadedFiles.isEmpty()) {
    + *            this.uploadedFile = uploadedFiles.get(0);
    + *        }
    + *    }
    + *
    + *    public UploadConfig getUploadConfig() {
    + *        return uploadConfig;
    + *    }
    + *
    + *    public String execute() {
    + *       //...
    + *       return SUCCESS;
    + *    }
    + *  }
    + * 
    + * + *

    Performance Note: When using dynamic parameters with ${...} expressions, + * parameters are evaluated for each request. For static validation rules, use literal values for better performance. + *

    + * *

    Example code:

    * *
    @@ -124,8 +197,12 @@ import java.util.List;
      *    }
      *  }
      * 
    + * + * @see WithLazyParams + * @see UploadedFilesAware + * @see AbstractFileUploadInterceptor */ -public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor { +public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor implements WithLazyParams { protected static final Logger LOG = LogManager.getLogger(ActionFileUploadInterceptor.class); 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 26cf7e180..61339327d 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@ -570,6 +570,260 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { }; } + /** + * Tests WithLazyParams functionality - verifies that the interceptor implements + * the WithLazyParams interface for dynamic parameter evaluation + */ + public void testImplementsWithLazyParams() { + assertThat(interceptor).isInstanceOf(WithLazyParams.class); + } + + /** + * Tests dynamic parameter evaluation with ${...} expressions from ValueStack + */ + public void testDynamicParameterEvaluation() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); + + // Upload two files with different content types + String content = encodeTextFile("test.txt", "text/plain", plainContent) + + encodeTextFile("test.html", "text/html", htmlContent) + + endLine + "--" + boundary + "--"; + request.setContent(content.getBytes()); + + MyDynamicFileUploadAction action = new MyDynamicFileUploadAction(); + action.setAllowedMimeTypes("text/plain"); // Only text/plain allowed + container.inject(action); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + + // Push action to ValueStack so ${allowedMimeTypes} can be resolved + ActionContext.getContext().getValueStack().push(action); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + // Simulate WithLazyParams injection by manually setting the parameters + // In real execution, DefaultActionInvocation.invoke() would call LazyParamInjector + interceptor.setAllowedTypes(action.getAllowedMimeTypes()); + + interceptor.intercept(mai); + + List files = action.getUploadFiles(); + + // Only the text/plain file should be accepted + assertThat(files).isNotNull().hasSize(1); + assertThat(files.get(0).getContentType()).isEqualTo("text/plain"); + assertThat(files.get(0).getOriginalName()).isEqualTo("test.txt"); + } + + /** + * Tests that dynamic parameters can change between requests + */ + public void testDynamicParametersChangePerRequest() throws Exception { + // First request - allow only text/plain + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); + + String content = encodeTextFile("test.txt", "text/plain", plainContent) + + endLine + "--" + boundary + "--"; + request.setContent(content.getBytes()); + + MyDynamicFileUploadAction action1 = new MyDynamicFileUploadAction(); + action1.setAllowedMimeTypes("text/plain"); + container.inject(action1); + + MockActionInvocation mai1 = new MockActionInvocation(); + mai1.setAction(action1); + mai1.setResultCode("success"); + mai1.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().getValueStack().push(action1); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + interceptor.setAllowedTypes(action1.getAllowedMimeTypes()); + interceptor.intercept(mai1); + + assertThat(action1.getUploadFiles()).isNotNull().hasSize(1); + assertThat(action1.getUploadFiles().get(0).getContentType()).isEqualTo("text/plain"); + + // Second request - allow only text/html + request = new MockHttpServletRequest(); + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); + + content = encodeTextFile("test.html", "text/html", htmlContent) + + endLine + "--" + boundary + "--"; + request.setContent(content.getBytes()); + + MyDynamicFileUploadAction action2 = new MyDynamicFileUploadAction(); + action2.setAllowedMimeTypes("text/html"); + container.inject(action2); + + MockActionInvocation mai2 = new MockActionInvocation(); + mai2.setAction(action2); + mai2.setResultCode("success"); + mai2.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().getValueStack().pop(); // Remove previous action + ActionContext.getContext().getValueStack().push(action2); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + // Simulate new parameter evaluation for second request + interceptor.setAllowedTypes(action2.getAllowedMimeTypes()); + interceptor.intercept(mai2); + + assertThat(action2.getUploadFiles()).isNotNull().hasSize(1); + assertThat(action2.getUploadFiles().get(0).getContentType()).isEqualTo("text/html"); + } + + /** + * Tests dynamic extension validation + */ + public void testDynamicExtensionValidation() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); + + String content = encodeTextFile("test.pdf", "application/pdf", "PDF content") + + encodeTextFile("test.doc", "application/msword", "DOC content") + + endLine + "--" + boundary + "--"; + request.setContent(content.getBytes()); + + MyDynamicFileUploadAction action = new MyDynamicFileUploadAction(); + action.setAllowedExtensions(".pdf"); + container.inject(action); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().getValueStack().push(action); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + interceptor.setAllowedExtensions(action.getAllowedExtensions()); + interceptor.intercept(mai); + + List files = action.getUploadFiles(); + + // Only the .pdf file should be accepted + assertThat(files).isNotNull().hasSize(1); + assertThat(files.get(0).getOriginalName()).isEqualTo("test.pdf"); + } + + /** + * Tests dynamic maximum size validation + */ + public void testDynamicMaximumSizeValidation() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=---1234"); + + String content = (""" + -----1234\r + Content-Disposition: form-data; name="file"; filename="test.txt"\r + Content-Type: text/plain\r + \r + This is a test file with some content\r + -----1234--\r + """); + request.setContent(content.getBytes(StandardCharsets.US_ASCII)); + + MyDynamicFileUploadAction action = new MyDynamicFileUploadAction(); + action.setMaxFileSize(10L); // Very small size to trigger validation error + container.inject(action); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().getValueStack().push(action); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + interceptor.setMaximumSize(action.getMaxFileSize()); + interceptor.intercept(mai); + + // File should be rejected due to size + assertThat(action.hasFieldErrors()).isTrue(); + assertThat(action.getFieldErrors().get("file")).isNotEmpty(); + } + + /** + * Tests that security validation still works correctly with dynamic parameters + */ + public void testSecurityValidationWithDynamicParameters() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); + + // Try to upload files with various potentially dangerous content types + String content = encodeTextFile("script.js", "application/javascript", "alert('xss')") + + encodeTextFile("test.pdf", "application/pdf", "PDF content") + + endLine + "--" + boundary + "--"; + request.setContent(content.getBytes()); + + MyDynamicFileUploadAction action = new MyDynamicFileUploadAction(); + action.setAllowedMimeTypes("application/pdf"); + action.setAllowedExtensions(".pdf"); + container.inject(action); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().getValueStack().push(action); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + interceptor.setAllowedTypes(action.getAllowedMimeTypes()); + interceptor.setAllowedExtensions(action.getAllowedExtensions()); + interceptor.intercept(mai); + + List files = action.getUploadFiles(); + + // Only the PDF should be accepted, JavaScript file should be rejected + assertThat(files).isNotNull().hasSize(1); + assertThat(files.get(0).getOriginalName()).isEqualTo("test.pdf"); + assertThat(files.get(0).getContentType()).isEqualTo("application/pdf"); + } + + /** + * Tests wildcard matching with dynamic parameters + */ + public void testWildcardMatchingWithDynamicParameters() throws Exception { + request.setCharacterEncoding(StandardCharsets.UTF_8.name()); + request.setMethod("POST"); + request.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); + + String content = encodeTextFile("test.jpg", "image/jpeg", "JPEG content") + + encodeTextFile("test.png", "image/png", "PNG content") + + encodeTextFile("test.html", "text/html", htmlContent) + + endLine + "--" + boundary + "--"; + request.setContent(content.getBytes()); + + MyDynamicFileUploadAction action = new MyDynamicFileUploadAction(); + action.setAllowedMimeTypes("image/*"); // Accept all image types + container.inject(action); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + ActionContext.getContext().getValueStack().push(action); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles()); + + interceptor.setAllowedTypes(action.getAllowedMimeTypes()); + interceptor.intercept(mai); + + List files = action.getUploadFiles(); + + // Both image files should be accepted, HTML should be rejected + assertThat(files).isNotNull().hasSize(2); + assertThat(files.get(0).getContentType()).startsWith("image/"); + assertThat(files.get(1).getContentType()).startsWith("image/"); + } + public static class MyFileUploadAction extends ActionSupport implements UploadedFilesAware { private List uploadedFiles; @@ -583,4 +837,47 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { } } + /** + * Test action class that demonstrates dynamic file upload configuration + */ + public static class MyDynamicFileUploadAction extends ActionSupport implements UploadedFilesAware { + private List uploadedFiles; + private String allowedMimeTypes; + private String allowedExtensions; + private Long maxFileSize; + + @Override + public void withUploadedFiles(List uploadedFiles) { + this.uploadedFiles = uploadedFiles; + } + + public List getUploadFiles() { + return this.uploadedFiles; + } + + public String getAllowedMimeTypes() { + return allowedMimeTypes; + } + + public void setAllowedMimeTypes(String allowedMimeTypes) { + this.allowedMimeTypes = allowedMimeTypes; + } + + public String getAllowedExtensions() { + return allowedExtensions; + } + + public void setAllowedExtensions(String allowedExtensions) { + this.allowedExtensions = allowedExtensions; + } + + public Long getMaxFileSize() { + return maxFileSize; + } + + public void setMaxFileSize(Long maxFileSize) { + this.maxFileSize = maxFileSize; + } + } + } diff --git a/thoughts/shared/research/2025-10-22-dynamic-file-upload-validation.md b/thoughts/shared/research/2025-10-22-dynamic-file-upload-validation.md new file mode 100644 index 000000000..b188dc056 --- /dev/null +++ b/thoughts/shared/research/2025-10-22-dynamic-file-upload-validation.md @@ -0,0 +1,590 @@ +--- +date: 2025-10-22T00:00:00Z +topic: "Dynamic File Upload Validation Without Custom Interceptors" +tags: [research, codebase, file-upload, validation, interceptor, UploadedFilesAware] +status: complete +git_commit: 06f9f9303387edf0557d128bbd7123bded4f24f5 +--- + +# Research: Dynamic File Upload Validation Without Custom Interceptors + +**Date**: 2025-10-22 + +## Research Question + +User asked: How can I dynamically set `allowedTypes` and `allowedExtensions` for file upload validation from my action class instead of static configuration in struts.xml? + +Specific issues: +1. Static configuration works: `application/pdf` +2. Dynamic expression doesn't work: `${acceptedFileTypes}` (due to TextParseUtil.commaDelimitedStringToSet) +3. Documentation mentions `setXContentType(String contentType)` method but cannot find it in Struts 7 core +4. Needs to match validation logic with parallel batch import function (without Struts) +5. Wants to avoid writing a custom interceptor + +## Summary + +**Key Findings:** + +1. **`${...}` expressions don't work in interceptor parameters** because `TextParseUtil.commaDelimitedStringToSet()` is a pure string splitter with no OGNL evaluation, and interceptor parameters are set at startup when no ValueStack exists. + +2. **`setXContentType` is a deprecated pattern** from pre-Struts 6.4.0 using naming conventions (`setUploadContentType`). Modern approach uses `UploadedFilesAware` interface with `UploadedFile` API. + +3. **Recommended solution: Programmatic validation in action** by implementing: + - `UploadedFilesAware` interface to receive files + - `validate()` method for custom validation logic + - Shared configuration class for both Struts and batch import + - No custom interceptor needed + +4. **Alternative if needed:** Implement `WithLazyParams` interface for runtime parameter evaluation (but with per-request overhead). + +## Detailed Findings + +### 1. Why `${acceptedFileTypes}` Doesn't Work + +**File**: [core/src/main/java/org/apache/struts2/util/TextParseUtil.java#L256](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/util/TextParseUtil.java#L256) + +```java +public static Set commaDelimitedStringToSet(String s) { + return Arrays.stream(s.split(",")) + .map(String::trim) + .filter(s1 -> !s1.isEmpty()) + .collect(Collectors.toSet()); +} +``` + +**The Problem:** +- This is a **pure string splitter** with zero OGNL evaluation +- If you pass `"${acceptedFileTypes}"`, it creates a Set with the literal string `"${acceptedFileTypes}"` +- No expression evaluation happens + +**Root Cause - Parameter Processing Lifecycle:** + +**Phase 1: XML Parsing (Startup)** +- File: [core/src/main/java/org/apache/struts2/config/providers/XmlHelper.java#L73-95](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/config/providers/XmlHelper.java#L73-L95) +- Parameters extracted as **literal strings** from XML +- `${foo}` remains as the string `"${foo}"` + +**Phase 2: Interceptor Instantiation (Startup)** +- File: [core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java#L54-81](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java#L54-L81) +- Properties set via `reflectionProvider.setProperties(params, interceptor)` +- OGNL is used to **set** properties, not **evaluate** the value strings +- No `ActionContext` or `ValueStack` exists yet (happens at startup) + +**Phase 3: Setter Execution** +- File: [core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java#L78](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java#L78) + +```java +public void setAllowedExtensions(String allowedExtensions) { + allowedExtensionsSet = TextParseUtil.commaDelimitedStringToSet(allowedExtensions); +} +``` + +- Receives literal string `"${acceptedFileTypes}"` +- No evaluation mechanism available +- Creates Set with that literal value + +### 2. About `setXContentType` Method + +**The Old Pattern (Deprecated - Pre-Struts 6.4.0):** + +Used automatic property binding with naming conventions: +- `setUpload(File file)` - receives the uploaded file +- `setUploadContentType(String contentType)` - receives the content type +- `setUploadFileName(String fileName)` - receives the original filename + +**Example**: [apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java#L245-246](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java#L245-L246) + +```java +public class UITagExample extends ActionSupport { + File picture; + String pictureContentType; // Notice the naming pattern + String pictureFileName; + + @StrutsParameter + public void setPicture(File picture) { + this.picture = picture; + } + + @StrutsParameter + public void setPictureContentType(String pictureContentType) { + this.pictureContentType = pictureContentType; + } + + @StrutsParameter + public void setPictureFileName(String pictureFileName) { + this.pictureFileName = pictureFileName; + } +} +``` + +**The Modern Pattern (Struts 6.4.0+):** + +Uses `UploadedFilesAware` interface with `UploadedFile` API: + +**Interface**: [core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java) + +**Example**: [apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java) + +```java +public class FileUploadAction extends ActionSupport implements UploadedFilesAware { + private UploadedFile uploadedFile; + private String contentType; + private String fileName; + private String originalName; + + @Override + public void withUploadedFiles(List uploadedFiles) { + this.uploadedFile = uploadedFiles.get(0); + this.contentType = uploadedFile.getContentType(); + this.fileName = uploadedFile.getName(); + this.originalName = uploadedFile.getOriginalName(); + } + + public String execute() { + // Programmatic validation possible here + if (contentType != null && !contentType.equals("application/pdf")) { + addFieldError("upload", "Only PDF files are allowed"); + return ERROR; + } + return SUCCESS; + } +} +``` + +### 3. File Upload Validation Architecture + +**Core Interceptor**: [core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java#L105-167](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java#L105-L167) + +The `acceptFile()` method provides validation: + +```java +protected boolean acceptFile(Object action, UploadedFile file, String originalFilename, + String contentType, String inputName) { + Set errorMessages = new HashSet<>(); + ValidationAware validation = null; + + if (action instanceof ValidationAware) { + validation = (ValidationAware) action; + } + + // Validation checks: + // 1. Null file check + if (file == null || file.getContent() == null) { + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOADING_KEY, + new String[]{inputName}); + if (validation != null) { + validation.addFieldError(inputName, errMsg); + } + return false; + } + + // 2. File size validation (line 139-143) + if (maximumSize != null && maximumSize < file.length()) { + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY, + new String[]{inputName, originalFilename, file.getName(), + "" + file.length(), getMaximumSizeStr(action)}); + errorMessages.add(errMsg); + } + + // 3. Content type validation (line 144-150) + if ((!allowedTypesSet.isEmpty()) && (!containsItem(allowedTypesSet, contentType))) { + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY, + new String[]{inputName, originalFilename, file.getName(), contentType}); + errorMessages.add(errMsg); + } + + // 4. File extension validation (line 151-157) + if ((!allowedExtensionsSet.isEmpty()) && (!hasAllowedExtension(allowedExtensionsSet, originalFilename))) { + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY, + new String[]{inputName, originalFilename, file.getName(), contentType}); + errorMessages.add(errMsg); + } + + if (validation != null) { + for (String errorMsg : errorMessages) { + validation.addFieldError(inputName, errorMsg); + } + } + + return errorMessages.isEmpty(); +} +``` + +**Extension Matching**: [AbstractFileUploadInterceptor.java#L169-181](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java#L169-L181) + +```java +private boolean hasAllowedExtension(Collection extensionCollection, String filename) { + if (filename == null) { + return false; + } + + String lowercaseFilename = filename.toLowerCase(); + for (String extension : extensionCollection) { + if (lowercaseFilename.endsWith(extension)) { + return true; + } + } + + return false; +} +``` + +**Content Type Matching with Wildcards**: [AbstractFileUploadInterceptor.java#L183-196](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java#L183-L196) + +```java +private boolean containsItem(Collection itemCollection, String item) { + for (String pattern : itemCollection) + if (matchesWildcard(pattern, item)) + return true; + return false; +} + +private boolean matchesWildcard(String pattern, String text) { + Object o = matcher.compilePattern(pattern); + return matcher.match(new HashMap<>(), text, o); +} +``` + +Supports patterns like `text/*` matching `text/plain`, `text/html`, etc. + +### 4. Programmatic Validation Examples + +**Multiple File Upload**: [apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/MultipleFileUploadUsingArrayAction.java](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/MultipleFileUploadUsingArrayAction.java) + +**XML-Based Validation**: [apps/showcase/src/main/resources/org/apache/struts2/showcase/fileupload/FileUploadAction-validation.xml](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/apps/showcase/src/main/resources/org/apache/struts2/showcase/fileupload/FileUploadAction-validation.xml) + +```xml + + + + 0]]> + File cannot be empty + + + +``` + +**Test Example**: [core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java) + +Shows how interceptor and ValidationAware integration works. + +### 5. Alternative: WithLazyParams Interface (Advanced) + +**File**: [core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java#L39-76](https://github.com/apache/struts/blob/06f9f9303387edf0557d128bbd7123bded4f24f5/core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java#L39-L76) + +```java +public interface WithLazyParams { + class LazyParamInjector { + public Interceptor injectParams(Interceptor interceptor, + Map params, + ActionContext invocationContext) { + for (Map.Entry entry : params.entrySet()) { + // CRITICAL: This DOES evaluate ${...} expressions + Object paramValue = textParser.evaluate( + new char[]{ '$' }, + entry.getValue(), + valueEvaluator, // Uses ValueStack.findValue() + TextParser.DEFAULT_LOOP_COUNT + ); + ognlUtil.setProperty(entry.getKey(), paramValue, interceptor, + invocationContext.getContextMap()); + } + return interceptor; + } + } +} +``` + +**How it works:** +- Interceptors implementing `WithLazyParams` skip parameter setting during initialization +- Parameters are injected **per-request** during action invocation +- `textParser.evaluate()` resolves `${...}` expressions against ValueStack +- Happens in `DefaultActionInvocation.invoke()` + +**Limitations:** +- Per-request evaluation overhead +- Requires custom interceptor implementation +- More complex than programmatic validation + +## Code References + +### Core Files +- `core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java:78` - `setAllowedExtensions()` setter +- `core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java:85` - `setAllowedTypes()` setter +- `core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java:105-167` - `acceptFile()` validation logic +- `core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java` - Concrete implementation +- `core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java` - Modern interface for file handling +- `core/src/main/java/org/apache/struts2/util/TextParseUtil.java:256` - `commaDelimitedStringToSet()` method +- `core/src/main/java/org/apache/struts2/config/providers/XmlHelper.java:73-95` - XML parameter parsing +- `core/src/main/java/org/apache/struts2/factory/DefaultInterceptorFactory.java:54-81` - Interceptor instantiation +- `core/src/main/java/org/apache/struts2/interceptor/WithLazyParams.java:39-76` - Lazy parameter evaluation + +### Example Files +- `apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java` - Modern UploadedFilesAware example +- `apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/MultipleFileUploadUsingArrayAction.java` - Multiple file upload +- `apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/MultipleFileUploadUsingListAction.java` - List-based upload +- `apps/showcase/src/main/java/org/apache/struts2/showcase/UITagExample.java:245-246` - Old setXContentType pattern +- `apps/showcase/src/main/resources/org/apache/struts2/showcase/fileupload/FileUploadAction-validation.xml` - XML validation example + +### Test Files +- `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java` - Validation integration tests + +## Architecture Insights + +### 1. Parameter Processing Timeline + +| Phase | When | OGNL Evaluation | ValueStack Available | Can Use ${...} | +|-------|------|-----------------|---------------------|----------------| +| **XML Parsing** | Startup | ❌ No | ❌ No | ❌ No | +| **Interceptor Init** | Startup | ⚠️ Property names only | ❌ No | ❌ No | +| **Setter Methods** | Startup | ❌ No | ❌ No | ❌ No | +| **WithLazyParams** | Per-request | ✅ Yes | ✅ Yes | ✅ Yes | +| **Request Processing** | Per-request | ✅ Yes | ✅ Yes | ✅ Yes | +| **Action validate()** | Per-request | ✅ Yes | ✅ Yes | ✅ Yes | + +### 2. Validation Layers + +**Layer 1: Interceptor (Pre-Action)** +- Configured via struts.xml parameters +- Executes before action +- Adds field errors to ValidationAware +- Cannot access action properties + +**Layer 2: Action validate() Method** +- Executes after interceptor +- Full access to UploadedFile objects +- Can implement dynamic business logic +- Can load configuration from database/properties + +**Layer 3: XML Validation** +- Declarative validation rules +- Can reference action methods via expressions +- Executes after validate() method + +### 3. Security Features + +1. **Case-insensitive extension matching** - Prevents bypassing via uppercase extensions +2. **Wildcard support for content types** - Allows flexible type matching (e.g., `text/*`) +3. **Multiple validation layers** - Defense in depth +4. **Integration with ValidationAware** - Consistent error handling +5. **Null safety** - Handles null files and content appropriately + +## Recommended Solution + +### Complete Implementation Example + +```java +package com.example; + +import org.apache.struts2.ActionSupport; +import org.apache.struts2.action.UploadedFilesAware; +import org.apache.struts2.dispatcher.multipart.UploadedFile; +import java.util.Arrays; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class DynamicFileUploadAction extends ActionSupport implements UploadedFilesAware { + + private UploadedFile uploadedFile; + private String contentType; + private String originalName; + + // Dynamic configuration - shared with batch import + private Set acceptedFileTypes; + private Set acceptedFileExtensions; + + public DynamicFileUploadAction() { + // Load dynamic configuration + // This matches your batch import function's configuration + acceptedFileTypes = FileUploadConfig.getAcceptedTypes(); + acceptedFileExtensions = FileUploadConfig.getAcceptedExtensions(); + } + + @Override + public void withUploadedFiles(List uploadedFiles) { + if (!uploadedFiles.isEmpty()) { + this.uploadedFile = uploadedFiles.get(0); + this.contentType = uploadedFile.getContentType(); + this.originalName = uploadedFile.getOriginalName(); + } + } + + @Override + public void validate() { + if (uploadedFile == null) { + addFieldError("upload", "Please select a file to upload"); + return; + } + + // Validate content type + if (!isValidContentType(contentType)) { + addFieldError("upload", + "File type not allowed: " + contentType + + ". Allowed types: " + acceptedFileTypes); + } + + // Validate file extension + if (!hasValidExtension(originalName)) { + addFieldError("upload", + "File extension not allowed. Allowed extensions: " + acceptedFileExtensions); + } + + // Additional validation + if (uploadedFile.length() == 0) { + addFieldError("upload", "File cannot be empty"); + } + } + + private boolean isValidContentType(String contentType) { + if (contentType == null || acceptedFileTypes.isEmpty()) { + return false; + } + + // Support wildcard matching (e.g., "image/*") + for (String allowedType : acceptedFileTypes) { + if (matchesWildcard(allowedType, contentType)) { + return true; + } + } + return false; + } + + private boolean hasValidExtension(String filename) { + if (filename == null || acceptedFileExtensions.isEmpty()) { + return false; + } + + String lowerFilename = filename.toLowerCase(); + for (String extension : acceptedFileExtensions) { + if (lowerFilename.endsWith(extension.toLowerCase())) { + return true; + } + } + return false; + } + + private boolean matchesWildcard(String pattern, String text) { + if (pattern.contains("*")) { + String prefix = pattern.substring(0, pattern.indexOf("*")); + return text.startsWith(prefix); + } + return pattern.equals(text); + } + + public String execute() { + // Process the uploaded file + // uploadedFile.getContent() gives you InputStream + return SUCCESS; + } + + // Getters for JSP access + public String getContentType() { return contentType; } + public String getOriginalName() { return originalName; } +} +``` + +### Shared Configuration Class + +```java +public class FileUploadConfig { + private static final Set ACCEPTED_TYPES; + private static final Set ACCEPTED_EXTENSIONS; + + static { + // Load from properties file or database + Properties props = loadProperties("file-upload-config.properties"); + ACCEPTED_TYPES = parseCommaSeparated(props.getProperty("accepted.types")); + ACCEPTED_EXTENSIONS = parseCommaSeparated(props.getProperty("accepted.extensions")); + } + + public static Set getAcceptedTypes() { + return new HashSet<>(ACCEPTED_TYPES); + } + + public static Set getAcceptedExtensions() { + return new HashSet<>(ACCEPTED_EXTENSIONS); + } + + private static Set parseCommaSeparated(String value) { + return Arrays.stream(value.split(",")) + .map(String::trim) + .collect(Collectors.toSet()); + } + + private static Properties loadProperties(String filename) { + Properties props = new Properties(); + try (InputStream is = FileUploadConfig.class.getClassLoader() + .getResourceAsStream(filename)) { + props.load(is); + } catch (IOException e) { + throw new RuntimeException("Failed to load config", e); + } + return props; + } +} +``` + +### struts.xml Configuration + +```xml + + + + 5242880 + + + upload-success.jsp + upload-form.jsp + +``` + +### Configuration Properties File + +```properties +# file-upload-config.properties +# Shared between Struts upload and batch import +accepted.types=application/pdf,image/jpeg,image/png +accepted.extensions=.pdf,.jpg,.jpeg,.png +``` + +## Benefits of Recommended Solution + +✅ **No custom interceptor needed** - Uses standard Struts patterns +✅ **Dynamic configuration loading** - Can change without recompilation +✅ **Shared config between Struts and batch import** - Single source of truth +✅ **Full control over validation logic** - Can add business-specific rules +✅ **Standard validation error handling** - Integrates with Struts validation +✅ **Wildcard support** - Same pattern matching as interceptor +✅ **Case-insensitive extension matching** - Consistent with Struts security practices + +## Related Research + +- File upload security patterns in Apache Struts +- Interceptor lifecycle and parameter injection +- OGNL expression evaluation contexts +- Struts validation framework integration + +## Open Questions + +1. **Performance consideration**: Should configuration be cached vs loaded per-action instance? +2. **Configuration source**: Database vs properties file vs external service? +3. **Validation error messages**: Should they be internationalized (i18n)? +4. **Batch import integration**: Should validation logic be extracted to a shared service class? +5. **Multiple file handling**: Should validation apply to all files or per-file basis? + +## Alternative Approaches Considered + +### Option A: WithLazyParams Custom Interceptor +**Pros:** Enables `${...}` evaluation +**Cons:** Requires custom interceptor, per-request overhead, more complex + +### Option B: Struts Constants +**Pros:** Simple, no code changes +**Cons:** Not truly dynamic, requires restart to change + +### Option C: Custom Validator +**Pros:** Reusable across actions +**Cons:** More complex setup, still requires configuration loading + +**Conclusion:** Programmatic validation in action provides the best balance of flexibility, simplicity, and maintainability for this use case. diff --git a/thoughts/shared/research/2025-11-21-WW-5585-lazy-multipart-parsing.md b/thoughts/shared/research/2025-11-21-WW-5585-lazy-multipart-parsing.md new file mode 100644 index 000000000..c08da90a1 --- /dev/null +++ b/thoughts/shared/research/2025-11-21-WW-5585-lazy-multipart-parsing.md @@ -0,0 +1,575 @@ +--- +date: 2025-11-21T00:00:00Z +topic: "Lazy Multipart Parsing for Dynamic File Upload Limits" +tags: [research, codebase, file-upload, multipart, lazy-parsing, dynamic-configuration] +status: complete +related_ticket: WW-5585 +--- + +# Research: Lazy Multipart Parsing for Dynamic File Upload Limits + +**Date**: 2025-11-21 + +## Research Question + +The current WW-5585 implementation allows dynamic configuration of file upload validation parameters (allowedTypes, allowedExtensions, maximumSize) at the interceptor level via `WithLazyParams`. However, the `MultiPartRequest` parsing happens **before** the action is instantiated, enforcing global limits (`struts.multipart.maxSize`) as hard caps. + +**Problem**: Even if an action specifies a higher `maximumSize` dynamically, files exceeding the global `struts.multipart.maxSize` are rejected during parsing before the interceptor can apply dynamic limits. + +**Goal**: Investigate implementing "lazy parsing" to defer multipart parsing until after `Preparable.prepare()` runs, allowing dynamic limits to be applied before parsing. + +## Summary + +### Architecture Constraint +The current request lifecycle is: +1. **Request arrives** → `Dispatcher.wrapRequest()` creates `MultiPartRequestWrapper` +2. **Constructor** → `multi.parse(request, saveDir)` with global limits +3. **Action instantiation** → `Preparable.prepare()` sets dynamic config +4. **Interceptor runs** → `ActionFileUploadInterceptor` validates files (too late for size limits) + +### Proposed Solution: Lazy Parsing +Defer `parse()` execution until after `Preparable.prepare()` runs, allowing the interceptor to inject dynamic limits before parsing. + +**Changes Required:** +| Component | Change | +|-----------|--------| +| `AbstractMultiPartRequest` | Add lazy parsing state, limit setters, `triggerParsing()` | +| `MultiPartRequestWrapper` | Defer `parse()` call, add `triggerParsing()` delegation | +| `ActionFileUploadInterceptor` | Trigger parsing after reading dynamic params | +| `StrutsConstants` | Add `struts.multipart.lazyParsing` flag | + +### Complexity Assessment +- **Medium complexity** - Changes to 4 classes +- **Backward compatible** - Flag defaults to `false` +- **Risk**: Breaking existing code that accesses files before interceptor + +## Detailed Findings + +### 1. Current Multipart Request Lifecycle + +**Entry Point**: `Dispatcher.wrapRequest()` + +```java +// Dispatcher.java - creates wrapper with immediate parsing +MultiPartRequestWrapper multiWrapper = new MultiPartRequestWrapper( + multiPartRequest, request, saveDir, provider, + disableRequestAttributeValueStackLookup +); +``` + +**MultiPartRequestWrapper Constructor** (line 72-89): +```java +public MultiPartRequestWrapper(MultiPartRequest multiPartRequest, HttpServletRequest request, + String saveDir, LocaleProvider provider, + boolean disableRequestAttributeValueStackLookup) { + super(request, disableRequestAttributeValueStackLookup); + errors = new ArrayList<>(); + multi = multiPartRequest; + defaultLocale = provider.getLocale(); + setLocale(request); + try { + multi.parse(request, saveDir); // <-- PARSING HAPPENS HERE + for (LocalizedMessage error : multi.getErrors()) { + addError(error); + } + } catch (IOException e) { + LOG.warn(e.getMessage(), e); + addError(buildErrorMessage(e, new Object[] {e.getMessage()})); + } +} +``` + +**Key Insight**: Parsing happens in constructor, before any action/interceptor code runs. + +### 2. Size Limit Enforcement in Commons FileUpload2 + +**AbstractMultiPartRequest.prepareServletFileUpload()** (line 213-229): +```java +protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset, Path saveDir) { + JakartaServletDiskFileUpload servletFileUpload = createJakartaFileUpload(charset, saveDir); + + if (maxSize != null) { + servletFileUpload.setSizeMax(maxSize); // Total request size + } + if (maxFiles != null) { + servletFileUpload.setFileCountMax(maxFiles); // Max file count + } + if (maxFileSize != null) { + servletFileUpload.setFileSizeMax(maxFileSize); // Per-file size + } + return servletFileUpload; +} +``` + +These limits are set on Commons FileUpload2 and enforced during `parseRequest()`. Once set, they cannot be changed retroactively. + +### 3. JakartaMultiPartRequest vs JakartaStreamMultiPartRequest + +**JakartaMultiPartRequest** (Traditional API): +- Uses `servletFileUpload.parseRequest()` - parses entire request at once +- All files loaded into memory or disk immediately +- Size validation during parsing + +**JakartaStreamMultiPartRequest** (Streaming API): +- Uses `servletFileUpload.getItemIterator()` - processes items one at a time +- Files streamed to disk as processed +- Still validates size during streaming + +Both implementations call `prepareServletFileUpload()` which sets the limits from injected constants. + +### 4. Interceptor Parameter Evaluation Timeline + +With `WithLazyParams` implementation: + +| Phase | When | Description | +|-------|------|-------------| +| XML Parsing | Startup | Parameters stored as literal strings | +| Interceptor Init | Startup | Singleton created, `@Inject` setters called | +| Request Arrives | Per-request | `MultiPartRequestWrapper` created with parsing | +| Action Created | Per-request | `Preparable.prepare()` can set dynamic values | +| LazyParams Injection | Per-request | `${...}` expressions evaluated against ValueStack | +| Interceptor.intercept() | Per-request | Validates already-parsed files | + +**Gap**: Parsing happens before action exists, so dynamic limits can't be applied. + +## Implementation Plan + +### Phase 1: AbstractMultiPartRequest Changes + +Add fields for deferred parsing state: +```java +private HttpServletRequest deferredRequest; +private String deferredSaveDir; +private boolean parsed = false; +private boolean lazyParsingEnabled = false; +``` + +Add setter methods for dynamic limits: +```java +public void setMaxSizeLimit(Long maxSize) { + if (maxSize != null) { + this.maxSize = maxSize; + } +} + +public void setMaxFileSizeLimit(Long maxFileSize) { ... } +public void setMaxFilesLimit(Long maxFiles) { ... } +public void setMaxSizeOfFilesLimit(Long maxSizeOfFiles) { ... } +``` + +Add lazy parsing control: +```java +public void setLazyParsingEnabled(boolean enabled) { + this.lazyParsingEnabled = enabled; +} + +public void triggerParsing() throws IOException { + if (!lazyParsingEnabled) { + throw new IllegalStateException("Lazy parsing is not enabled"); + } + if (parsed) { + return; // Already parsed + } + if (deferredRequest == null) { + throw new IllegalStateException("No deferred request to parse"); + } + doParse(deferredRequest, deferredSaveDir); +} +``` + +Modify `parse()` to support deferred mode: +```java +public void parse(HttpServletRequest request, String saveDir) throws IOException { + if (lazyParsingEnabled) { + this.deferredRequest = request; + this.deferredSaveDir = saveDir; + return; // Defer actual parsing + } + doParse(request, saveDir); +} + +private void doParse(HttpServletRequest request, String saveDir) throws IOException { + // Move existing parse() logic here + try { + processUpload(request, saveDir); + } catch (FileUploadException e) { + // ... error handling + } finally { + parsed = true; + deferredRequest = null; + deferredSaveDir = null; + } +} +``` + +### Phase 2: MultiPartRequestWrapper Changes + +Add delegation methods: +```java +public void triggerParsing() throws IOException { + if (multi instanceof AbstractMultiPartRequest abstractMulti) { + abstractMulti.triggerParsing(); + // Re-collect errors after parsing + for (LocalizedMessage error : multi.getErrors()) { + addError(error); + } + } +} + +public void setMaxSizeLimit(Long maxSize) { + if (multi instanceof AbstractMultiPartRequest abstractMulti) { + abstractMulti.setMaxSizeLimit(maxSize); + } +} + +public void setMaxFileSizeLimit(Long maxFileSize) { ... } +public void setMaxFilesLimit(Long maxFiles) { ... } + +public boolean isLazyParsingEnabled() { + if (multi instanceof AbstractMultiPartRequest abstractMulti) { + return abstractMulti.isLazyParsingEnabled(); + } + return false; +} +``` + +Modify constructor to support lazy mode: +```java +public MultiPartRequestWrapper(MultiPartRequest multiPartRequest, HttpServletRequest request, + String saveDir, LocaleProvider provider, + boolean disableRequestAttributeValueStackLookup) { + super(request, disableRequestAttributeValueStackLookup); + errors = new ArrayList<>(); + multi = multiPartRequest; + defaultLocale = provider.getLocale(); + setLocale(request); + + // Check if lazy parsing is enabled + boolean lazyParsing = (multi instanceof AbstractMultiPartRequest abstractMulti) + && abstractMulti.isLazyParsingEnabled(); + + if (!lazyParsing) { + try { + multi.parse(request, saveDir); + collectErrors(); + } catch (IOException e) { + LOG.warn(e.getMessage(), e); + addError(buildErrorMessage(e, new Object[] {e.getMessage()})); + } + } else { + // Store for later parsing + try { + multi.parse(request, saveDir); // This just stores the request + } catch (IOException e) { + // Should not happen in lazy mode + LOG.warn("Unexpected error during lazy parse setup", e); + } + } +} + +private void collectErrors() { + for (LocalizedMessage error : multi.getErrors()) { + addError(error); + } +} +``` + +### Phase 3: ActionFileUploadInterceptor Changes + +Modify `intercept()` to trigger lazy parsing: +```java +@Override +public String intercept(ActionInvocation invocation) throws Exception { + HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); + MultiPartRequestWrapper multiWrapper = findMultipartRequestWrapper(request); + + if (multiWrapper == null) { + // ... existing bypass logic + return invocation.invoke(); + } + + // Trigger lazy parsing with dynamic limits if enabled + if (multiWrapper.isLazyParsingEnabled()) { + // Apply dynamic limits before parsing + applyDynamicLimits(multiWrapper, invocation); + + try { + multiWrapper.triggerParsing(); + } catch (IOException e) { + LOG.warn("Error during lazy multipart parsing", e); + } + } + + if (!(invocation.getAction() instanceof UploadedFilesAware action)) { + // ... existing logic + return invocation.invoke(); + } + + // ... rest of existing validation logic +} + +private void applyDynamicLimits(MultiPartRequestWrapper multiWrapper, ActionInvocation invocation) { + // Apply interceptor's maximumSize to wrapper before parsing + Long maxSize = getMaximumSize(); // From interceptor config (possibly dynamic) + if (maxSize != null) { + multiWrapper.setMaxFileSizeLimit(maxSize); + } + + // Could also read from action if it implements a config interface + Object action = invocation.getAction(); + if (action instanceof FileUploadConfigurable configurable) { + Long actionMaxSize = configurable.getMaxFileSize(); + if (actionMaxSize != null) { + multiWrapper.setMaxFileSizeLimit(actionMaxSize); + } + } +} +``` + +### Phase 4: Configuration Flag + +Add to `StrutsConstants`: +```java +String STRUTS_MULTIPART_LAZY_PARSING = "struts.multipart.lazyParsing"; +``` + +Add to `default.properties`: +```properties +struts.multipart.lazyParsing = false +``` + +Inject in `AbstractMultiPartRequest`: +```java +@Inject(value = StrutsConstants.STRUTS_MULTIPART_LAZY_PARSING, required = false) +public void setLazyParsingEnabled(String enabled) { + this.lazyParsingEnabled = Boolean.parseBoolean(enabled); +} +``` + +### Phase 5: Optional FileUploadConfigurable Interface + +Create interface for actions that provide dynamic upload config: +```java +public interface FileUploadConfigurable { + Long getMaxFileSize(); + Long getMaxRequestSize(); + Long getMaxFiles(); + Set getAllowedTypes(); + Set getAllowedExtensions(); +} +``` + +This allows actions to provide upload limits that are applied before parsing. + +## Code References + +### Files to Modify +- `core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java` - Add lazy parsing support +- `core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java` - Add delegation methods +- `core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java` - Trigger lazy parsing +- `core/src/main/java/org/apache/struts2/StrutsConstants.java` - Add flag constant +- `core/src/main/resources/org/apache/struts2/default.properties` - Add default value + +### Files to Create +- `core/src/main/java/org/apache/struts2/action/FileUploadConfigurable.java` - Optional interface + +### Test Files +- `core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java` - Add lazy parsing tests +- `apps/showcase/src/test/java/it/org/apache/struts2/showcase/DynamicFileUploadTest.java` - Integration tests + +## Architecture Insights + +### Request Flow with Lazy Parsing + +``` +Request Arrives + │ + ▼ +Dispatcher.wrapRequest() + │ + ▼ +MultiPartRequestWrapper(lazyParsing=true) + │ + ├─► Stores request, saveDir + │ Does NOT call processUpload() + │ + ▼ +Action Instantiation + │ + ▼ +Preparable.prepare() + │ + ├─► Action sets dynamic config + │ + ▼ +ActionFileUploadInterceptor.intercept() + │ + ├─► Reads dynamic limits from action/config + ├─► Calls multiWrapper.setMaxFileSizeLimit(...) + ├─► Calls multiWrapper.triggerParsing() + │ │ + │ ▼ + │ processUpload() with dynamic limits + │ + ▼ +File Validation (existing logic) + │ + ▼ +Action.execute() +``` + +### Backward Compatibility + +1. **Default behavior unchanged**: `struts.multipart.lazyParsing=false` by default +2. **Opt-in feature**: Users must explicitly enable lazy parsing +3. **Graceful fallback**: If lazy parsing enabled but not triggered, files still accessible (empty) +4. **No API breaking changes**: All existing code continues to work + +### Security Considerations + +1. **Memory usage**: With lazy parsing, request body is buffered until parsing +2. **Timeout handling**: Long-running prepare() could delay parsing +3. **Error handling**: Parsing errors must be properly propagated to action +4. **Resource cleanup**: Ensure deferred request references are cleared + +## Open Questions + +1. ~~**Buffer management**: How is the request body buffered during deferral? Servlet container may not support re-reading.~~ **RESOLVED**: HTTP input stream cannot be re-read. Lazy parsing as originally designed is not feasible. +2. **Streaming impact**: Does lazy parsing break streaming implementations? +3. **Error timing**: Should parse errors be reported as action errors or interceptor errors? +4. **Multiple interceptors**: What if multiple interceptors try to trigger parsing? + +## Follow-up Research: Request Body Re-readability (2025-11-21) + +### Finding: HTTP Input Stream Cannot Be Re-read + +The HTTP servlet input stream is a **one-time read stream**: +- Once consumed by `parseRequest()` or `getItemIterator()`, it's exhausted +- No built-in buffering or rewinding capability +- This is a fundamental limitation of the Servlet API + +### Solutions Considered + +| Solution | Feasibility | Issue | +|----------|-------------|-------| +| Defer parsing, read later | ❌ Not possible | Stream consumed | +| Cache body with `ContentCachingRequestWrapper` | ⚠️ Risky | Large files → OOM | +| Custom `HttpServletRequestWrapper` with buffering | ⚠️ Risky | Same memory issue | + +### Alternative: Early Action Resolution (Option 2 Revised) + +**Key Discovery**: Action mapping can be resolved from URL path BEFORE parsing! + +In `DefaultActionMapper.getMapping()`: +```java +String uri = RequestUtils.getUri(request); // URL path only +parseNameAndNamespace(uri, mapping, configManager); // No body needed +handleSpecialParameters(request, mapping); // Uses getParameterMap() - won't have multipart params +``` + +The core action/namespace is determined from URL. `handleSpecialParameters()` handles button prefixes which are rarely used with file uploads. + +### Revised Implementation Plan + +**Instead of lazy parsing, use early action config lookup:** + +1. **In `StrutsPrepareFilter.doFilter()` - change order:** + ```java + // BEFORE: wrapRequest() then findActionMapping() + // AFTER: findActionMapping() then wrapRequest() + + ActionMapping mapping = prepare.findActionMapping(request, response, true); + // Look up action config, get interceptor params + Long maxSize = getMaxSizeFromActionConfig(mapping); + // Apply to multipart request before parsing + request = prepare.wrapRequest(request, maxSize); + ``` + +2. **Add method to look up interceptor params from ActionConfig:** + ```java + private Long getMaxSizeFromActionConfig(ActionMapping mapping) { + if (mapping == null) return null; + ActionConfig actionConfig = configuration.getActionConfig( + mapping.getNamespace(), mapping.getName()); + if (actionConfig == null) return null; + + // Find actionFileUpload interceptor params + for (InterceptorMapping im : actionConfig.getInterceptors()) { + if ("actionFileUpload".equals(im.getName())) { + String maxSize = im.getParams().get("maximumSize"); + if (maxSize != null) { + // Handle ${...} expressions by evaluating later + return parseSize(maxSize); + } + } + } + return null; + } + ``` + +3. **Modify `Dispatcher.wrapRequest()` to accept optional maxSize:** + ```java + public HttpServletRequest wrapRequest(HttpServletRequest request, Long maxSize) { + if (maxSize != null) { + multiPartRequest.setMaxSizeLimit(maxSize); + } + // ... existing wrapping logic + } + ``` + +### Limitations of Early Action Resolution + +1. **Dynamic `${...}` expressions won't work** - No ValueStack available yet +2. **`handleSpecialParameters()` won't have multipart form fields** - But rarely needed for file uploads +3. **Action not instantiated** - Can't call `Preparable.prepare()` + +### Recommendation + +For **static interceptor params** (e.g., `10485760`), early action resolution works. + +For **dynamic params** using `${...}` expressions: +- Current `WithLazyParams` implementation handles post-parsing validation +- Pre-parsing dynamic limits require `Preparable.prepare()` which needs action instantiation +- **True dynamic pre-parsing limits may not be achievable** without significant architectural changes + +## Final Conclusion + +**Decision: Keep current approach - no lazy parsing implementation.** + +The current architecture is sufficient: + +| Layer | Limit | When | Purpose | +|-------|-------|------|---------| +| `struts.multipart.maxSize` | Global hard ceiling | During parsing | Prevent oversized requests | +| Interceptor `maximumSize` | Per-action (via `WithLazyParams`) | After parsing | Fine-grained validation | + +**Rationale:** +1. Early action resolution adds complexity users won't understand +2. Difference between "pre-parsing" and "post-parsing" limits is confusing +3. Current `WithLazyParams` provides adequate dynamic control +4. Users simply set global limit high enough, then use interceptor params + +**Recommendation:** Document current behavior clearly - set `struts.multipart.maxSize` as ceiling, use dynamic interceptor params for per-action limits + +## Risk Assessment + +| Risk | Impact | Mitigation | +|------|--------|------------| +| Request body not re-readable | High | May need to cache input stream | +| Breaking existing behavior | Medium | Default to disabled | +| Performance overhead | Low | Only affects lazy-enabled requests | +| Complexity increase | Medium | Clear documentation, tests | + +## Next Steps + +1. **Prototype Phase 1**: Implement AbstractMultiPartRequest changes +2. **Test re-readability**: Verify request body can be read after deferral +3. **Implement Phase 2-4**: Complete wrapper and interceptor changes +4. **Add comprehensive tests**: Unit and integration tests +5. **Documentation**: Update file upload documentation +6. **Performance testing**: Measure overhead of lazy parsing + +## Related Research + +- `thoughts/shared/research/2025-10-22-dynamic-file-upload-validation.md` - WithLazyParams implementation +- WW-5585 ticket - Dynamic file upload parameters feature