WW-5474 Count files only for maxFiles, add maxParameterCount

core/src/main/java/org/apache/struts2/StrutsConstants.java

@@ -226,6 +226,11 @@ public final class StrutsConstants {
      */
     public static final String STRUTS_MULTIPART_MAX_FILES = "struts.multipart.maxFiles";
 
+    /**
+     * The maximum number of non-file form fields (parameters) allowed in a multipart request.
+     */
+    public static final String STRUTS_MULTIPART_MAX_PARAMETER_COUNT = "struts.multipart.maxParameterCount";
+
     /**
      * The maximum length of a string parameter in a multipart request.
      */

core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java

@@ -93,6 +93,11 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
      */
     protected Long maxFiles;
 
+    /**
+     * Specifies the maximum number of non-file form fields (parameters) in one request.
+     */
+    protected Long maxParameterCount;
+
     /**
      * Specifies the maximum length of a string parameter in a multipart request.
      */
@@ -160,6 +165,14 @@ public void setMaxFiles(String maxFiles) {
         this.maxFiles = Long.parseLong(maxFiles);
     }
 
+    /**
+     * @param maxParameterCount Injects the Struts maximum number of non-file form fields.
+     */
+    @Inject(StrutsConstants.STRUTS_MULTIPART_MAX_PARAMETER_COUNT)
+    public void setMaxParameterCount(String maxParameterCount) {
+        this.maxParameterCount = Long.parseLong(maxParameterCount);
+    }
+
     /**
      * @param maxFileSize Injects the Struts maximum number of files, which can be uploaded.
      */
@@ -226,9 +239,17 @@ protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset,
             LOG.debug("Applies max size: {} to file upload request", maxSize);
             servletFileUpload.setMaxSize(maxSize);
         }
-        if (maxFiles != null) {
-            LOG.debug("Applies max files number: {} to file upload request", maxFiles);
-            servletFileUpload.setMaxFileCount(maxFiles);
+        if (maxFiles != null && maxFiles >= 0 && maxParameterCount != null && maxParameterCount >= 0) {
+            // Clamp on overflow: a wrapped-negative sum would silently disable the backstop
+            // (commons-fileupload2 treats a negative count as "unlimited").
+            long maxParts;
+            try {
+                maxParts = Math.addExact(maxFiles, maxParameterCount);
+            } catch (ArithmeticException overflow) {
+                maxParts = Long.MAX_VALUE;
+            }
+            LOG.debug("Applies total parts backstop: {} to file upload request", maxParts);
+            servletFileUpload.setMaxFileCount(maxParts);
         }
         if (maxFileSize != null) {
             LOG.debug("Applies max size of single file: {} to file upload request", maxFileSize);
@@ -298,6 +319,44 @@ protected boolean exceedsMaxStringLength(String fieldName, String fieldValue) {
         return false;
     }
 
+    /**
+     * Fail-closed guard: throws when accepting another file would exceed {@link #maxFiles}.
+     * A negative {@link #maxFiles} means "no limit", matching commons-fileupload2's own
+     * {@code fileCountMax = -1} convention (see {@code AbstractFileUpload.setFileCountMax}).
+     *
+     * @param currentFileCount number of files already accepted in this request
+     * @param fileName         name of the file being considered (for logging)
+     */
+    protected void enforceMaxFiles(int currentFileCount, String fileName) throws FileUploadFileCountLimitException {
+        if (maxFiles != null && maxFiles >= 0 && currentFileCount >= maxFiles) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Cannot accept another file: {} as it would exceed max files: {}", normalizeSpace(fileName), maxFiles);
+            }
+            throw new FileUploadFileCountLimitException(
+                    String.format("Request exceeds allowed number of files, permitted: %s", maxFiles),
+                    maxFiles, currentFileCount + 1L);
+        }
+    }
+
+    /**
+     * Fail-closed guard: throws when accepting another form field would exceed {@link #maxParameterCount}.
+     * A negative {@link #maxParameterCount} means "no limit", matching the same convention as
+     * {@link #maxFiles}.
+     *
+     * @param currentParameterCount number of form fields already accepted in this request
+     * @param fieldName             name of the field being considered (for logging)
+     */
+    protected void enforceMaxParameterCount(int currentParameterCount, String fieldName) throws FileUploadParameterCountLimitException {
+        if (maxParameterCount != null && maxParameterCount >= 0 && currentParameterCount >= maxParameterCount) {
+            if (LOG.isDebugEnabled()) {
+                LOG.debug("Cannot accept another parameter: {} as it would exceed max parameter count: {}", normalizeSpace(fieldName), maxParameterCount);
+            }
+            throw new FileUploadParameterCountLimitException(
+                    String.format("Request exceeds allowed number of parameters, permitted: %s", maxParameterCount),
+                    maxParameterCount, currentParameterCount + 1L);
+        }
+    }
+
     /**
      * Processes the upload.
      *
@@ -324,10 +383,14 @@ public void parse(HttpServletRequest request, String saveDir) throws IOException
             } else if (e instanceof FileUploadContentTypeException ex) {
                 exClass = ex.getClass();
                 args = new Object[]{ex.getContentType()};
+            } else if (e instanceof FileUploadParameterCountLimitException ex) {
+                exClass = ex.getClass();
+                args = new Object[]{ex.getPermitted(), ex.getActual()};
             }
 
             LocalizedMessage errorMessage = buildErrorMessage(exClass, e.getMessage(), args);
             addErrorIfAbsent(errorMessage);
+            clearCollectedData();
         } catch (IOException e) {
             LOG.warn("Unable to parse request", e);
             LocalizedMessage errorMessage = buildErrorMessage(e.getClass(), e.getMessage(), new Object[]{});
@@ -341,6 +404,23 @@ private void addErrorIfAbsent(LocalizedMessage errorMessage) {
         }
     }
 
+    /**
+     * Fail-closed: discards everything collected so far so a rejected request exposes
+     * no partial parameters or files to the action. Deletes partial upload files first
+     * to avoid leaking temporary files.
+     */
+    private void clearCollectedData() {
+        for (List<UploadedFile> files : uploadedFiles.values()) {
+            for (UploadedFile file : files) {
+                if (file.isFile() && !file.delete()) {
+                    LOG.warn("Could not delete partial upload file: {}", file.getName());
+                }
+            }
+        }
+        uploadedFiles.clear();
+        parameters.clear();
+    }
+
     /**
      * Build error message.
      *

core/src/main/java/org/apache/struts2/dispatcher/multipart/FileUploadParameterCountLimitException.java

@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.struts2.dispatcher.multipart;
+
+import org.apache.commons.fileupload2.core.FileUploadException;
+
+/**
+ * Thrown when a multipart request contains more non-file form fields (parameters)
+ * than allowed by {@code struts.multipart.maxParameterCount}.
+ */
+public class FileUploadParameterCountLimitException extends FileUploadException {
+
+    private final long permitted;
+    private final long actual;
+
+    public FileUploadParameterCountLimitException(final String message, final long permitted, final long actual) {
+        super(message);
+        this.permitted = permitted;
+        this.actual = actual;
+    }
+
+    public long getPermitted() {
+        return permitted;
+    }
+
+    public long getActual() {
+        return actual;
+    }
+}

