Skip to content

Commit 63f2c8b

Browse files
committed
Adds missing JavaDocs
1 parent 359b654 commit 63f2c8b

File tree

2 files changed

+260
-49
lines changed

2 files changed

+260
-49
lines changed

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

Lines changed: 170 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,37 @@
3838

3939
/**
4040
* Multipart form data request adapter for Jakarta Commons FileUpload package.
41+
*
42+
* <p>This implementation provides secure handling of multipart requests with proper
43+
* resource management and cleanup. It tracks all temporary files created during
44+
* the upload process and ensures they are properly cleaned up to prevent
45+
* resource leaks and security vulnerabilities.</p>
46+
*
47+
* <p>Key features:</p>
48+
* <ul>
49+
* <li>Automatic tracking and cleanup of temporary files</li>
50+
* <li>Proper error handling with user-friendly error messages</li>
51+
* <li>Support for both in-memory and disk-based file uploads</li>
52+
* <li>Extensible cleanup mechanisms for customization</li>
53+
* </ul>
54+
*
55+
* <p>Usage example:</p>
56+
* <pre>
57+
* JakartaMultiPartRequest multipartRequest = new JakartaMultiPartRequest();
58+
* try {
59+
* multipartRequest.parse(request, "/tmp/uploads");
60+
* // Process uploaded files
61+
* for (String fieldName : multipartRequest.getFileParameterNames()) {
62+
* List&lt;UploadedFile&gt; files = multipartRequest.getFile(fieldName);
63+
* // Handle files
64+
* }
65+
* } finally {
66+
* multipartRequest.cleanUp(); // Always clean up resources
67+
* }
68+
* </pre>
69+
*
70+
* @see AbstractMultiPartRequest
71+
* @see org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload
4172
*/
4273
public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
4374

@@ -53,6 +84,27 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
5384
*/
5485
private final List<File> temporaryFiles = new ArrayList<>();
5586

87+
/**
88+
* Processes the multipart upload request using Jakarta Commons FileUpload.
89+
*
90+
* <p>This method handles the core upload processing by:</p>
91+
* <ol>
92+
* <li>Reading the character encoding from the request</li>
93+
* <li>Preparing the Jakarta servlet file upload handler</li>
94+
* <li>Creating a request context for processing</li>
95+
* <li>Iterating through all form items (fields and files)</li>
96+
* <li>Processing each item appropriately based on its type</li>
97+
* </ol>
98+
*
99+
* <p>All {@link org.apache.commons.fileupload2.core.DiskFileItem} instances
100+
* are automatically tracked for proper cleanup.</p>
101+
*
102+
* @param request the HTTP servlet request containing the multipart data
103+
* @param saveDir the directory where uploaded files will be stored
104+
* @throws IOException if an error occurs during upload processing
105+
* @see #processNormalFormField(DiskFileItem, Charset)
106+
* @see #processFileField(DiskFileItem)
107+
*/
56108
@Override
57109
protected void processUpload(HttpServletRequest request, String saveDir) throws IOException {
58110
Charset charset = readCharsetEncoding(request);
@@ -63,29 +115,51 @@ protected void processUpload(HttpServletRequest request, String saveDir) throws
63115
RequestContext requestContext = createRequestContext(request);
64116

65117
for (DiskFileItem item : servletFileUpload.parseRequest(requestContext)) {
66-
// Track all DiskFileItem instances for cleanup
118+
// Track all DiskFileItem instances for cleanup - this is critical for security
119+
// as it ensures temporary files are properly cleaned up even if processing fails
67120
diskFileItems.add(item);
68121

69122
LOG.debug(() -> "Processing a form field: " + normalizeSpace(item.getFieldName()));
70123
if (item.isFormField()) {
124+
// Process regular form fields (text inputs, checkboxes, etc.)
71125
processNormalFormField(item, charset);
72126
} else {
127+
// Process file upload fields
73128
LOG.debug(() -> "Processing a file: " + normalizeSpace(item.getFieldName()));
74129
processFileField(item);
75130
}
76131
}
77132
}
78133

