Skip to content

Commit a1c4cb6

Browse files
lukaszlenartlukaszlenart
committed
Uses a dedicated RequestContext to avoid NPE
1 parent b796bab commit a1c4cb6

File tree

4 files changed

+154
-42
lines changed

4 files changed

+154
-42
lines changed

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

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
*/
1919
package org.apache.struts2.dispatcher.multipart;
2020

21+
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
22+
import org.apache.commons.fileupload2.core.RequestContext;
2123
import org.apache.struts2.inject.Inject;
2224
import jakarta.servlet.http.HttpServletRequest;
2325
import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException;
@@ -43,6 +45,8 @@
4345
import java.util.List;
4446
import java.util.Map;
4547

48+
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
49+
4650
/**
4751
* Abstract class with some helper methods, it should be used
4852
* when starting development of another implementation of {@link MultiPartRequest}
@@ -187,7 +191,21 @@ protected Charset readCharsetEncoding(HttpServletRequest request) {
187191
* @param charset used charset from incoming request
188192
* @param saveDir a temporary folder to store uploaded files (not always needed)
189193
*/
190-
protected abstract JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir);
194+
protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir) {
195+
DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
196+
197+
LOG.debug("Using file save directory: {}", saveDir);
198+
builder.setPath(saveDir);
199+
200+
LOG.debug("Sets buffer size: {}", bufferSize);
201+
builder.setBufferSize(bufferSize);
202+
203+
LOG.debug("Using charset: {}", charset);
204+
builder.setCharset(charset);
205+
206+
DiskFileItemFactory factory = builder.get();
207+
return new JakartaServletDiskFileUpload(factory);
208+
}
191209

192210
protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset, Path saveDir) {
193211
JakartaServletDiskFileUpload servletFileUpload = createJakartaFileUpload(charset, saveDir);
@@ -207,11 +225,15 @@ protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset,
207225
return servletFileUpload;
208226
}
209227