core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java

@@ -115,18 +115,31 @@ protected void processUpload(HttpServletRequest request, String saveDir) throws
 
         RequestContext requestContext = createRequestContext(request);
 
-        for (DiskFileItem item : servletFileUpload.parseRequest(requestContext)) {
-            // Track all DiskFileItem instances for cleanup - this is critical for security
-            // as it ensures temporary files are properly cleaned up even if processing fails
-            diskFileItems.add(item);
-
-            LOG.debug(() -> "Processing a form field: " + normalizeSpace(item.getFieldName()));
+        int fileCount = 0;
+        int parameterCount = 0;
+        // parseRequest() fully materializes every part (spilling large ones to disk) before we
+        // iterate, so register them all for cleanup up front - otherwise a fail-closed breach
+        // mid-loop would leak the temp files of every part after the breaching one.
+        List<DiskFileItem> items = servletFileUpload.parseRequest(requestContext);
+        diskFileItems.addAll(items);
+        for (DiskFileItem item : items) {
             if (item.isFormField()) {
+                LOG.debug(() -> "Processing a form field: " + normalizeSpace(item.getFieldName()));
                 // Process regular form fields (text inputs, checkboxes, etc.)
+                if (item.getFieldName() != null) {
+                    enforceMaxParameterCount(parameterCount, item.getFieldName());
+                    parameterCount++;
+                }
                 processNormalFormField(item, charset);
             } else {
-                // Process file upload fields
+                // Process file upload fields (only count parts that would actually be accepted:
+                // a real filename AND a non-null field name, matching JakartaStreamMultiPartRequest
+                // so both parsers enforce maxFiles identically on malformed parts).
                 LOG.debug(() -> "Processing a file: " + normalizeSpace(item.getFieldName()));
+                if (item.getName() != null && !item.getName().trim().isEmpty() && item.getFieldName() != null) {
+                    enforceMaxFiles(fileCount, item.getName());
+                    fileCount++;
+                }
                 processFileField(item, saveDir);
             }
         }

core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java

@@ -20,7 +20,6 @@
 
 import jakarta.servlet.http.HttpServletRequest;
 import org.apache.commons.fileupload2.core.FileItemInput;
-import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException;
 import org.apache.commons.fileupload2.core.FileUploadSizeException;
 import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload;
 import org.apache.logging.log4j.LogManager;
@@ -54,6 +53,9 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
 
     private static final Logger LOG = LogManager.getLogger(JakartaStreamMultiPartRequest.class);
 
+    private int fileCount;
+    private int parameterCount;
+
     /**
      * Processes the upload.
      *
@@ -64,6 +66,8 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
     protected void processUpload(HttpServletRequest request, String saveDir) throws IOException {
         Charset charset = readCharsetEncoding(request);
         Path location = Path.of(saveDir);
+        fileCount = 0;
+        parameterCount = 0;
 
         JakartaServletDiskFileUpload servletFileUpload =
                 prepareServletFileUpload(charset, location);
@@ -130,6 +134,9 @@ protected void processFileItemAsFormField(FileItemInput fileItemInput) throws IO
             return;
         }
 
+        enforceMaxParameterCount(parameterCount, fieldName);
+        parameterCount++;
+
         String fieldValue = readStream(fileItemInput.getInputStream());
         if (exceedsMaxStringLength(fieldName, fieldValue)) {
             return;
@@ -148,26 +155,6 @@ protected Long actualSizeOfUploadedFiles() {
                 .reduce(0L, Long::sum);
     }
 
-    private boolean exceedsMaxFiles(FileItemInput fileItemInput) {
-        if (maxFiles != null && maxFiles == uploadedFiles.size()) {
-            if (LOG.isDebugEnabled()) {
-                LOG.debug("Cannot accept another file: {} as it will exceed max files: {}",
-                        normalizeSpace(fileItemInput.getName()), maxFiles);
-            }
-            LocalizedMessage errorMessage = buildErrorMessage(
-                    FileUploadFileCountLimitException.class,
-                    String.format("File %s exceeds allowed maximum number of files %s",
-                            fileItemInput.getName(), maxFiles),
-                    new Object[]{maxFiles, uploadedFiles.size()}
-            );
-            if (!errors.contains(errorMessage)) {
-                errors.add(errorMessage);
-            }
-            return true;
-        }
-        return false;
-    }
-
     private void exceedsMaxSizeOfFiles(FileItemInput fileItemInput, File file, Long currentFilesSize) {
         if (LOG.isDebugEnabled()) {
             LOG.debug("File: {} of size: {} exceeds allowed max size: {}, actual size of already uploaded files: {}",
@@ -226,9 +213,8 @@ protected void processFileItemAsFileField(FileItemInput fileItemInput, Path loca
             return;
         }
 
-        if (exceedsMaxFiles(fileItemInput)) {
-            return;
-        }
+        enforceMaxFiles(fileCount, fileItemInput.getName());
+        fileCount++;
 
         File file = createTemporaryFile(fileItemInput.getName(), location);
         streamFileToDisk(fileItemInput, file);

core/src/main/resources/org/apache/struts2/default.properties

@@ -67,7 +67,10 @@ struts.multipart.parser=jakarta
 ### Uses jakarta.servlet.context.tempdir by default
 struts.multipart.saveDir=
 struts.multipart.maxSize=2097152
+# Maximum number of uploaded files (files only, not form fields)
 struts.multipart.maxFiles=256
+# Maximum number of non-file form fields (parameters)
+struts.multipart.maxParameterCount=256
 struts.multipart.maxStringLength=4096
 # struts.multipart.maxFileSize=
 

core/src/main/resources/org/apache/struts2/struts-messages.properties

@@ -63,6 +63,10 @@ struts.messages.upload.error.FileUploadByteCountLimitException=File {1} assigned
 # 0 - limit
 struts.messages.upload.error.FileUploadFileCountLimitException=Request exceeded allowed number of files! Permitted number of files is: {0}!
 
+# FileUploadParameterCountLimitException
+# 0 - limit
+struts.messages.upload.error.FileUploadParameterCountLimitException=Request exceeded allowed number of parameters! Permitted number of parameters is: {0}!
+
 # FileUploadSizeException
 # 1 - permitted size
 # 2 - actual size

core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java

@@ -389,6 +389,12 @@ public void maxFiles() throws IOException {
                 .containsExactly("struts.messages.upload.error.FileUploadFileCountLimitException");
     }
 
+    @Test
+    public void maxParameterCountSetterStoresValue() {
+        multiPart.setMaxParameterCount("42");
+        assertThat(multiPart.maxParameterCount).isEqualTo(42L);
+    }
+
     @Test
     public void maxStringLength() throws IOException {
         String content = formFile("file1", "test1.csv", "1,2,3,4") +