WW-5366 Rejects empty files during upload

Author: lukaszlenart

.claude/settings.local.json

@@ -46,3 +46,6 @@ test-output
 
 # Tidelift CLI scanner
 .tidelift
+
+# Claude Code local settings
+.claude/

.gitignore

@@ -424,6 +424,43 @@ protected File createTemporaryFile(String fileName, Path location) {
         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()
      */

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

@@ -206,6 +206,11 @@ protected void processFileField(DiskFileItem item, String saveDir) {
             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()) {

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

@@ -232,6 +232,15 @@ protected void processFileItemAsFileField(FileItemInput fileItemInput, Path loca
 
         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) {

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

@@ -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}].

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

@@ -743,6 +743,119 @@ public void createTemporaryFileConsistentNaming() {
         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 +