228+
protected RequestContext createRequestContext(HttpServletRequest request) {
229+
return new StrutsRequestContext(request);
230+
}
231+
210232
protected boolean exceedsMaxStringLength(String fieldName, String fieldValue) {
211233
if (maxStringLength != null && fieldValue.length() > maxStringLength) {
212234
if (LOG.isDebugEnabled()) {
213235
LOG.debug("Form field: {} of size: {} bytes exceeds limit of: {}.",
214-
sanitizeNewlines(fieldName), fieldValue.length(), maxStringLength);
236+
normalizeSpace(fieldName), fieldValue.length(), maxStringLength);
215237
}
216238
LocalizedMessage localizedMessage = new LocalizedMessage(this.getClass(),
217239
STRUTS_MESSAGES_UPLOAD_ERROR_PARAMETER_TOO_LONG_KEY, null,
@@ -234,7 +256,7 @@ public void parse(HttpServletRequest request, String saveDir) throws IOException
234256
try {
235257
processUpload(request, saveDir);
236258
} catch (FileUploadException e) {
237-
LOG.debug("Error parsing the multi-part request!", e);
259+
LOG.warn("Error parsing the multi-part request!", e);
238260
Class<? extends Throwable> exClass = FileUploadException.class;
239261
Object[] args = new Object[]{};
240262

@@ -257,7 +279,7 @@ public void parse(HttpServletRequest request, String saveDir) throws IOException
257279
errors.add(errorMessage);
258280
}
259281
} catch (IOException e) {
260-
LOG.debug("Unable to parse request", e);
282+
LOG.warn("Unable to parse request", e);
261283
LocalizedMessage errorMessage = buildErrorMessage(e.getClass(), e.getMessage(), new Object[]{});
262284
if (!errors.contains(errorMessage)) {
263285
errors.add(errorMessage);

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

Lines changed: 66 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,13 @@
2020

2121
import jakarta.servlet.http.HttpServletRequest;
2222
import org.apache.commons.fileupload2.core.DiskFileItem;
23-
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
23+
import org.apache.commons.fileupload2.core.RequestContext;
2424
import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload;
2525
import org.apache.commons.lang3.StringUtils;
2626
import org.apache.logging.log4j.LogManager;
2727
import org.apache.logging.log4j.Logger;
2828

29+
import java.io.File;
2930
import java.io.IOException;
3031
import java.nio.charset.Charset;
3132
import java.nio.file.Path;
@@ -41,14 +42,24 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
4142

4243
private static final Logger LOG = LogManager.getLogger(JakartaMultiPartRequest.class);
4344

45+
/**
46+
* List to track all DiskFileItem instances for proper cleanup
47+
*/
48+
private final List<DiskFileItem> diskFileItems = new ArrayList<>();
49+
4450
@Override
4551
protected void processUpload(HttpServletRequest request, String saveDir) throws IOException {
4652
Charset charset = readCharsetEncoding(request);
4753

4854
JakartaServletDiskFileUpload servletFileUpload =
4955
prepareServletFileUpload(charset, Path.of(saveDir));
5056

51-
for (DiskFileItem item : servletFileUpload.parseRequest(request)) {
57+
RequestContext requestContext = createRequestContext(request);
58+
59+
for (DiskFileItem item : servletFileUpload.parseRequest(requestContext)) {
60+
// Track all DiskFileItem instances for cleanup
61+
diskFileItems.add(item);
62+
5263
LOG.debug(() -> "Processing a form field: " + normalizeSpace(item.getFieldName()));
5364
if (item.isFormField()) {
5465
processNormalFormField(item, charset);
@@ -59,23 +70,6 @@ protected void processUpload(HttpServletRequest request, String saveDir) throws
5970
}
6071
}
6172

62-
@Override
63-
protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir) {
64-
DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
65-
66-
LOG.debug("Using file save directory: {}", saveDir);
67-
builder.setPath(saveDir);
68-
69-
LOG.debug("Sets minimal buffer size to always write file to disk");
70-
builder.setBufferSize(1);
71-
72-
LOG.debug("Using charset: {}", charset);
73-
builder.setCharset(charset);
74-
75-
DiskFileItemFactory factory = builder.get();
76-
return new JakartaServletDiskFileUpload(factory);
77-
}
78-
7973
protected void processNormalFormField(DiskFileItem item, Charset charset) throws IOException {
8074
LOG.debug("Item: {} is a normal form field", normalizeSpace(item.getName()));
8175

@@ -114,7 +108,30 @@ protected void processFileField(DiskFileItem item) {
114108
}
115109

116110
if (item.isInMemory()) {
117-
LOG.warn(() -> "Storing uploaded files just in memory isn't supported currently, skipping file: %s!".formatted(normalizeSpace(item.getName())));
111+
LOG.debug("Creating temporary file representing in-memory uploaded item: {}", normalizeSpace(item.getFieldName()));
112+
try {
113+
File tempFile = File.createTempFile("struts_upload_", "_" + item.getName());
114+
tempFile.deleteOnExit(); // Ensure cleanup on JVM exit
115+
116+
// Write the in-memory content to the temporary file
117+
try (java.io.FileOutputStream fos = new java.io.FileOutputStream(tempFile)) {
118+
fos.write(item.get());
119+
}
120+
121+
UploadedFile uploadedFile = StrutsUploadedFile.Builder
122+
.create(tempFile)
123+
.withOriginalName(item.getName())
124+
.withContentType(item.getContentType())
125+
.withInputName(item.getFieldName())
126+
.build();
127+
values.add(uploadedFile);
128+
129+
LOG.debug("Created temporary file for in-memory uploaded item: {} at {}",
130+
normalizeSpace(item.getName()), tempFile.getAbsolutePath());
131+
} catch (IOException e) {
132+
LOG.warn("Failed to create temporary file for in-memory uploaded item: {}",
133+
normalizeSpace(item.getName()), e);
134+
}
118135
} else {
119136
UploadedFile uploadedFile = StrutsUploadedFile.Builder
120137
.create(item.getPath().toFile())
@@ -128,4 +145,33 @@ protected void processFileField(DiskFileItem item) {
128145
uploadedFiles.put(item.getFieldName(), values);
129146
}
130147

148+
/**
149+
* Override cleanUp to ensure all DiskFileItem instances are properly cleaned up
150+
*/
151+
@Override
152+
public void cleanUp() {
153+
super.cleanUp();
154+
try {
155+
LOG.debug("Clean up all DiskFileItem instances (both form fields and file uploads");
156+
for (DiskFileItem item : diskFileItems) {
157+
try {
158+
if (item.isInMemory()) {
159+
LOG.debug("Cleaning up in-memory item: {}", normalizeSpace(item.getFieldName()));
160+
} else {
161+
LOG.debug("Cleaning up disk item: {} at {}", normalizeSpace(item.getFieldName()), item.getPath());
162+
if (item.getPath() != null && item.getPath().toFile().exists()) {
163+
if (!item.getPath().toFile().delete()) {
164+
LOG.warn("There was a problem attempting to delete temporary file: {}", item.getPath());
165+
}
166+
}
167+
}
168+
} catch (Exception e) {
169+
LOG.warn("Error cleaning up DiskFileItem: {}", normalizeSpace(item.getFieldName()), e);
170+
}
171+
}
172+
} finally {
173+
diskFileItems.clear();
174+
}
175+
}
176+
131177
}

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

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
package org.apache.struts2.dispatcher.multipart;
2020

2121
import jakarta.servlet.http.HttpServletRequest;
22-
import org.apache.commons.fileupload2.core.DiskFileItemFactory;
2322
import org.apache.commons.fileupload2.core.FileItemInput;
2423
import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException;
2524
import org.apache.commons.fileupload2.core.FileUploadSizeException;
@@ -45,7 +44,7 @@
4544
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
4645

4746
/**
48-
* Multi-part form data request adapter for Jakarta Commons FileUpload package that
47+
* Multipart form data request adapter for Jakarta Commons FileUpload package that
4948
* leverages the streaming API rather than the traditional non-streaming API.
5049
* <p>
5150
* For more details see WW-3025
@@ -82,22 +81,6 @@ protected void processUpload(HttpServletRequest request, String saveDir) throws
8281
});
8382
}
8483

85-
protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path location) {
86-
DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder();
87-
88-
LOG.debug("Using file save directory: {}", location);
89-
builder.setPath(location);
90-
91-
LOG.debug("Sets buffer size: {}", bufferSize);
92-
builder.setBufferSize(bufferSize);
93-
94-
LOG.debug("Using charset: {}", charset);
95-
builder.setCharset(charset);
96-
97-
DiskFileItemFactory factory = builder.get();
98-
return new JakartaServletDiskFileUpload(factory);
99-
}
100-
10184
private String readStream(InputStream inputStream) throws IOException {
10285
ByteArrayOutputStream result = new ByteArrayOutputStream();
10386
byte[] buffer = new byte[1024];

‎core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsRequestContext.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.struts2.dispatcher.multipart;
20+
21+
import jakarta.servlet.http.HttpServletRequest;
22+
import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletRequestContext;
23+
24+
/**
25+
* Provides a specialized request context for Struts applications,
26+
* extending the Jakarta Servlet request context to add custom handling
27+
* for multipart-related requests.
28+
* <p>
29+
* This class overrides multipart detection logic to ensure that requests
30+
* without a content type are not treated as multipart, improving robustness
31+
* in file upload scenarios.
32+
*/
33+
public class StrutsRequestContext extends JakartaServletRequestContext {
34+
35+
/**
36+
* Constructs a context for this request.
37+
*
38+
* @param request The request to which this context applies.
39+
*/
40+
public StrutsRequestContext(HttpServletRequest request) {
41+
super(request);
42+
}
43+
44+
/**
45+
* Determines if the current request is multipart-related.
46+
* <p>
47+
* This implementation first checks if the request's content type is set.
48+
* If the content type is {@code null}, it returns {@code false} immediately.
49+
* Otherwise, it delegates to the superclass implementation to perform
50+
* further checks.
51+
*
52+
* @return {@code true} if the request is multipart-related; {@code false} otherwise.
53+
*/
54+
@Override
55+
public boolean isMultipartRelated() {
56+
if (this.getRequest().getContentType() == null) {
57+
return false;
58+
}
59+
return super.isMultipartRelated();
60+
}
61+
}

0 commit comments

Comments
 (0)