Apache/struts
1
0
Fork 0
mirror of https://github.com/apache/struts.git synced 2026-08-05 14:47:09 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1283 from apache/dependabot/maven/org.apache.commons-commons-fileupload2-jakarta-servlet6-2.0.0-M4

Browse Source 
Bump org.apache.commons:commons-fileupload2-jakarta-servlet6 from 2.0.0-M2 to 2.0.0-M4
This commit is contained in:
Lukasz Lenart
2025-08-06 10:16:27 +02:00
committed by GitHub
parent fa78ec43a8 f16abd171b
commit dd68723a47
8 changed files with 1482 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+44 -4
View File
@@ -18,6 +18,8 @@
*/
package org.apache.struts2.dispatcher.multipart;
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
import org.apache.commons.fileupload2.core.RequestContext;
import org.apache.struts2.inject.Inject;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException;
@@ -33,6 +35,7 @@ import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
import org.apache.struts2.dispatcher.LocalizedMessage;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Path;
@@ -42,6 +45,9 @@ import java.util.Enumeration;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
/**
* Abstract class with some helper methods, it should be used
@@ -187,7 +193,21 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
* @param charset used charset from incoming request
* @param saveDir a temporary folder to store uploaded files (not always needed)
*/
protected abstract JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir);
protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir) {
DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
LOG.debug("Using file save directory: {}", saveDir);
builder.setPath(saveDir);
LOG.debug("Sets buffer size: {}", bufferSize);
builder.setBufferSize(bufferSize);
LOG.debug("Using charset: {}", charset);
builder.setCharset(charset);
DiskFileItemFactory factory = builder.get();
return new JakartaServletDiskFileUpload(factory);
}
protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset, Path saveDir) {
JakartaServletDiskFileUpload servletFileUpload = createJakartaFileUpload(charset, saveDir);
@@ -207,11 +227,15 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
return servletFileUpload;
}
protected RequestContext createRequestContext(HttpServletRequest request) {
return new StrutsRequestContext(request);
}
protected boolean exceedsMaxStringLength(String fieldName, String fieldValue) {
if (maxStringLength != null && fieldValue.length() > maxStringLength) {
if (LOG.isDebugEnabled()) {
LOG.debug("Form field: {} of size: {} bytes exceeds limit of: {}.",
sanitizeNewlines(fieldName), fieldValue.length(), maxStringLength);
normalizeSpace(fieldName), fieldValue.length(), maxStringLength);
}
LocalizedMessage localizedMessage = new LocalizedMessage(this.getClass(),
STRUTS_MESSAGES_UPLOAD_ERROR_PARAMETER_TOO_LONG_KEY, null,
@@ -234,7 +258,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
try {
processUpload(request, saveDir);
} catch (FileUploadException e) {
LOG.debug("Error parsing the multi-part request!", e);
LOG.warn("Error parsing the multi-part request!", e);
Class<? extends Throwable> exClass = FileUploadException.class;
Object[] args = new Object[]{};
@@ -257,7 +281,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
errors.add(errorMessage);
}
} catch (IOException e) {
LOG.debug("Unable to parse request", e);
LOG.warn("Unable to parse request", e);
LocalizedMessage errorMessage = buildErrorMessage(e.getClass(), e.getMessage(), new Object[]{});
if (!errors.contains(errorMessage)) {
errors.add(errorMessage);
@@ -384,6 +408,22 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
return values.toArray(new String[0]);
}
/**
* Creates a secure temporary file in the specified directory using UUID-based naming.
* This method ensures files are created in a controlled location rather than the
* system temporary directory, reducing security risks.
*
* @param fileName the original filename for logging purposes
* @param location the directory where the temporary file should be created
* @return a new temporary file in the specified location
*/
protected File createTemporaryFile(String fileName, Path location) {
String uid = UUID.randomUUID().toString().replace("-", "_");
File file = location.resolve("upload_" + uid + ".tmp").toFile();
LOG.debug("Creating temporary file: {} (originally: {})", file.getName(), fileName);
return file;
}
/* (non-Javadoc)
* @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
*/
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+280 -33
View File
@@ -20,12 +20,14 @@ package org.apache.struts2.dispatcher.multipart;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.fileupload2.core.DiskFileItem;
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
import org.apache.commons.fileupload2.core.RequestContext;
import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.dispatcher.LocalizedMessage;
import java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Path;
@@ -36,11 +38,73 @@ import static org.apache.commons.lang3.StringUtils.normalizeSpace;
/**
* Multipart form data request adapter for Jakarta Commons FileUpload package.
*
* <p>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.</p>
*
* <p>Key features:</p>
* <ul>
* <li>Automatic tracking and cleanup of temporary files</li>
* <li>Proper error handling with user-friendly error messages</li>
* <li>Support for both in-memory and disk-based file uploads</li>
* <li>Extensible cleanup mechanisms for customization</li>
* </ul>
*
* <p>Usage example:</p>
* <pre>
* JakartaMultiPartRequest multipartRequest = new JakartaMultiPartRequest();
* try {
* multipartRequest.parse(request, "/tmp/uploads");
* // Process uploaded files
* for (String fieldName : multipartRequest.getFileParameterNames()) {
* List&lt;UploadedFile&gt; files = multipartRequest.getFile(fieldName);
* // Handle files
* }
* } finally {
* multipartRequest.cleanUp(); // Always clean up resources
* }
* </pre>
*
* @see AbstractMultiPartRequest
* @see org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload
*/
public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
private static final Logger LOG = LogManager.getLogger(JakartaMultiPartRequest.class);
/**
* List to track all DiskFileItem instances for proper cleanup
*/
private final List<DiskFileItem> diskFileItems = new ArrayList<>();
/**
* List to track temporary files created for in-memory uploads
*/
private final List<File> temporaryFiles = new ArrayList<>();
/**
* Processes the multipart upload request using Jakarta Commons FileUpload.
*
* <p>This method handles the core upload processing by:</p>
* <ol>
* <li>Reading the character encoding from the request</li>
* <li>Preparing the Jakarta servlet file upload handler</li>
* <li>Creating a request context for processing</li>
* <li>Iterating through all form items (fields and files)</li>
* <li>Processing each item appropriately based on its type</li>
* </ol>
*
* <p>All {@link org.apache.commons.fileupload2.core.DiskFileItem} instances
* are automatically tracked for proper cleanup.</p>
*
* @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, String)
*/
@Override
protected void processUpload(HttpServletRequest request, String saveDir) throws IOException {
Charset charset = readCharsetEncoding(request);
@@ -48,44 +112,54 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
JakartaServletDiskFileUpload servletFileUpload =
prepareServletFileUpload(charset, Path.of(saveDir));
for (DiskFileItem item : servletFileUpload.parseRequest(request)) {
RequestContext requestContext = createRequestContext(request);
for (DiskFileItem item : servletFileUpload.parseRequest(requestContext)) {
// 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);
processFileField(item, saveDir);
}
}
}
@Override
protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir) {
DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
LOG.debug("Using file save directory: {}", saveDir);
builder.setPath(saveDir);
LOG.debug("Sets minimal buffer size to always write file to disk");
builder.setBufferSize(1);
LOG.debug("Using charset: {}", charset);
builder.setCharset(charset);
DiskFileItemFactory factory = builder.get();
return new JakartaServletDiskFileUpload(factory);
}
/**
* Processes a normal form field (non-file) from the multipart request.
*
* <p>This method handles text form fields by:</p>
* <ol>
* <li>Validating the field name is not null</li>
* <li>Extracting the field value using the specified charset</li>
* <li>Checking if the field value exceeds maximum string length</li>
* <li>Adding the value to the parameters map</li>
* </ol>
*
* <p>Fields with null names are skipped with a warning log message.</p>
* <p>Empty form fields are stored as empty strings.</p>
*
* @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<String> 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<String> values = parameters.computeIfAbsent(fieldName, k -> new ArrayList<>());
String fieldValue = item.getString(charset);
if (exceedsMaxStringLength(fieldName, fieldValue)) {
@@ -99,22 +173,76 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
parameters.put(fieldName, values);
}
protected void processFileField(DiskFileItem item) {
/**
* Processes a file field from the multipart request.
*
* <p>This method handles file uploads by:</p>
* <ol>
* <li>Validating the file name and field name are not null/empty</li>
* <li>Determining if the file is stored in memory or on disk</li>
* <li>For in-memory files: creating a temporary file and copying content</li>
* <li>For disk files: using the existing file directly</li>
* <li>Creating an {@link UploadedFile} abstraction</li>
* <li>Adding the file to the uploaded files collection</li>
* </ol>
*
* <p>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.</p>
*
* @param item the disk file item representing the uploaded file
* @see #cleanUpTemporaryFiles()
*/
protected void processFileField(DiskFileItem item, String saveDir) {
// Skip file uploads that don't have a file name - meaning that no file was selected.
if (item.getName() == null || item.getName().trim().isEmpty()) {
LOG.debug(() -> "No file has been uploaded for the field: " + normalizeSpace(item.getFieldName()));
return;
}
List<UploadedFile> 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<UploadedFile> values = uploadedFiles.computeIfAbsent(fieldName, k -> new ArrayList<>());
if (item.isInMemory()) {
LOG.warn(() -> "Storing uploaded files just in memory isn't supported currently, skipping file: %s!".formatted(normalizeSpace(item.getName())));
LOG.debug(() -> "Creating temporary file representing in-memory uploaded item: " + normalizeSpace(item.getFieldName()));
try {
File tempFile = createTemporaryFile(item.getName(), Path.of(saveDir));
// Track the temporary file for explicit cleanup
temporaryFiles.add(tempFile);
// Write the in-memory content to the temporary file
try (java.io.FileOutputStream fos = new java.io.FileOutputStream(tempFile)) {
fos.write(item.get());
}
UploadedFile uploadedFile = StrutsUploadedFile.Builder
.create(tempFile)
.withOriginalName(item.getName())
.withContentType(item.getContentType())
.withInputName(item.getFieldName())
.build();
values.add(uploadedFile);
if (LOG.isDebugEnabled()) {
LOG.debug("Created temporary file for in-memory uploaded item: {} at {}",
normalizeSpace(item.getName()), tempFile.getAbsolutePath());
}
} catch (IOException e) {
LOG.warn("Failed to create temporary file for in-memory uploaded item: {}",
normalizeSpace(item.getName()), e);
// Add the error to the errors list for proper user feedback
LocalizedMessage errorMessage = buildErrorMessage(e.getClass(), e.getMessage(), new Object[]{item.getName()});
if (!errors.contains(errorMessage)) {
errors.add(errorMessage);
}
}
} else {
UploadedFile uploadedFile = StrutsUploadedFile.Builder
.create(item.getPath().toFile())
@@ -125,7 +253,126 @@ 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.
*
* <p>This method iterates through all tracked {@link DiskFileItem} instances
* and performs cleanup operations:</p>
* <ul>
* <li>For in-memory items: logs cleanup (no files to delete)</li>
* <li>For disk items: deletes the associated temporary file</li>
* </ul>
*
* <p>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.</p>
*
* @see #cleanUp()
* @see #cleanUpTemporaryFiles()
*/
protected void cleanUpDiskFileItems() {
LOG.debug("Clean up all DiskFileItem instances (both form fields and file uploads");
for (DiskFileItem item : diskFileItems) {
try {
if (item.isInMemory()) {
LOG.debug(() -> "Cleaning up in-memory item: " + normalizeSpace(item.getFieldName()));
} else {
Path itemPath = item.getPath();
if (LOG.isDebugEnabled()) {
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);
}
}
}
} catch (Exception e) {
LOG.warn("Error cleaning up DiskFileItem: {}", normalizeSpace(item.getFieldName()), e);
}
}
}
/**
* Cleans up temporary files created for in-memory uploads.
*
* <p>This method deletes all temporary files that were created when
* processing in-memory uploads. These files are created in
* {@link #processFileField(DiskFileItem, String)} when an uploaded file is
* stored in memory and needs to be written to disk.</p>
*
* <p>The cleanup process:</p>
* <ol>
* <li>Iterates through all tracked temporary files</li>
* <li>Checks if each file still exists</li>
* <li>Attempts to delete existing files</li>
* <li>Logs warnings for files that cannot be deleted</li>
* </ol>
*
* <p>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.</p>
*
* @see #cleanUp()
* @see #cleanUpDiskFileItems()
*/
protected void cleanUpTemporaryFiles() {
LOG.debug("Cleaning up {} temporary files created for in-memory uploads", temporaryFiles.size());
for (File tempFile : temporaryFiles) {
try {
if (tempFile.exists()) {
LOG.debug("Deleting temporary file: {}", tempFile.getAbsolutePath());
if (!tempFile.delete()) {
LOG.warn("There was a problem attempting to delete temporary file: {}", tempFile.getAbsolutePath());
}
} else {
LOG.debug("Temporary file already deleted: {}", tempFile.getAbsolutePath());
}
} catch (Exception e) {
LOG.warn("Error cleaning up temporary file: {}", tempFile.getAbsolutePath(), e);
}
}
}
/**
* Performs complete cleanup of all resources associated with this request.
*
* <p>This method extends the parent cleanup functionality to ensure proper
* cleanup of Jakarta-specific resources:</p>
* <ol>
* <li>Calls parent cleanup to handle base class resources</li>
* <li>Cleans up all tracked {@link DiskFileItem} instances</li>
* <li>Cleans up all temporary files created for in-memory uploads</li>
* <li>Clears internal tracking collections</li>
* </ol>
*
* <p>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.</p>
*
* <p><strong>Important:</strong> This method should always be called in a
* finally block to ensure resources are properly released, even if
* exceptions occur during request processing.</p>
*
* @see #cleanUpDiskFileItems()
* @see #cleanUpTemporaryFiles()
* @see AbstractMultiPartRequest#cleanUp()
*/
@Override
public void cleanUp() {
super.cleanUp();
try {
cleanUpDiskFileItems();
cleanUpTemporaryFiles();
} finally {
diskFileItems.clear();
temporaryFiles.clear();
}
}
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
+96 -66
View File
@@ -19,7 +19,6 @@
package org.apache.struts2.dispatcher.multipart;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
import org.apache.commons.fileupload2.core.FileItemInput;
import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException;
import org.apache.commons.fileupload2.core.FileUploadSizeException;
@@ -40,12 +39,11 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
/**
* Multi-part form data request adapter for Jakarta Commons FileUpload package that
* Multipart form data request adapter for Jakarta Commons FileUpload package that
* leverages the streaming API rather than the traditional non-streaming API.
* <p>
* For more details see WW-3025
@@ -82,51 +80,62 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
});
}
protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path location) {
DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
LOG.debug("Using file save directory: {}", location);
builder.setPath(location);
LOG.debug("Sets buffer size: {}", bufferSize);
builder.setBufferSize(bufferSize);
LOG.debug("Using charset: {}", charset);
builder.setCharset(charset);
DiskFileItemFactory factory = builder.get();
return new JakartaServletDiskFileUpload(factory);
}
/**
* Reads the entire contents of an input stream into a string.
*
* <p>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.</p>
*
* @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 {
ByteArrayOutputStream result = new ByteArrayOutputStream();
byte[] buffer = new byte[1024];
for (int length; (length = inputStream.read(buffer)) != -1; ) {
result.write(buffer, 0, length);
// Use try-with-resources to ensure ByteArrayOutputStream is properly closed
try (ByteArrayOutputStream result = new ByteArrayOutputStream()) {
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);
}
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.
*
* <p>This method handles text form fields by:</p>
* <ol>
* <li>Validating the field name is not null</li>
* <li>Reading the field value from the input stream</li>
* <li>Checking if the field value exceeds maximum string length</li>
* <li>Adding the value to the parameters collection</li>
* </ol>
*
* <p>Fields with null names are skipped with a warning log message.</p>
* <p>The streaming approach is more memory-efficient for large form data.</p>
*
* @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<String> values;
if (parameters.containsKey(fieldName)) {
values = parameters.get(fieldName);
} else {
values = new ArrayList<>();
parameters.put(fieldName, values);
}
List<String> values = parameters.computeIfAbsent(fieldName, k -> new ArrayList<>());
values.add(fieldValue);
}
@@ -181,10 +190,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.
*
* <p>This method handles file uploads by:</p>
* <ol>
* <li>Validating the file name and field name are not null/empty</li>
* <li>Checking if the upload exceeds maximum file count</li>
* <li>Creating a temporary file in the specified location</li>
* <li>Streaming the file content directly to disk</li>
* <li>Checking if the total size exceeds maximum allowed size</li>
* <li>Creating an {@link UploadedFile} abstraction or cleaning up on size exceeded</li>
* </ol>
*
* <p>Files with null names or field names are skipped with appropriate logging.</p>
* <p>The streaming approach is more memory-efficient for large file uploads
* as it writes directly to disk rather than loading into memory first.</p>
*
* @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.
@@ -192,6 +219,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;
@@ -208,20 +241,6 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
}
}
/**
* Creates a temporary file based on the given filename and location.
*
* @param fileName file name
* @param location location
* @return a temporary file based on the given filename and location
*/
protected File createTemporaryFile(String fileName, Path location) {
String uid = UUID.randomUUID().toString().replace("-", "_");
File file = location.resolve("upload_" + uid + ".tmp").toFile();
LOG.debug("Creating temporary file: {} (originally: {})", file.getName(), fileName);
return file;
}
/**
* Streams the file upload stream to the specified file.
*
@@ -240,29 +259,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.
*
* <p>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:</p>
* <ul>
* <li>The original filename as provided by the client</li>
* <li>The content type (MIME type) if available</li>
* <li>The form field name that contained the file</li>
* <li>A reference to the temporary file on disk</li>
* </ul>
*
* <p>The file is automatically added to the uploaded files collection,
* grouped by field name to support multiple file uploads per field.</p>
*
* @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<UploadedFile> infos = new ArrayList<>();
infos.add(uploadedFile);
uploadedFiles.put(fieldName, infos);
}
List<UploadedFile> infos = uploadedFiles.computeIfAbsent(fieldName, key -> new ArrayList<>());
infos.add(uploadedFile);
}
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsRequestContext.java
+61
View File
@@ -0,0 +1,61 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.apache.struts2.dispatcher.multipart;
import jakarta.servlet.http.HttpServletRequest;
import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletRequestContext;
/**
* Provides a specialized request context for Struts applications,
* extending the Jakarta Servlet request context to add custom handling
* for multipart-related requests.
* <p>
* This class overrides multipart detection logic to ensure that requests
* without a content type are not treated as multipart, improving robustness
* in file upload scenarios.
*/
public class StrutsRequestContext extends JakartaServletRequestContext {
/**
* Constructs a context for this request.
*
* @param request The request to which this context applies.
*/
public StrutsRequestContext(HttpServletRequest request) {
super(request);
}
/**
* Determines if the current request is multipart-related.
* <p>
* This implementation first checks if the request's content type is set.
* If the content type is {@code null}, it returns {@code false} immediately.
* Otherwise, it delegates to the superclass implementation to perform
* further checks.
*
* @return {@code true} if the request is multipart-related; {@code false} otherwise.
*/
@Override
public boolean isMultipartRelated() {
if (this.getRequest().getContentType() == null) {
return false;
}
return super.isMultipartRelated();
}
}
core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java
+252
View File
@@ -30,6 +30,9 @@ import org.springframework.mock.web.MockHttpServletRequest;
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -491,6 +494,255 @@ abstract class AbstractMultiPartRequestTest {
.containsExactly("struts.messages.upload.error.FileUploadException");
}
@Test
public void cleanupDoesNotClearErrorsList() throws IOException {
// given - create a scenario that generates errors
String content = formFile("file1", "test1.csv", "1,2,3,4");
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
multiPart.setMaxSize("1"); // Very small to trigger error
multiPart.parse(mockRequest, tempDir);
// Verify errors exist
assertThat(multiPart.getErrors()).isNotEmpty();
int originalErrorCount = multiPart.getErrors().size();
// when
multiPart.cleanUp();
// then - errors should remain (cleanup doesn't clear errors)
assertThat(multiPart.getErrors()).hasSize(originalErrorCount);
}
@Test
public void largeFileUploadHandling() throws IOException {
// Test that large files are handled properly
StringBuilder largeContent = new StringBuilder();
for (int i = 0; i < 1000; i++) {
largeContent.append("line").append(i).append(",");
}
String content = formFile("largefile", "large.csv", largeContent.toString()) +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - should complete without memory issues
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.getFile("largefile")).hasSize(1);
// Cleanup should properly handle large files
multiPart.cleanUp();
assertThat(multiPart.uploadedFiles).isEmpty();
}
@Test
public void multipleFileUploadWithMixedContent() throws IOException {
// Test mixed content with multiple files and parameters
String content = formFile("file1", "test1.csv", "1,2,3,4") +
formField("param1", "value1") +
formFile("file2", "test2.csv", "5,6,7,8") +
formField("param2", "value2") +
formFile("file3", "test3.csv", "9,10,11,12") +
formField("param3", "value3") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - verify all content was processed
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.getFile("file1")).hasSize(1);
assertThat(multiPart.getFile("file2")).hasSize(1);
assertThat(multiPart.getFile("file3")).hasSize(1);
assertThat(multiPart.getParameter("param1")).isEqualTo("value1");
assertThat(multiPart.getParameter("param2")).isEqualTo("value2");
assertThat(multiPart.getParameter("param3")).isEqualTo("value3");
// Store file paths for post-cleanup verification
List<String> filePaths = new ArrayList<>();
for (UploadedFile file : multiPart.getFile("file1")) {
filePaths.add(file.getAbsolutePath());
}
for (UploadedFile file : multiPart.getFile("file2")) {
filePaths.add(file.getAbsolutePath());
}
for (UploadedFile file : multiPart.getFile("file3")) {
filePaths.add(file.getAbsolutePath());
}
// when - cleanup
multiPart.cleanUp();
// then - verify complete cleanup
assertThat(multiPart.uploadedFiles).isEmpty();
assertThat(multiPart.parameters).isEmpty();
// Verify files are deleted
for (String filePath : filePaths) {
assertThat(new File(filePath)).doesNotExist();
}
}
@Test
public void createTemporaryFileGeneratesSecureNames() {
// Create a test instance to access the protected method
AbstractMultiPartRequest testRequest = createMultipartRequest();
Path testLocation = Paths.get(tempDir);
// when - create multiple temporary files
File tempFile1 = testRequest.createTemporaryFile("test1.csv", testLocation);
File tempFile2 = testRequest.createTemporaryFile("test2.csv", testLocation);
File tempFile3 = testRequest.createTemporaryFile("../../../malicious.csv", testLocation);
// then - verify secure naming
assertThat(tempFile1.getName()).startsWith("upload_");
assertThat(tempFile1.getName()).endsWith(".tmp");
assertThat(tempFile2.getName()).startsWith("upload_");
assertThat(tempFile2.getName()).endsWith(".tmp");
assertThat(tempFile3.getName()).startsWith("upload_");
assertThat(tempFile3.getName()).endsWith(".tmp");
// Verify each file has a unique name
assertThat(tempFile1.getName()).isNotEqualTo(tempFile2.getName());
assertThat(tempFile2.getName()).isNotEqualTo(tempFile3.getName());
assertThat(tempFile1.getName()).isNotEqualTo(tempFile3.getName());
// Verify all files are in the correct location
assertThat(tempFile1.getParent()).isEqualTo(tempDir);
assertThat(tempFile2.getParent()).isEqualTo(tempDir);
assertThat(tempFile3.getParent()).isEqualTo(tempDir);
// Verify malicious filename doesn't affect the location
assertThat(tempFile3.getName()).doesNotContain("..");
assertThat(tempFile3.getName()).doesNotContain("/");
assertThat(tempFile3.getName()).doesNotContain("\\");
// Clean up test files
tempFile1.delete();
tempFile2.delete();
tempFile3.delete();
}
@Test
public void createTemporaryFileInSpecificDirectory() throws IOException {
// Create a subdirectory for testing
Path subDir = Paths.get(tempDir, "subdir");
Files.createDirectories(subDir);
AbstractMultiPartRequest testRequest = createMultipartRequest();
// when
File tempFile = testRequest.createTemporaryFile("test.csv", subDir);
// then - verify file is created in the specified subdirectory
assertThat(tempFile.getParent()).isEqualTo(subDir.toString());
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
// Clean up
tempFile.delete();
Files.delete(subDir);
}
@Test
public void createTemporaryFileWithNullFileName() throws IOException {
AbstractMultiPartRequest testRequest = createMultipartRequest();
Path testLocation = Paths.get(tempDir);
// when - create temp file with null filename
File tempFile = testRequest.createTemporaryFile(null, testLocation);
// then - should still create a valid temporary file
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
assertThat(tempFile.getParent()).isEqualTo(tempDir);
// Clean up
tempFile.delete();
}
@Test
public void createTemporaryFileWithEmptyFileName() throws IOException {
AbstractMultiPartRequest testRequest = createMultipartRequest();
Path testLocation = Paths.get(tempDir);
// when - create temp file with empty filename
File tempFile = testRequest.createTemporaryFile("", testLocation);
// then - should still create a valid temporary file
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
assertThat(tempFile.getParent()).isEqualTo(tempDir);
// Clean up
tempFile.delete();
}
@Test
public void createTemporaryFileWithSpecialCharacters() {
AbstractMultiPartRequest testRequest = createMultipartRequest();
Path testLocation = Paths.get(tempDir);
// when - create temp files with various special characters
File tempFile1 = testRequest.createTemporaryFile("file with spaces.csv", testLocation);
File tempFile2 = testRequest.createTemporaryFile("file@#$%^&*().csv", testLocation);
File tempFile3 = testRequest.createTemporaryFile("файл.csv", testLocation); // Cyrillic
// then - all should create valid secure temporary files
File[] tempFiles = {tempFile1, tempFile2, tempFile3};
for (File tempFile : tempFiles) {
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
assertThat(tempFile.getParent()).isEqualTo(tempDir);
// Verify no special characters leak into the actual filename
assertThat(tempFile.getName()).matches("upload_[a-zA-Z0-9_]+\\.tmp");
}
// All should have unique names
assertThat(tempFile1.getName()).isNotEqualTo(tempFile2.getName());
assertThat(tempFile2.getName()).isNotEqualTo(tempFile3.getName());
assertThat(tempFile1.getName()).isNotEqualTo(tempFile3.getName());
// Clean up
tempFile1.delete();
tempFile2.delete();
tempFile3.delete();
}
@Test
public void createTemporaryFileConsistentNaming() {
AbstractMultiPartRequest testRequest = createMultipartRequest();
Path testLocation = Paths.get(tempDir);
// when - create many temporary files to verify naming consistency
List<File> tempFiles = new ArrayList<>();
for (int i = 0; i < 100; i++) {
tempFiles.add(testRequest.createTemporaryFile("test" + i + ".csv", testLocation));
}
// then - all should follow the same naming pattern
for (File tempFile : tempFiles) {
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
assertThat(tempFile.getParent()).isEqualTo(tempDir);
// Verify UUID pattern (without hyphens, replaced with underscores)
assertThat(tempFile.getName()).matches("upload_[a-zA-Z0-9_]+\\.tmp");
}
// Verify all names are unique
List<String> fileNames = tempFiles.stream().map(File::getName).toList();
assertThat(fileNames).doesNotHaveDuplicates();
// Clean up
tempFiles.forEach(File::delete);
}
protected String formFile(String fieldName, String filename, String content) {
return endline +
"--" + boundary + endline +
core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java
+428
View File
@@ -18,6 +18,22 @@
*/
package org.apache.struts2.dispatcher.multipart;
import org.apache.commons.fileupload2.core.DiskFileItem;
import org.apache.struts2.dispatcher.LocalizedMessage;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.junit.Test;
import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.List;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
import static org.assertj.core.api.Assertions.assertThat;
public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest {
@Override
@@ -25,4 +41,416 @@ public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest {
return new JakartaMultiPartRequest();
}
@Test
public void temporaryFileCleanupForInMemoryUploads() throws IOException, NoSuchFieldException, IllegalAccessException {
// given - small files that will be in-memory
String content = formFile("file1", "test1.csv", "a,b,c,d") +
formFile("file2", "test2.csv", "1,2,3,4") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// Access private field to verify temporary files are tracked
Field tempFilesField = JakartaMultiPartRequest.class.getDeclaredField("temporaryFiles");
tempFilesField.setAccessible(true);
@SuppressWarnings("unchecked")
List<File> temporaryFiles = (List<File>) tempFilesField.get(multiPart);
// Store file paths before cleanup for verification
List<String> tempFilePaths = temporaryFiles.stream()
.map(File::getAbsolutePath)
.toList();
// Verify temporary files exist before cleanup
assertThat(temporaryFiles).isNotEmpty();
for (File tempFile : temporaryFiles) {
assertThat(tempFile).exists();
}
// when - cleanup
multiPart.cleanUp();
// then - verify files are deleted and tracking list is cleared
for (String tempFilePath : tempFilePaths) {
assertThat(new File(tempFilePath)).doesNotExist();
}
assertThat(temporaryFiles).isEmpty();
}
@Test
public void cleanupMethodsCanBeOverridden() {
// Create a custom implementation to test extensibility
class CustomJakartaMultiPartRequest extends JakartaMultiPartRequest {
boolean diskFileItemsCleanedUp = false;
boolean temporaryFilesCleanedUp = false;
@Override
protected void cleanUpDiskFileItems() {
diskFileItemsCleanedUp = true;
super.cleanUpDiskFileItems();
}
@Override
protected void cleanUpTemporaryFiles() {
temporaryFilesCleanedUp = true;
super.cleanUpTemporaryFiles();
}
}
CustomJakartaMultiPartRequest customMultiPart = new CustomJakartaMultiPartRequest();
// when
customMultiPart.cleanUp();
// then
assertThat(customMultiPart.diskFileItemsCleanedUp).isTrue();
assertThat(customMultiPart.temporaryFilesCleanedUp).isTrue();
}
@Test
public void temporaryFileCreationFailureAddsError() throws IOException {
// Create a custom implementation that simulates temp file creation failure
class FaultyJakartaMultiPartRequest extends JakartaMultiPartRequest {
@Override
protected void processFileField(DiskFileItem item, String saveDir) {
// Simulate in-memory upload that fails to create temp file
if (item.isInMemory()) {
try {
// Simulate IOException during temp file creation
throw new IOException("Simulated temp file creation failure");
} catch (IOException e) {
// Add the error to the errors list for proper user feedback
LocalizedMessage errorMessage = buildErrorMessage(e.getClass(), e.getMessage(),
new Object[]{item.getName()});
if (!errors.contains(errorMessage)) {
errors.add(errorMessage);
}
}
} else {
super.processFileField(item, saveDir);
}
}
}
FaultyJakartaMultiPartRequest faultyMultiPart = new FaultyJakartaMultiPartRequest();
// given - small file that would normally be in-memory
String content = formFile("file1", "test1.csv", "a,b") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
faultyMultiPart.parse(mockRequest, tempDir);
// then - verify error is properly captured
assertThat(faultyMultiPart.getErrors())
.hasSize(1)
.first()
.extracting(LocalizedMessage::getTextKey)
.isEqualTo("struts.messages.upload.error.IOException");
}
@Test
public void temporaryFileCreationErrorsAreNotDuplicated() throws IOException {
// Test that duplicate errors are not added to the errors list
JakartaMultiPartRequest multiPartWithDuplicateErrors = new JakartaMultiPartRequest();
// Simulate adding the same error twice
IOException testException = new IOException("Test exception");
LocalizedMessage errorMessage = multiPartWithDuplicateErrors.buildErrorMessage(
testException.getClass(), testException.getMessage(), new Object[]{"test.csv"});
// when - add same error twice
multiPartWithDuplicateErrors.errors.add(errorMessage);
if (!multiPartWithDuplicateErrors.errors.contains(errorMessage)) {
multiPartWithDuplicateErrors.errors.add(errorMessage);
}
// then - only one error should be present
assertThat(multiPartWithDuplicateErrors.getErrors()).hasSize(1);
}
@Test
public void cleanupIsIdempotent() throws IOException {
// given - process some files
String content = formFile("file1", "test1.csv", "1,2,3,4") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
multiPart.parse(mockRequest, tempDir);
// when - call cleanup multiple times
multiPart.cleanUp();
multiPart.cleanUp();
multiPart.cleanUp();
// then - should not throw exceptions and should be safe
assertThat(multiPart.uploadedFiles).isEmpty();
assertThat(multiPart.parameters).isEmpty();
}
@Test
public void endToEndMultipartProcessingWithCleanup() throws IOException {
// Test complete multipart processing lifecycle
String content = formFile("file1", "test1.csv", "1,2,3,4") +
formField("param1", "value1") +
formFile("file2", "test2.csv", "5,6,7,8") +
formField("param2", "value2") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when - full processing
multiPart.parse(mockRequest, tempDir);
// then - verify everything was processed
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.getFile("file1")).hasSize(1);
assertThat(multiPart.getFile("file2")).hasSize(1);
assertThat(multiPart.getParameter("param1")).isEqualTo("value1");
assertThat(multiPart.getParameter("param2")).isEqualTo("value2");
// when - cleanup
multiPart.cleanUp();
// then - verify complete cleanup
assertThat(multiPart.uploadedFiles).isEmpty();
assertThat(multiPart.parameters).isEmpty();
}
@Test
public void temporaryFilesCreatedInSaveDirectory() throws IOException, NoSuchFieldException, IllegalAccessException {
// Test that temporary files for in-memory uploads are created in the saveDir, not system temp
String content = formFile("file1", "test1.csv", "small,content") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// Access private field to get temporary files
Field tempFilesField = JakartaMultiPartRequest.class.getDeclaredField("temporaryFiles");
tempFilesField.setAccessible(true);
@SuppressWarnings("unchecked")
List<File> temporaryFiles = (List<File>) tempFilesField.get(multiPart);
// then - verify temporary files are created in saveDir
assertThat(temporaryFiles).isNotEmpty();
for (File tempFile : temporaryFiles) {
// Verify the temporary file is in the saveDir, not system temp
assertThat(tempFile.getParent()).isEqualTo(tempDir);
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
assertThat(tempFile).exists();
}
}
@Test
public void secureTemporaryFileNaming() throws IOException, NoSuchFieldException, IllegalAccessException {
// Test that temporary files use UUID-based naming for security
String content = formFile("file1", "malicious../../../etc/passwd", "content") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// Access private field to get temporary files
Field tempFilesField = JakartaMultiPartRequest.class.getDeclaredField("temporaryFiles");
tempFilesField.setAccessible(true);
@SuppressWarnings("unchecked")
List<File> temporaryFiles = (List<File>) tempFilesField.get(multiPart);
// then - verify secure naming prevents directory traversal
assertThat(temporaryFiles).isNotEmpty();
for (File tempFile : temporaryFiles) {
// Verify the temporary file uses secure UUID naming
assertThat(tempFile.getName()).startsWith("upload_");
assertThat(tempFile.getName()).endsWith(".tmp");
// Verify it doesn't contain malicious path elements
assertThat(tempFile.getName()).doesNotContain("..");
assertThat(tempFile.getName()).doesNotContain("/");
assertThat(tempFile.getName()).doesNotContain("\\");
// Verify it's in the correct directory
assertThat(tempFile.getParent()).isEqualTo(tempDir);
}
}
@Test
public void processNormalFormFieldHandlesNullFieldName() throws IOException {
// Test null field name handling in processNormalFormField
String content =
endline + "--" + boundary + endline +
"Content-Disposition: form-data" + endline + // No name attribute
endline +
"field value without name" +
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"validfield\"" + endline +
endline +
"valid field value" +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - should only process the valid field
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.getParameter("validfield")).isEqualTo("valid field value");
assertThat(multiPart.getParameterNames().asIterator()).toIterable().hasSize(1);
}
@Test
public void processFileFieldHandlesNullFieldName() throws IOException {
// Test null field name handling in processFileField
String content =
endline + "--" + boundary + endline +
"Content-Disposition: form-data; filename=\"orphan.txt\"" + endline + // No name attribute
"Content-Type: text/plain" + endline +
endline +
"orphaned file content" +
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"validfile\"; filename=\"valid.txt\"" + endline +
"Content-Type: text/plain" + endline +
endline +
"valid file content" +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - should only process the valid file
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.uploadedFiles).hasSize(1);
assertThat(multiPart.getFile("validfile")).hasSize(1);
assertThat(multiPart.getFile("validfile")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo("valid file content");
}
@Test
public void diskFileItemCleanupCoverage() throws IOException, NoSuchFieldException, IllegalAccessException {
// Test disk file item cleanup paths
String content = formFile("file1", "test1.csv", "1,2,3,4") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when - force files to disk with small buffer
multiPart.setBufferSize("1");
multiPart.parse(mockRequest, tempDir);
// Access private field to verify disk file items are tracked
Field diskFileItemsField = JakartaMultiPartRequest.class.getDeclaredField("diskFileItems");
diskFileItemsField.setAccessible(true);
@SuppressWarnings("unchecked")
java.util.List<org.apache.commons.fileupload2.core.DiskFileItem> diskFileItems =
(java.util.List<org.apache.commons.fileupload2.core.DiskFileItem>) diskFileItemsField.get(multiPart);
// then - should have disk file items tracked
assertThat(diskFileItems).isNotEmpty();
// when - cleanup
multiPart.cleanUp();
// then - should clear tracking
assertThat(diskFileItems).isEmpty();
}
@Test
public void inMemoryVsDiskFileHandling() throws IOException {
// Test both in-memory and disk file handling paths
String smallContent = "small"; // Should be in-memory
String largeContent = "x".repeat(20000); // Should go to disk
String content = formFile("smallfile", "small.txt", smallContent) +
formFile("largefile", "large.txt", largeContent) +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when - use default buffer size
multiPart.parse(mockRequest, tempDir);
// then - both files should be processed
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.uploadedFiles).hasSize(2);
assertThat(multiPart.getFile("smallfile")).hasSize(1);
assertThat(multiPart.getFile("largefile")).hasSize(1);
// Verify content
assertThat(multiPart.getFile("smallfile")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo(smallContent);
assertThat(multiPart.getFile("largefile")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo(largeContent);
}
@Test
public void errorDuplicationPrevention() throws IOException {
// Test that duplicate errors are not added
JakartaMultiPartRequest multiPartRequest = new JakartaMultiPartRequest();
// Simulate adding the same error multiple times
IOException testException = new IOException("Test error");
LocalizedMessage errorMessage = multiPartRequest.buildErrorMessage(
testException.getClass(), testException.getMessage(), new Object[]{"test.csv"});
// when - try to add same error multiple times
multiPartRequest.errors.add(errorMessage);
if (!multiPartRequest.errors.contains(errorMessage)) {
multiPartRequest.errors.add(errorMessage); // Should not be added
}
if (!multiPartRequest.errors.contains(errorMessage)) {
multiPartRequest.errors.add(errorMessage); // Should not be added
}
// then - should only have one error
assertThat(multiPartRequest.getErrors()).hasSize(1);
}
@Test
public void processFileFieldHandlesEmptyFileName() throws IOException {
String content =
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"emptyfile\"; filename=\"\"" + endline +
"Content-Type: text/plain" + endline +
endline +
"some content that should be ignored" +
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"validfile\"; filename=\"test.txt\"" + endline +
"Content-Type: text/plain" + endline +
endline +
"valid file content" +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - should only process the file with valid filename
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.uploadedFiles).hasSize(1);
assertThat(multiPart.getFile("validfile")).hasSize(1);
assertThat(multiPart.getFile("emptyfile")).isEmpty();
assertThat(multiPart.getFile("validfile")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo("valid file content");
}
}
core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java
+320
View File
@@ -23,10 +23,18 @@ import org.apache.struts2.dispatcher.LocalizedMessage;
import org.assertj.core.api.InstanceOfAssertFactories;
import org.junit.Test;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Field;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.nio.charset.StandardCharsets;
import java.nio.file.Path;
import java.nio.file.Paths;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
public class JakartaStreamMultiPartRequestTest extends AbstractMultiPartRequestTest {
@@ -71,4 +79,316 @@ public class JakartaStreamMultiPartRequestTest extends AbstractMultiPartRequestT
.containsExactly("struts.messages.upload.error.FileUploadSizeException");
}
@Test
public void readStreamProperlyHandlesResources() throws Exception {
// Create a test input stream with known data
byte[] testData = "test data for stream reading".getBytes(StandardCharsets.UTF_8);
InputStream testStream = new java.io.ByteArrayInputStream(testData);
JakartaStreamMultiPartRequest streamMultiPart = new JakartaStreamMultiPartRequest();
// Use reflection to access private readStream method
Method readStreamMethod = JakartaStreamMultiPartRequest.class.getDeclaredMethod("readStream", InputStream.class);
readStreamMethod.setAccessible(true);
// when
String result = (String) readStreamMethod.invoke(streamMultiPart, testStream);
// then
assertThat(result).isEqualTo("test data for stream reading");
}
@Test
public void readStreamHandlesExceptionsProperly() throws Exception {
// Create a stream that throws an exception
InputStream faultyStream = new InputStream() {
@Override
public int read() throws IOException {
throw new IOException("Simulated stream failure");
}
};
JakartaStreamMultiPartRequest streamMultiPart = new JakartaStreamMultiPartRequest();
// Use reflection to access private readStream method
Method readStreamMethod = JakartaStreamMultiPartRequest.class.getDeclaredMethod("readStream", InputStream.class);
readStreamMethod.setAccessible(true);
// when/then - should propagate the exception
assertThatThrownBy(() -> readStreamMethod.invoke(streamMultiPart, faultyStream))
.isInstanceOf(InvocationTargetException.class)
.cause()
.isInstanceOf(IOException.class)
.hasMessage("Simulated stream failure");
}
@Test
public void readStreamHandlesEmptyStream() throws Exception {
// Create an empty stream
InputStream emptyStream = new java.io.ByteArrayInputStream(new byte[0]);
JakartaStreamMultiPartRequest streamMultiPart = new JakartaStreamMultiPartRequest();
// Use reflection to access private readStream method
Method readStreamMethod = JakartaStreamMultiPartRequest.class.getDeclaredMethod("readStream", InputStream.class);
readStreamMethod.setAccessible(true);
// when
String result = (String) readStreamMethod.invoke(streamMultiPart, emptyStream);
// then
assertThat(result).isEmpty();
}
@Test
public void readStreamHandlesLargeData() throws Exception {
// Create a large data stream to test buffer handling
StringBuilder largeData = new StringBuilder();
for (int i = 0; i < 2000; i++) {
largeData.append("line").append(i).append("\n");
}
byte[] testData = largeData.toString().getBytes(StandardCharsets.UTF_8);
InputStream largeStream = new java.io.ByteArrayInputStream(testData);
JakartaStreamMultiPartRequest streamMultiPart = new JakartaStreamMultiPartRequest();
// Use reflection to access private readStream method
Method readStreamMethod = JakartaStreamMultiPartRequest.class.getDeclaredMethod("readStream", InputStream.class);
readStreamMethod.setAccessible(true);
// when
String result = (String) readStreamMethod.invoke(streamMultiPart, largeStream);
// then
assertThat(result).isEqualTo(largeData.toString());
assertThat(result.length()).isGreaterThan(1024); // Verify it's larger than internal buffer
}
@Test
public void processFileItemAsFormFieldHandlesNullFieldName() throws IOException {
// Test the null field name path in processFileItemAsFormField
String content = formFile("", "test.csv", "data") + // Field name will be empty/null-like
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - should complete without error, but no parameters should be added
assertThat(multiPart.getErrors()).isEmpty();
}
@Test
public void processFileItemAsFileFieldHandlesNullFieldName() throws IOException {
// This test covers the null field name path in processFileItemAsFileField
JakartaStreamMultiPartRequest streamMultiPart = new JakartaStreamMultiPartRequest();
// Create a mock file item with null field name
String content = "--" + boundary + endline +
"Content-Disposition: form-data; filename=\"test.csv\"" + endline +
"Content-Type: text/csv" + endline +
endline +
"test data" +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
streamMultiPart.parse(mockRequest, tempDir);
// then - should complete without error but no files should be uploaded
assertThat(streamMultiPart.getErrors()).isEmpty();
assertThat(streamMultiPart.uploadedFiles).isEmpty();
}
@Test
public void exceedsMaxFilesPath() throws IOException {
// Test the exceedsMaxFiles method path
String content = formFile("file1", "test1.csv", "data1") +
formFile("file2", "test2.csv", "data2") +
formFile("file3", "test3.csv", "data3") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// 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);
assertThat(multiPart.getErrors())
.isNotEmpty()
.allSatisfy(error ->
assertThat(error.getTextKey()).isEqualTo("struts.messages.upload.error.FileUploadFileCountLimitException")
);
}
@Test
public void actualSizeOfUploadedFilesCalculation() throws IOException {
// Test the actualSizeOfUploadedFiles method
String content = formFile("file1", "test1.csv", "data1234567890") + // 14 bytes + headers
formFile("file2", "test2.csv", "moredata") + // 8 bytes + headers
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then - should have uploaded files and calculate their total size
assertThat(multiPart.uploadedFiles).hasSize(2);
assertThat(multiPart.getFile("file1")).hasSize(1);
assertThat(multiPart.getFile("file2")).hasSize(1);
// Verify files have the expected content
assertThat(multiPart.getFile("file1")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo("data1234567890");
assertThat(multiPart.getFile("file2")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo("moredata");
}
@Test
public void createTemporaryFileMethod() throws Exception {
// Test the createTemporaryFile method directly
JakartaStreamMultiPartRequest streamMultiPart = new JakartaStreamMultiPartRequest();
Path testLocation = Paths.get(tempDir);
// when
File tempFile1 = streamMultiPart.createTemporaryFile("test.csv", testLocation);
File tempFile2 = streamMultiPart.createTemporaryFile("another.txt", testLocation);
// then
assertThat(tempFile1.getName()).startsWith("upload_");
assertThat(tempFile1.getName()).endsWith(".tmp");
assertThat(tempFile1.getParent()).isEqualTo(tempDir);
assertThat(tempFile2.getName()).startsWith("upload_");
assertThat(tempFile2.getName()).endsWith(".tmp");
assertThat(tempFile2.getParent()).isEqualTo(tempDir);
// Should be unique names
assertThat(tempFile1.getName()).isNotEqualTo(tempFile2.getName());
// Clean up
tempFile1.delete();
tempFile2.delete();
}
@Test
public void streamFileToDiskWithDifferentBufferSizes() throws IOException {
// Test streamFileToDisk with different buffer sizes
String largeContent = "x".repeat(5000); // Content larger than default buffer
String content = formFile("largefile", "large.csv", largeContent) +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when - use small buffer size to ensure multiple reads
multiPart.setBufferSize("100");
multiPart.parse(mockRequest, tempDir);
// then
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.getFile("largefile")).hasSize(1);
assertThat(multiPart.getFile("largefile")[0].getContent())
.asInstanceOf(InstanceOfAssertFactories.FILE)
.content()
.isEqualTo(largeContent);
}
@Test
public void exceedsMaxSizeOfFilesWithFileCleanup() throws IOException {
// Test the file deletion path when max size is exceeded
String content = formFile("file1", "test1.csv", "small") +
formFile("file2", "test2.csv", "this is a much larger file content that should exceed the limit") +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when - set very small max size
multiPart.setMaxSizeOfFiles("20");
multiPart.parse(mockRequest, tempDir);
// then - should have first file uploaded but error for second
assertThat(multiPart.uploadedFiles).hasSize(1);
assertThat(multiPart.getFile("file1")).hasSize(1);
assertThat(multiPart.getFile("file2")).isEmpty();
assertThat(multiPart.getErrors())
.isNotEmpty()
.anyMatch(error ->
error.getTextKey().equals("struts.messages.upload.error.FileUploadSizeException")
);
}
@Test
public void createUploadedFileWithVariousContentTypes() throws IOException {
// Test different content types and file names
String content =
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"textfile\"; filename=\"document.txt\"" + endline +
"Content-Type: text/plain" + endline +
endline +
"Plain text content" +
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"jsonfile\"; filename=\"data.json\"" + endline +
"Content-Type: application/json" + endline +
endline +
"{\"key\": \"value\"}" +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.uploadedFiles).hasSize(2);
// Verify text file
assertThat(multiPart.getFile("textfile")).hasSize(1);
assertThat(multiPart.getFile("textfile")[0].getContentType()).isEqualTo("text/plain");
assertThat(multiPart.getFile("textfile")[0].getOriginalName()).isEqualTo("document.txt");
// Verify JSON file
assertThat(multiPart.getFile("jsonfile")).hasSize(1);
assertThat(multiPart.getFile("jsonfile")[0].getContentType()).isEqualTo("application/json");
assertThat(multiPart.getFile("jsonfile")[0].getOriginalName()).isEqualTo("data.json");
}
@Test
public void emptyFileNameFieldsAreSkipped() throws IOException {
// Test files with empty names are skipped
String content =
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"emptyfile\"; filename=\"\"" + endline +
"Content-Type: text/plain" + endline +
endline +
"This should be skipped" +
endline + "--" + boundary + endline +
"Content-Disposition: form-data; name=\"validfile\"; filename=\"valid.txt\"" + endline +
"Content-Type: text/plain" + endline +
endline +
"This should be processed" +
endline + "--" + boundary + "--";
mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8));
// when
multiPart.parse(mockRequest, tempDir);
// then
assertThat(multiPart.getErrors()).isEmpty();
assertThat(multiPart.uploadedFiles).hasSize(1);
assertThat(multiPart.getFile("emptyfile")).isEmpty();
assertThat(multiPart.getFile("validfile")).hasSize(1);
}
}
parent/pom.xml
+1 -1
View File
@@ -133,7 +133,7 @@
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-fileupload2-jakarta-servlet6</artifactId>
<version>2.0.0-M2</version>
<version>2.0.0-M4</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>