diff --git a/.claude/settings.local.json b/.claude/settings.local.json deleted file mode 100644 index 052e28d880..0000000000 --- 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 af4e6813cc..f88925ccaf 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 63afbdf221..22191e40b1 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 @@ 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. + * + *

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.

+ * + *

When an empty file is detected:

+ * + * + * @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 0b4ce88fe7..924b5ed218 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 @@ 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 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 2942b7b03d..8b4fe28308 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 @@ 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) { 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 7c418f75c0..b36124bbfa 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 98d1325ef5..6a2450201a 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 @@ 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 +