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 56f9e0a49..a8880d47a 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 @@ -38,6 +38,37 @@ import static org.apache.commons.lang3.StringUtils.normalizeSpace; /** * Multipart form data request adapter for Jakarta Commons FileUpload package. + * + *

This implementation provides secure handling of multipart requests with proper + * resource management and cleanup. It tracks all temporary files created during + * the upload process and ensures they are properly cleaned up to prevent + * resource leaks and security vulnerabilities.

+ * + *

Key features:

+ * + * + *

Usage example:

+ * 
+ * JakartaMultiPartRequest multipartRequest = new JakartaMultiPartRequest();
+ * try {
+ *     multipartRequest.parse(request, "/tmp/uploads");
+ *     // Process uploaded files
+ *     for (String fieldName : multipartRequest.getFileParameterNames()) {
+ *         List<UploadedFile> files = multipartRequest.getFile(fieldName);
+ *         // Handle files
+ *     }
+ * } finally {
+ *     multipartRequest.cleanUp(); // Always clean up resources
+ * }
+ *
+ * + * @see AbstractMultiPartRequest + * @see org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload */ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { @@ -53,6 +84,27 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { */ private final List temporaryFiles = new ArrayList<>(); + /** + * Processes the multipart upload request using Jakarta Commons FileUpload. + * + *

This method handles the core upload processing by:

+ *
    + *
  1. Reading the character encoding from the request
    2. + *
  2. Preparing the Jakarta servlet file upload handler
    3. + *
  3. Creating a request context for processing
    4. + *
  4. Iterating through all form items (fields and files)
    5. + *
  5. Processing each item appropriately based on its type
    6. + *
+ * + *

All {@link org.apache.commons.fileupload2.core.DiskFileItem} instances + * are automatically tracked for proper cleanup.

+ * + * @param request the HTTP servlet request containing the multipart data + * @param saveDir the directory where uploaded files will be stored + * @throws IOException if an error occurs during upload processing + * @see #processNormalFormField(DiskFileItem, Charset) + * @see #processFileField(DiskFileItem) + */ @Override protected void processUpload(HttpServletRequest request, String saveDir) throws IOException { Charset charset = readCharsetEncoding(request); @@ -63,29 +115,51 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { RequestContext requestContext = createRequestContext(request); for (DiskFileItem item : servletFileUpload.parseRequest(requestContext)) { - // Track all DiskFileItem instances for cleanup + // 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())); if (item.isFormField()) { + // Process regular form fields (text inputs, checkboxes, etc.) processNormalFormField(item, charset); } else { + // Process file upload fields LOG.debug(() -> "Processing a file: " + normalizeSpace(item.getFieldName())); processFileField(item); } } } + /** + * Processes a normal form field (non-file) from the multipart request. + * + *

This method handles text form fields by:

+ *
    + *
  1. Validating the field name is not null
    2. + *
  2. Extracting the field value using the specified charset
    3. + *
  3. Checking if the field value exceeds maximum string length
    4. + *
  4. Adding the value to the parameters map
    5. + *
+ * + *

Fields with null names are skipped with a warning log message.

+ *

Empty form fields are stored as empty strings.

+ * + * @param item the disk file item representing the form field + * @param charset the character set to use for decoding the field value + * @throws IOException if an error occurs reading the field value + * @see #exceedsMaxStringLength(String, String) + */ protected void processNormalFormField(DiskFileItem item, Charset charset) throws IOException { LOG.debug("Item: {} is a normal form field", normalizeSpace(item.getName())); - List values; String fieldName = item.getFieldName(); - if (parameters.get(fieldName) != null) { - values = parameters.get(fieldName); - } else { - values = new ArrayList<>(); + if (fieldName == null) { + LOG.warn("Form field has null fieldName, skipping"); + return; } + + List values = parameters.computeIfAbsent(fieldName, k -> new ArrayList<>()); String fieldValue = item.getString(charset); if (exceedsMaxStringLength(fieldName, fieldValue)) { @@ -99,6 +173,26 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { parameters.put(fieldName, values); } + /** + * Processes a file field from the multipart request. + * + *

This method handles file uploads by:

+ *
    + *
  1. Validating the file name and field name are not null/empty
    2. + *
  2. Determining if the file is stored in memory or on disk
    3. + *
  3. For in-memory files: creating a temporary file and copying content
    4. + *
  4. For disk files: using the existing file directly
    5. + *
  5. Creating an {@link UploadedFile} abstraction
    6. + *
  6. Adding the file to the uploaded files collection
    7. + *
+ * + *

Temporary files created for in-memory uploads are automatically + * tracked for cleanup. Any errors during temporary file creation are + * logged and added to the error list for user feedback.

+ * + * @param item the disk file item representing the uploaded file + * @see #cleanUpTemporaryFiles() + */ protected void processFileField(DiskFileItem item) { // Skip file uploads that don't have a file name - meaning that no file was selected. if (item.getName() == null || item.getName().trim().isEmpty()) { @@ -106,18 +200,18 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { return; } - List values; - if (uploadedFiles.get(item.getFieldName()) != null) { - values = uploadedFiles.get(item.getFieldName()); - } else { - values = new ArrayList<>(); + String fieldName = item.getFieldName(); + if (fieldName == null) { + LOG.warn("File field has null fieldName, skipping"); + return; } + + List values = uploadedFiles.computeIfAbsent(fieldName, k -> new ArrayList<>()); if (item.isInMemory()) { LOG.debug("Creating temporary file representing in-memory uploaded item: {}", normalizeSpace(item.getFieldName())); try { File tempFile = File.createTempFile("struts_upload_", "_" + item.getName()); - tempFile.deleteOnExit(); // Ensure cleanup on JVM exit as fallback // Track the temporary file for explicit cleanup temporaryFiles.add(tempFile); @@ -157,12 +251,26 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { values.add(uploadedFile); } - uploadedFiles.put(item.getFieldName(), values); + uploadedFiles.put(fieldName, values); } /** * Cleans up disk file items by deleting associated temporary files. - * This method can be overridden by subclasses to customize cleanup behavior. + * + *

This method iterates through all tracked {@link DiskFileItem} instances + * and performs cleanup operations:

+ *
    + *
  • For in-memory items: logs cleanup (no files to delete)
    • + *
  • For disk items: deletes the associated temporary file
    • + *
+ * + *

This method is called automatically during {@link #cleanUp()} but can + * be overridden by subclasses to customize cleanup behavior. All exceptions + * are caught and logged to prevent cleanup failures from affecting the + * overall cleanup process.

+ * + * @see #cleanUp() + * @see #cleanUpTemporaryFiles() */ protected void cleanUpDiskFileItems() { LOG.debug("Clean up all DiskFileItem instances (both form fields and file uploads"); @@ -171,10 +279,12 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { if (item.isInMemory()) { LOG.debug("Cleaning up in-memory item: {}", normalizeSpace(item.getFieldName())); } else { - LOG.debug("Cleaning up disk item: {} at {}", normalizeSpace(item.getFieldName()), item.getPath()); - if (item.getPath() != null && item.getPath().toFile().exists()) { - if (!item.getPath().toFile().delete()) { - LOG.warn("There was a problem attempting to delete temporary file: {}", item.getPath()); + Path itemPath = item.getPath(); + LOG.debug("Cleaning up disk item: {} at {}", normalizeSpace(item.getFieldName()), itemPath); + if (itemPath != null) { + File itemFile = itemPath.toFile(); + if (itemFile.exists() && !itemFile.delete()) { + LOG.warn("There was a problem attempting to delete temporary file: {}", itemPath); } } } @@ -186,7 +296,26 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { /** * Cleans up temporary files created for in-memory uploads. - * This method can be overridden by subclasses to customize cleanup behavior. + * + *

This method deletes all temporary files that were created when + * processing in-memory uploads. These files are created in + * {@link #processFileField(DiskFileItem)} when an uploaded file is + * stored in memory and needs to be written to disk.

+ * + *

The cleanup process:

+ *
    + *
  1. Iterates through all tracked temporary files
    2. + *
  2. Checks if each file still exists
    3. + *
  3. Attempts to delete existing files
    4. + *
  4. Logs warnings for files that cannot be deleted
    5. + *
+ * + *

This method can be overridden by subclasses to customize cleanup + * behavior. All exceptions are caught and logged to ensure cleanup + * continues even if individual file deletions fail.

+ * + * @see #cleanUp() + * @see #cleanUpDiskFileItems() */ protected void cleanUpTemporaryFiles() { LOG.debug("Cleaning up {} temporary files created for in-memory uploads", temporaryFiles.size()); @@ -207,7 +336,28 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { } /** - * Override cleanUp to ensure all DiskFileItem instances and temporary files are properly cleaned up + * Performs complete cleanup of all resources associated with this request. + * + *

This method extends the parent cleanup functionality to ensure proper + * cleanup of Jakarta-specific resources:

+ *
    + *
  1. Calls parent cleanup to handle base class resources
    2. + *
  2. Cleans up all tracked {@link DiskFileItem} instances
    3. + *
  3. Cleans up all temporary files created for in-memory uploads
    4. + *
  4. Clears internal tracking collections
    5. + *
+ * + *

This method is designed to be safe to call multiple times and will + * not throw exceptions even if cleanup operations fail. All errors are + * logged for debugging purposes.

+ * + *

Important: This method should always be called in a + * finally block to ensure resources are properly released, even if + * exceptions occur during request processing.

+ * + * @see #cleanUpDiskFileItems() + * @see #cleanUpTemporaryFiles() + * @see AbstractMultiPartRequest#cleanUp() */ @Override public void cleanUp() { 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 5dba4fdd3..9e59aa766 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 @@ -81,36 +81,62 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { }); } + /** + * Reads the entire contents of an input stream into a string. + * + *

This method uses a buffered approach to efficiently read the stream + * content without loading the entire stream into memory at once. It uses + * try-with-resources to ensure proper cleanup of resources.

+ * + * @param inputStream the input stream to read from + * @return the stream contents as a UTF-8 string + * @throws IOException if an error occurs reading the stream + */ private String readStream(InputStream inputStream) throws IOException { + // Use try-with-resources to ensure ByteArrayOutputStream is properly closed try (ByteArrayOutputStream result = new ByteArrayOutputStream()) { - byte[] buffer = new byte[1024]; + byte[] buffer = new byte[1024]; // 1KB buffer for efficient reading + // Read the stream in chunks to avoid loading everything into memory at once for (int length; (length = inputStream.read(buffer)) != -1; ) { result.write(buffer, 0, length); } + // Convert to string using UTF-8 encoding return result.toString(StandardCharsets.UTF_8); } } /** - * Processes the FileItem as a normal form field. - * - * @param fileItemInput a form field item input + * Processes a normal form field (non-file) from the multipart request using streaming API. + * + *

This method handles text form fields by:

+ *
    + *
  1. Validating the field name is not null
    2. + *
  2. Reading the field value from the input stream
    3. + *
  3. Checking if the field value exceeds maximum string length
    4. + *
  4. Adding the value to the parameters collection
    5. + *
+ * + *

Fields with null names are skipped with a warning log message.

+ *

The streaming approach is more memory-efficient for large form data.

+ * + * @param fileItemInput a form field item input from the streaming API + * @throws IOException if an error occurs reading the input stream + * @see #readStream(InputStream) + * @see #exceedsMaxStringLength(String, String) */ protected void processFileItemAsFormField(FileItemInput fileItemInput) throws IOException { String fieldName = fileItemInput.getFieldName(); + if (fieldName == null) { + LOG.warn("Form field has null fieldName, skipping"); + return; + } + String fieldValue = readStream(fileItemInput.getInputStream()); - if (exceedsMaxStringLength(fieldName, fieldValue)) { return; } - List values; - if (parameters.containsKey(fieldName)) { - values = parameters.get(fieldName); - } else { - values = new ArrayList<>(); - parameters.put(fieldName, values); - } + List values = parameters.computeIfAbsent(fieldName, k -> new ArrayList<>()); values.add(fieldValue); } @@ -165,10 +191,28 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { } /** - * Processes the FileItem as a file field. - * - * @param fileItemInput file item representing upload file - * @param location location + * Processes a file field from the multipart request using streaming API. + * + *

This method handles file uploads by:

+ *
    + *
  1. Validating the file name and field name are not null/empty
    2. + *
  2. Checking if the upload exceeds maximum file count
    3. + *
  3. Creating a temporary file in the specified location
    4. + *
  4. Streaming the file content directly to disk
    5. + *
  5. Checking if the total size exceeds maximum allowed size
    6. + *
  6. Creating an {@link UploadedFile} abstraction or cleaning up on size exceeded
    7. + *
+ * + *

Files with null names or field names are skipped with appropriate logging.

+ *

The streaming approach is more memory-efficient for large file uploads + * as it writes directly to disk rather than loading into memory first.

+ * + * @param fileItemInput file item representing upload file from streaming API + * @param location the directory where temporary files will be created + * @throws IOException if an error occurs during file processing + * @see #createTemporaryFile(String, Path) + * @see #streamFileToDisk(FileItemInput, File) + * @see #createUploadedFile(FileItemInput, File) */ protected void processFileItemAsFileField(FileItemInput fileItemInput, Path location) throws IOException { // Skip file uploads that don't have a file name - meaning that no file was selected. @@ -176,6 +220,12 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { LOG.debug(() -> "No file has been uploaded for the field: " + normalizeSpace(fileItemInput.getFieldName())); return; } + + // Skip file uploads that don't have a field name + if (fileItemInput.getFieldName() == null) { + LOG.warn("File upload has null fieldName, skipping"); + return; + } if (exceedsMaxFiles(fileItemInput)) { return; @@ -224,29 +274,40 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { } /** - * Create {@link UploadedFile} abstraction over uploaded file - * - * @param fileItemInput file item stream - * @param file the file + * Creates an {@link UploadedFile} abstraction over an uploaded file. + * + *

This method creates a wrapper around the uploaded file that provides + * a consistent interface for accessing file information and content. + * The created {@link UploadedFile} instance contains:

+ *
    + *
  • The original filename as provided by the client
    • + *
  • The content type (MIME type) if available
    • + *
  • The form field name that contained the file
    • + *
  • A reference to the temporary file on disk
    • + *
+ * + *

The file is automatically added to the uploaded files collection, + * grouped by field name to support multiple file uploads per field.

+ * + * @param fileItemInput file item stream containing file metadata + * @param file the temporary file containing the uploaded content + * @see UploadedFile + * @see StrutsUploadedFile */ protected void createUploadedFile(FileItemInput fileItemInput, File file) { String fileName = fileItemInput.getName(); String fieldName = fileItemInput.getFieldName(); - + + // fieldName null check already done in processFileItemAsFileField UploadedFile uploadedFile = StrutsUploadedFile.Builder .create(file) .withOriginalName(fileName) .withContentType(fileItemInput.getContentType()) - .withInputName(fileItemInput.getFieldName()) + .withInputName(fieldName) .build(); - if (uploadedFiles.containsKey(fieldName)) { - uploadedFiles.get(fieldName).add(uploadedFile); - } else { - List infos = new ArrayList<>(); - infos.add(uploadedFile); - uploadedFiles.put(fieldName, infos); - } + List infos = uploadedFiles.computeIfAbsent(fieldName, key -> new ArrayList<>()); + infos.add(uploadedFile); } }