This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5366-empty-file
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 63a945852afc52e3a63f263d9be2fcafaca4d619
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Thu Aug 7 18:40:44 2025 +0200
WW-5366 Rejects empty files during upload
---
.claude/settings.local.json | 11 --
.gitignore | 3 +
.../multipart/AbstractMultiPartRequest.java | 37 +++++++
.../multipart/JakartaMultiPartRequest.java | 5 +
.../multipart/JakartaStreamMultiPartRequest.java | 9 ++
.../org/apache/struts2/struts-messages.properties | 5 +
.../multipart/AbstractMultiPartRequestTest.java | 113 +++++++++++++++++++++
7 files changed, 172 insertions(+), 11 deletions(-)
diff --git a/.claude/settings.local.json b/.claude/settings.local.json
deleted file mode 100644
index 052e28d88..000000000
--- a/.claude/settings.local.json
+++ /dev/null
@@ -1,11 +0,0 @@
-{
- "permissions": {
- "allow": [
- "Bash(find:*)",
- "Bash(mvn:*)",
- "Bash(grep:*)",
- "Bash(cat:*)"
- ],
- "deny": []
- }
-}
\ No newline at end of file
diff --git a/.gitignore b/.gitignore
index af4e6813c..f88925cca 100644
--- a/.gitignore
+++ b/.gitignore
@@ -46,3 +46,6 @@ test-output
# Tidelift CLI scanner
.tidelift
+
+# Claude Code local settings
+.claude/
\ No newline at end of file
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
index 63afbdf22..22191e40b 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
@@ -424,6 +424,43 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
return file;
}
+ /**
+ * Validates that an uploaded file is not empty (0 bytes) and adds an
error if it is.
+ *
+ * <p>Empty file uploads are rejected as they are not considered valid
uploads.
+ * This validation ensures consistent behavior across all multipart
implementations
+ * and provides proper user feedback when empty files are uploaded.</p>
+ *
+ * <p>When an empty file is detected:</p>
+ * <ul>
+ * <li>A debug log message is written with field name and filename</li>
+ * <li>A localized error message is created and added to the errors
list</li>
+ * <li>The method returns true to indicate the file should be
rejected</li>
+ * </ul>
+ *
+ * @param fileSize the size of the uploaded file in bytes
+ * @param fileName the original filename of the uploaded file
+ * @param fieldName the form field name containing the file upload
+ * @return true if the file is empty and should be rejected, false
otherwise
+ * @see #buildErrorMessage(Class, String, Object[])
+ */
+ protected boolean rejectEmptyFile(long fileSize, String fileName, String
fieldName) {
+ if (fileSize == 0) {
+ LOG.debug("Rejecting empty file upload for field: {} with
filename: {}",
+ normalizeSpace(fieldName), normalizeSpace(fileName));
+ LocalizedMessage errorMessage = buildErrorMessage(
+ IllegalArgumentException.class,
+ "Empty files are not allowed",
+ new Object[]{fileName, fieldName}
+ );
+ if (!errors.contains(errorMessage)) {
+ errors.add(errorMessage);
+ }
+ return true;
+ }
+ return false;
+ }
+
/* (non-Javadoc)
* @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
*/
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
index 0b4ce88fe..924b5ed21 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
@@ -206,6 +206,11 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
return;
}
+ // Reject empty files (0 bytes) as they are not considered valid
uploads
+ if (rejectEmptyFile(item.getSize(), item.getName(), fieldName)) {
+ return;
+ }
+
List<UploadedFile> values = uploadedFiles.computeIfAbsent(fieldName, k
-> new ArrayList<>());
if (item.isInMemory()) {
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 2942b7b03..8b4fe2830 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
@@ -232,6 +232,15 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
File file = createTemporaryFile(fileItemInput.getName(), location);
streamFileToDisk(fileItemInput, file);
+
+ // Reject empty files (0 bytes) as they are not considered valid
uploads
+ if (rejectEmptyFile(file.length(), fileItemInput.getName(),
fileItemInput.getFieldName())) {
+ // Clean up the empty temporary file
+ if (file.exists() && !file.delete()) {
+ LOG.warn("Failed to delete empty temporary file: {}",
file.getAbsolutePath());
+ }
+ return;
+ }
Long currentFilesSize = maxSizeOfFiles != null ?
actualSizeOfUploadedFiles() : null;
if (maxSizeOfFiles != null && currentFilesSize + file.length() >=
maxSizeOfFiles) {
diff --git
a/core/src/main/resources/org/apache/struts2/struts-messages.properties
b/core/src/main/resources/org/apache/struts2/struts-messages.properties
index 7c418f75c..b36124bbf 100644
--- a/core/src/main/resources/org/apache/struts2/struts-messages.properties
+++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties
@@ -75,6 +75,11 @@
struts.messages.upload.error.FileUploadContentTypeException=Request has wrong co
# Default error message when handling multi-part request
struts.messages.upload.error.FileUploadException=Error parsing the multi-part
request.
+# IllegalArgumentException for empty files
+# 0 - original filename
+# 1 - field name
+struts.messages.upload.error.IllegalArgumentException=File {0} for field {1}
is empty (0 bytes). Empty file uploads are not allowed.
+
devmode.notification=Developer Notification (set struts.devMode to false to
disable this message):\n{0}
struts.exception.missing-package-action.with-context = There is no Action
mapped for namespace [{0}] and action name [{1}] associated with context path
[{2}].
diff --git
a/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java
b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java
index 98d1325ef..6a2450201 100644
---
a/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java
+++
b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java
@@ -743,6 +743,119 @@ abstract class AbstractMultiPartRequestTest {
tempFiles.forEach(File::delete);
}
+ @Test
+ public void emptyFileUploadsAreRejected() throws IOException {
+ // Test that empty files (0 bytes) are rejected with proper error
message
+ String content =
+ endline + "--" + boundary + endline +
+ "Content-Disposition: form-data; name=\"emptyfile\";
filename=\"empty.txt\"" + endline +
+ "Content-Type: text/plain" + endline +
+ endline +
+ // No content - this creates a 0-byte file
+ endline + "--" + boundary + "--";
+
+ mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
+
+ // when
+ multiPart.parse(mockRequest, tempDir);
+
+ // then - should reject empty file and add error
+ assertThat(multiPart.getErrors())
+ .hasSize(1)
+ .first()
+ .satisfies(error -> {
+
assertThat(error.getTextKey()).isEqualTo("struts.messages.upload.error.IllegalArgumentException");
+ assertThat(error.getArgs()).containsExactly("empty.txt",
"emptyfile");
+ });
+ assertThat(multiPart.uploadedFiles).isEmpty();
+ assertThat(multiPart.getFile("emptyfile")).isEmpty();
+ }
+
+ @Test
+ public void mixedEmptyAndValidFilesProcessedCorrectly() throws IOException
{
+ // Test that valid files are processed while empty files are rejected
+ String content =
+ endline + "--" + boundary + endline +
+ "Content-Disposition: form-data; name=\"emptyfile1\";
filename=\"empty1.txt\"" + endline +
+ "Content-Type: text/plain" + endline +
+ endline +
+ // No content - empty file
+ endline + "--" + boundary + endline +
+ "Content-Disposition: form-data; name=\"validfile\";
filename=\"valid.txt\"" + endline +
+ "Content-Type: text/plain" + endline +
+ endline +
+ "some valid content" +
+ endline + "--" + boundary + endline +
+ "Content-Disposition: form-data; name=\"emptyfile2\";
filename=\"empty2.txt\"" + endline +
+ "Content-Type: application/octet-stream" + endline +
+ endline +
+ // Another empty file
+ endline + "--" + boundary + "--";
+
+ mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
+
+ // when
+ multiPart.parse(mockRequest, tempDir);
+
+ // then - should have 2 errors for empty files, 1 valid file processed
+ assertThat(multiPart.getErrors()).hasSize(2);
+ assertThat(multiPart.getErrors().get(0))
+ .satisfies(error -> {
+
assertThat(error.getTextKey()).isEqualTo("struts.messages.upload.error.IllegalArgumentException");
+ assertThat(error.getArgs()).containsExactly("empty1.txt",
"emptyfile1");
+ });
+ assertThat(multiPart.getErrors().get(1))
+ .satisfies(error -> {
+
assertThat(error.getTextKey()).isEqualTo("struts.messages.upload.error.IllegalArgumentException");
+ assertThat(error.getArgs()).containsExactly("empty2.txt",
"emptyfile2");
+ });
+
+ // Only the valid file should be processed
+ assertThat(multiPart.uploadedFiles).hasSize(1);
+ assertThat(multiPart.getFile("validfile")).hasSize(1);
+ assertThat(multiPart.getFile("emptyfile1")).isEmpty();
+ assertThat(multiPart.getFile("emptyfile2")).isEmpty();
+
+ // Verify valid file content
+ assertThat(multiPart.getFile("validfile")[0].getContent())
+ .asInstanceOf(InstanceOfAssertFactories.FILE)
+ .content()
+ .isEqualTo("some valid content");
+ }
+
+ @Test
+ public void emptyFileTemporaryFileCleanup() throws IOException {
+ // Test that temporary files for empty files are properly cleaned up
+ String content =
+ endline + "--" + boundary + endline +
+ "Content-Disposition: form-data; name=\"emptyfile\";
filename=\"empty.txt\"" + endline +
+ "Content-Type: text/plain" + endline +
+ endline +
+ // Empty file
+ endline + "--" + boundary + "--";
+
+ mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
+
+ // Count temp files before processing
+ File[] tempFilesBefore = new File(tempDir).listFiles((dir, name) ->
name.startsWith("upload_") && name.endsWith(".tmp"));
+ int countBefore = tempFilesBefore != null ? tempFilesBefore.length : 0;
+
+ // when
+ multiPart.parse(mockRequest, tempDir);
+
+ // then - should reject empty file and clean up temp file
+ assertThat(multiPart.getErrors()).hasSize(1);
+ assertThat(multiPart.uploadedFiles).isEmpty();
+
+ // Verify that temporary files are cleaned up (may have implementation
differences)
+ // Some implementations create temp files first, others don't create
any for empty uploads
+ File[] tempFilesAfter = new File(tempDir).listFiles((dir, name) ->
name.startsWith("upload_") && name.endsWith(".tmp"));
+ int countAfter = tempFilesAfter != null ? tempFilesAfter.length : 0;
+
+ // Allow for implementation differences - just ensure no new temp
files remain
+ assertThat(countAfter).isLessThanOrEqualTo(countBefore);
+ }
+
protected String formFile(String fieldName, String filename, String
content) {
return endline +
"--" + boundary + endline +