134+
/**
135+
* Processes a normal form field (non-file) from the multipart request.
136+
*
137+
* <p>This method handles text form fields by:</p>
138+
* <ol>
139+
* <li>Validating the field name is not null</li>
140+
* <li>Extracting the field value using the specified charset</li>
141+
* <li>Checking if the field value exceeds maximum string length</li>
142+
* <li>Adding the value to the parameters map</li>
143+
* </ol>
144+
*
145+
* <p>Fields with null names are skipped with a warning log message.</p>
146+
* <p>Empty form fields are stored as empty strings.</p>
147+
*
148+
* @param item the disk file item representing the form field
149+
* @param charset the character set to use for decoding the field value
150+
* @throws IOException if an error occurs reading the field value
151+
* @see #exceedsMaxStringLength(String, String)
152+
*/
79153
protected void processNormalFormField(DiskFileItem item, Charset charset) throws IOException {
80154
LOG.debug("Item: {} is a normal form field", normalizeSpace(item.getName()));
81155

82-
List<String> values;
83156
String fieldName = item.getFieldName();
84-
if (parameters.get(fieldName) != null) {
85-
values = parameters.get(fieldName);
86-
} else {
87-
values = new ArrayList<>();
157+
if (fieldName == null) {
158+
LOG.warn("Form field has null fieldName, skipping");
159+
return;
88160
}
161+
162+
List<String> values = parameters.computeIfAbsent(fieldName, k -> new ArrayList<>());
89163

90164
String fieldValue = item.getString(charset);
91165
if (exceedsMaxStringLength(fieldName, fieldValue)) {
@@ -99,25 +173,45 @@ protected void processNormalFormField(DiskFileItem item, Charset charset) throws
99173
parameters.put(fieldName, values);
100174
}
101175

176+
/**
177+
* Processes a file field from the multipart request.
178+
*
179+
* <p>This method handles file uploads by:</p>
180+
* <ol>
181+
* <li>Validating the file name and field name are not null/empty</li>
182+
* <li>Determining if the file is stored in memory or on disk</li>
183+
* <li>For in-memory files: creating a temporary file and copying content</li>
184+
* <li>For disk files: using the existing file directly</li>
185+
* <li>Creating an {@link UploadedFile} abstraction</li>
186+
* <li>Adding the file to the uploaded files collection</li>
187+
* </ol>
188+
*
189+
* <p>Temporary files created for in-memory uploads are automatically
190+
* tracked for cleanup. Any errors during temporary file creation are
191+
* logged and added to the error list for user feedback.</p>
192+
*
193+
* @param item the disk file item representing the uploaded file
194+
* @see #cleanUpTemporaryFiles()
195+
*/
102196
protected void processFileField(DiskFileItem item) {
103197
// Skip file uploads that don't have a file name - meaning that no file was selected.
104198
if (item.getName() == null || item.getName().trim().isEmpty()) {
105199
LOG.debug(() -> "No file has been uploaded for the field: " + normalizeSpace(item.getFieldName()));
106200
return;
107201
}
108202

109-
List<UploadedFile> values;
110-
if (uploadedFiles.get(item.getFieldName()) != null) {
111-
values = uploadedFiles.get(item.getFieldName());
112-
} else {
113-
values = new ArrayList<>();
203+
String fieldName = item.getFieldName();
204+
if (fieldName == null) {
205+
LOG.warn("File field has null fieldName, skipping");
206+
return;
114207
}
208+
209+
List<UploadedFile> values = uploadedFiles.computeIfAbsent(fieldName, k -> new ArrayList<>());
115210

116211
if (item.isInMemory()) {
117212
LOG.debug("Creating temporary file representing in-memory uploaded item: {}", normalizeSpace(item.getFieldName()));
118213
try {
119214
File tempFile = File.createTempFile("struts_upload_", "_" + item.getName());
120-
tempFile.deleteOnExit(); // Ensure cleanup on JVM exit as fallback
121215

122216
// Track the temporary file for explicit cleanup
123217
temporaryFiles.add(tempFile);
@@ -157,12 +251,26 @@ protected void processFileField(DiskFileItem item) {
157251
values.add(uploadedFile);
158252
}
159253

160-
uploadedFiles.put(item.getFieldName(), values);
254+
uploadedFiles.put(fieldName, values);
161255
}
162256

163257
/**
164258
* Cleans up disk file items by deleting associated temporary files.
165-
* This method can be overridden by subclasses to customize cleanup behavior.
259+
*
260+
* <p>This method iterates through all tracked {@link DiskFileItem} instances
261+
* and performs cleanup operations:</p>
262+
* <ul>
263+
* <li>For in-memory items: logs cleanup (no files to delete)</li>
264+
* <li>For disk items: deletes the associated temporary file</li>
265+
* </ul>
266+
*
267+
* <p>This method is called automatically during {@link #cleanUp()} but can
268+
* be overridden by subclasses to customize cleanup behavior. All exceptions
269+
* are caught and logged to prevent cleanup failures from affecting the
270+
* overall cleanup process.</p>
271+
*
272+
* @see #cleanUp()
273+
* @see #cleanUpTemporaryFiles()
166274
*/
167275
protected void cleanUpDiskFileItems() {
168276
LOG.debug("Clean up all DiskFileItem instances (both form fields and file uploads");
@@ -171,10 +279,12 @@ protected void cleanUpDiskFileItems() {
171279
if (item.isInMemory()) {
172280
LOG.debug("Cleaning up in-memory item: {}", normalizeSpace(item.getFieldName()));
173281
} else {
174-
LOG.debug("Cleaning up disk item: {} at {}", normalizeSpace(item.getFieldName()), item.getPath());
175-
if (item.getPath() != null && item.getPath().toFile().exists()) {
176-
if (!item.getPath().toFile().delete()) {
177-
LOG.warn("There was a problem attempting to delete temporary file: {}", item.getPath());
282+
Path itemPath = item.getPath();
283+
LOG.debug("Cleaning up disk item: {} at {}", normalizeSpace(item.getFieldName()), itemPath);
284+
if (itemPath != null) {
285+
File itemFile = itemPath.toFile();
286+
if (itemFile.exists() && !itemFile.delete()) {
287+
LOG.warn("There was a problem attempting to delete temporary file: {}", itemPath);
178288
}
179289
}
180290
}
@@ -186,7 +296,26 @@ protected void cleanUpDiskFileItems() {
186296

187297
/**
188298
* Cleans up temporary files created for in-memory uploads.
189-
* This method can be overridden by subclasses to customize cleanup behavior.
299+
*
300+
* <p>This method deletes all temporary files that were created when
301+
* processing in-memory uploads. These files are created in
302+
* {@link #processFileField(DiskFileItem)} when an uploaded file is
303+
* stored in memory and needs to be written to disk.</p>
304+
*
305+
* <p>The cleanup process:</p>
306+
* <ol>
307+
* <li>Iterates through all tracked temporary files</li>
308+
* <li>Checks if each file still exists</li>
309+
* <li>Attempts to delete existing files</li>
310+
* <li>Logs warnings for files that cannot be deleted</li>
311+
* </ol>
312+
*
313+
* <p>This method can be overridden by subclasses to customize cleanup
314+
* behavior. All exceptions are caught and logged to ensure cleanup
315+
* continues even if individual file deletions fail.</p>
316+
*
317+
* @see #cleanUp()
318+
* @see #cleanUpDiskFileItems()
190319
*/
191320
protected void cleanUpTemporaryFiles() {
192321
LOG.debug("Cleaning up {} temporary files created for in-memory uploads", temporaryFiles.size());
@@ -207,7 +336,28 @@ protected void cleanUpTemporaryFiles() {
207336
}
208337

209338
/**
210-
* Override cleanUp to ensure all DiskFileItem instances and temporary files are properly cleaned up
339+
* Performs complete cleanup of all resources associated with this request.
340+
*
341+
* <p>This method extends the parent cleanup functionality to ensure proper
342+
* cleanup of Jakarta-specific resources:</p>
343+
* <ol>
344+
* <li>Calls parent cleanup to handle base class resources</li>
345+
* <li>Cleans up all tracked {@link DiskFileItem} instances</li>
346+
* <li>Cleans up all temporary files created for in-memory uploads</li>
347+
* <li>Clears internal tracking collections</li>
348+
* </ol>
349+
*
350+
* <p>This method is designed to be safe to call multiple times and will
351+
* not throw exceptions even if cleanup operations fail. All errors are
352+
* logged for debugging purposes.</p>
353+
*
354+
* <p><strong>Important:</strong> This method should always be called in a
355+
* finally block to ensure resources are properly released, even if
356+
* exceptions occur during request processing.</p>
357+
*
358+
* @see #cleanUpDiskFileItems()
359+
* @see #cleanUpTemporaryFiles()
360+
* @see AbstractMultiPartRequest#cleanUp()
211361
*/
212362
@Override
213363
public void cleanUp() {

0 commit comments

Comments
 (0)