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

Uses a dedicated RequestContext to avoid NPE

Browse Source
This commit is contained in:
Lukasz Lenart
2025-07-16 18:34:35 +02:00
parent b796bab432
commit a1c4cb60a2
4 changed files with 154 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+26 -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;
@@ -43,6 +45,8 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.apache.commons.lang3.StringUtils.normalizeSpace;
/**
* Abstract class with some helper methods, it should be used
* when starting development of another implementation of {@link MultiPartRequest}
@@ -187,7 +191,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 +225,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 +256,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 +279,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);
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+66 -20
View File
@@ -20,12 +20,13 @@ 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 java.io.File;
import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.file.Path;
@@ -41,6 +42,11 @@ 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<>();
@Override
protected void processUpload(HttpServletRequest request, String saveDir) throws IOException {
Charset charset = readCharsetEncoding(request);
@@ -48,7 +54,12 @@ 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
diskFileItems.add(item);
LOG.debug(() -> "Processing a form field: " + normalizeSpace(item.getFieldName()));
if (item.isFormField()) {
processNormalFormField(item, charset);
@@ -59,23 +70,6 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
}
}
@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);
}
protected void processNormalFormField(DiskFileItem item, Charset charset) throws IOException {
LOG.debug("Item: {} is a normal form field", normalizeSpace(item.getName()));
@@ -114,7 +108,30 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
}
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 = File.createTempFile("struts_upload_", "_" + item.getName());
tempFile.deleteOnExit(); // Ensure cleanup on JVM exit
// 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);
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);
}
} else {
UploadedFile uploadedFile = StrutsUploadedFile.Builder
.create(item.getPath().toFile())
@@ -128,4 +145,33 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
uploadedFiles.put(item.getFieldName(), values);
}
/**
* Override cleanUp to ensure all DiskFileItem instances are properly cleaned up
*/
@Override
public void cleanUp() {
super.cleanUp();
try {
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 {
LOG.debug("Cleaning up disk item: {} at {}", normalizeSpace(item.getFieldName()), item.getPath());
if (item.getPath() != null && item.getPath().toFile().exists()) {
if (!item.getPath().toFile().delete()) {
LOG.warn("There was a problem attempting to delete temporary file: {}", item.getPath());
}
}
}
} catch (Exception e) {
LOG.warn("Error cleaning up DiskFileItem: {}", normalizeSpace(item.getFieldName()), e);
}
}
} finally {
diskFileItems.clear();
}
}
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
+1 -18
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;
@@ -45,7 +44,7 @@ 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,22 +81,6 @@ 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);
}
private String readStream(InputStream inputStream) throws IOException {
ByteArrayOutputStream result = new ByteArrayOutputStream();
byte[] buffer = new byte[1024];
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();
}
}