This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch WW-5474-multipart-maxfiles-semantics in repository https://gitbox.apache.org/repos/asf/struts.git
commit 3b6a5437afd246aa9218e9bd0dd8b25619d1551d Author: Lukasz Lenart <[email protected]> AuthorDate: Wed Jul 22 15:52:30 2026 +0200 WW-5474 fix(multipart): apply files-only maxFiles + maxParameterCount to stream parser Replace the field-name-based exceedsMaxFiles with the shared files-only enforcement and add parameter-count enforcement, matching the jakarta parser and failing closed on breach. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../multipart/JakartaStreamMultiPartRequest.java | 34 ++++-------- .../JakartaStreamMultiPartRequestTest.java | 63 +++++++++++++++++++--- 2 files changed, 66 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java index a318f0511..62ab294d8 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java @@ -20,7 +20,6 @@ package org.apache.struts2.dispatcher.multipart; 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 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { return; } + enforceMaxParameterCount(parameterCount, fieldName); + parameterCount++; + String fieldValue = readStream(fileItemInput.getInputStream()); if (exceedsMaxStringLength(fieldName, fieldValue)) { return; @@ -148,26 +155,6 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { .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 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { return; } - if (exceedsMaxFiles(fileItemInput)) { - return; - } + enforceMaxFiles(fileCount, fileItemInput.getName()); + fileCount++; File file = createTemporaryFile(fileItemInput.getName(), location); streamFileToDisk(fileItemInput, file); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java index fc78021f5..988be53eb 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java @@ -216,14 +216,63 @@ public class JakartaStreamMultiPartRequestTest extends AbstractMultiPartRequestT // when - set max files to 1 multiPart.setMaxFiles("1"); multiPart.parse(mockRequest, tempDir); - - // then - should have only 1 file and errors for others - assertThat(multiPart.uploadedFiles).hasSize(1); + + // then - fail-closed: no partial files, one error + assertThat(multiPart.uploadedFiles).isEmpty(); assertThat(multiPart.getErrors()) - .isNotEmpty() - .allSatisfy(error -> - assertThat(error.getTextKey()).isEqualTo("struts.messages.upload.error.FileUploadFileCountLimitException") - ); + .map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.FileUploadFileCountLimitException"); + } + + @Test + public void streamManyFormFieldsWithFewFilesAreAccepted() throws IOException { + StringBuilder content = new StringBuilder(); + for (int i = 0; i < 10; i++) { + content.append(formField("field" + i, "value" + i)); + } + content.append(formFile("file1", "test1.csv", "1,2,3,4")); + content.append(endline).append("--").append(boundary).append("--"); + mockRequest.setContent(content.toString().getBytes(StandardCharsets.UTF_8)); + + multiPart.setMaxFiles("1"); + multiPart.parse(mockRequest, tempDir); + + assertThat(multiPart.getErrors()).isEmpty(); + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asInstanceOf(InstanceOfAssertFactories.LIST).containsOnly("file1"); + } + + @Test + public void streamMultipleFilesUnderOneFieldNameAreCounted() throws IOException { + String content = formFile("file", "a.csv", "1") + + formFile("file", "b.csv", "2") + + formFile("file", "c.csv", "3") + + endline + "--" + boundary + "--"; + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + multiPart.setMaxFiles("2"); + multiPart.parse(mockRequest, tempDir); + + // Field-name counting bug would keep all 3 under one key; files-only counting rejects. + assertThat(multiPart.uploadedFiles).isEmpty(); + assertThat(multiPart.getErrors()).map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.FileUploadFileCountLimitException"); + } + + @Test + public void streamExceedsMaxParameterCountIsFailClosed() throws IOException { + String content = formField("field1", "a") + + formField("field2", "b") + + formField("field3", "c") + + endline + "--" + boundary + "--"; + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + multiPart.setMaxParameterCount("2"); + multiPart.parse(mockRequest, tempDir); + + assertThat(multiPart.getErrors()).map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.FileUploadParameterCountLimitException"); + assertThat(multiPart.getParameterNames().asIterator()).toIterable().isEmpty(); } @Test
