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

Merge pull request #1215 from apache/WW-5528-multipart-illegal-char-errors-67

Browse Source
This commit is contained in:
Kusal Kithul-Godage
2025-02-09 23:44:10 +11:00
committed by GitHub
parent 7a77c7a82c fe3dd6ba9f
commit 58f37babbf
7 changed files with 68 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+43 -18
View File
@@ -22,7 +22,7 @@ import com.opensymphony.xwork2.LocaleProviderFactory;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker;
import com.opensymphony.xwork2.security.ExcludedPatternsChecker;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.io.FilenameUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
@@ -33,12 +33,17 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
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}
*/
public abstract class AbstractMultiPartRequest implements MultiPartRequest {
protected static final String STRUTS_MESSAGES_UPLOAD_ERROR_ILLEGAL_CHARACTERS_FIELD = "struts.messages.upload.error.illegal.characters.field";
protected static final String STRUTS_MESSAGES_UPLOAD_ERROR_ILLEGAL_CHARACTERS_NAME = "struts.messages.upload.error.illegal.characters.name";
private static final Logger LOG = LogManager.getLogger(AbstractMultiPartRequest.class);
private static final String EXCLUDED_FILE_PATTERN = "^(.*[<>&\"'|;\\\\/?*:]+.*|.*\\.\\..*)$";
@@ -88,13 +93,14 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
private final ExcludedPatternsChecker patternsChecker;
protected AbstractMultiPartRequest(String dmiValue) {
patternsChecker = new DefaultExcludedPatternsChecker();
if (BooleanUtils.toBoolean(dmiValue)) {
((DefaultExcludedPatternsChecker) patternsChecker).setAdditionalExcludePatterns(EXCLUDED_FILE_PATTERN_WITH_DMI_SUPPORT);
} else {
((DefaultExcludedPatternsChecker) patternsChecker).setAdditionalExcludePatterns(EXCLUDED_FILE_PATTERN);
}
protected AbstractMultiPartRequest() {
this(false);
}
protected AbstractMultiPartRequest(boolean dmiValue) {
DefaultExcludedPatternsChecker patternsChecker = new DefaultExcludedPatternsChecker();
patternsChecker.setAdditionalExcludePatterns(dmiValue ? EXCLUDED_FILE_PATTERN_WITH_DMI_SUPPORT : EXCLUDED_FILE_PATTERN);
this.patternsChecker = patternsChecker;
}
/**
@@ -174,16 +180,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
* @return the canonical name based on the supplied filename
*/
protected String getCanonicalName(final String originalFileName) {
String fileName = originalFileName;
int forwardSlash = fileName.lastIndexOf('/');
int backwardSlash = fileName.lastIndexOf('\\');
if (forwardSlash != -1 && forwardSlash > backwardSlash) {
fileName = fileName.substring(forwardSlash + 1);
} else {
fileName = fileName.substring(backwardSlash + 1);
}
return fileName;
return FilenameUtils.getName(originalFileName);
}
/**
@@ -194,4 +191,32 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
return patternsChecker.isExcluded(fileName).isExcluded();
}
protected boolean isInvalidInput(String fieldName, String fileName) {
// Skip file uploads that don't have a file name - meaning that no file was selected.
if (fileName == null || fileName.trim().isEmpty()) {
LOG.debug(() -> "No file has been uploaded for the field: " + normalizeSpace(fieldName));
return true;
}
if (isExcluded(fileName)) {
String normalizedFileName = normalizeSpace(fileName);
LOG.debug("File name [{}] is not accepted", normalizedFileName);
errors.add(new LocalizedMessage(getClass(), STRUTS_MESSAGES_UPLOAD_ERROR_ILLEGAL_CHARACTERS_NAME, null,
new String[]{normalizedFileName}));
return true;
}
return isInvalidInput(fieldName);
}
protected boolean isInvalidInput(String fieldName) {
if (isExcluded(fieldName)) {
String normalizedFieldName = normalizeSpace(fieldName);
LOG.debug("Form field [{}] is rejected!", normalizedFieldName);
errors.add(new LocalizedMessage(getClass(), STRUTS_MESSAGES_UPLOAD_ERROR_ILLEGAL_CHARACTERS_FIELD, null,
new String[]{normalizedFieldName}));
return true;
}
return false;
}
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+8 -20
View File
@@ -27,6 +27,7 @@ import org.apache.commons.fileupload.RequestContext;
import org.apache.commons.fileupload.disk.DiskFileItem;
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -60,13 +61,14 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
// maps parameter name -> List of param values
protected Map<String, List<String>> params = new HashMap<>();
public JakartaMultiPartRequest() {
super(Boolean.FALSE.toString());
super();
}
@Inject(value = StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, required = false)
public JakartaMultiPartRequest(String dmiValue) {
super(dmiValue);
super(BooleanUtils.toBoolean(dmiValue));
}
/**
@@ -123,21 +125,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
}
protected void processFileField(FileItem item) {
LOG.debug("Item is a file upload");
if (isExcluded(item.getName())) {
LOG.warn("File name [{}] is not accepted", normalizeSpace(item.getName()));
return;
}
if (isExcluded(item.getFieldName())) {
LOG.warn("Field name [{}] is not accepted", normalizeSpace(item.getFieldName()));
return;
}
// 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()));
if (isInvalidInput(item.getFieldName(), item.getName())) {
return;
}
@@ -154,10 +142,10 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
protected void processNormalFormField(FileItem item, String charset) throws UnsupportedEncodingException {
try {
LOG.debug("Item is a normal form field");
String fieldName = item.getFieldName();
LOG.debug("Item: {} is a normal form field", normalizeSpace(fieldName));
if (isExcluded(item.getFieldName())) {
LOG.warn("Form field name [{}] is not accepted", normalizeSpace(item.getFieldName()));
if (isInvalidInput(fieldName)) {
return;
}
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
+5 -12
View File
@@ -26,6 +26,7 @@ import org.apache.commons.fileupload.FileUploadBase.FileSizeLimitExceededExcepti
import org.apache.commons.fileupload.FileUploadException;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.commons.fileupload.util.Streams;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
@@ -209,12 +210,12 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
}
public JakartaStreamMultiPartRequest() {
super(Boolean.FALSE.toString());
super();
}
@Inject(value = StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, required = false)
public JakartaStreamMultiPartRequest(String dmiValue) {
super(dmiValue);
super(BooleanUtils.toBoolean(dmiValue));
}
/**
@@ -325,8 +326,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
*/
protected void processFileItemStreamAsFormField(FileItemStream itemStream) {
String fieldName = itemStream.getFieldName();
if (isExcluded(fieldName)) {
LOG.warn("Form field [{}] rejected!", normalizeSpace(fieldName));
if (isInvalidInput(fieldName)) {
return;
}
try {
@@ -351,14 +351,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
* @param location location
*/
protected void processFileItemStreamAsFileField(FileItemStream itemStream, String location) {
// Skip file uploads that don't have a file name - meaning that no file was selected.
if (itemStream.getName() == null || itemStream.getName().trim().isEmpty()) {
LOG.debug("No file has been uploaded for the field: {}", normalizeSpace(itemStream.getFieldName()));
return;
}
if (isExcluded(itemStream.getName())) {
LOG.warn("File field [{}] rejected", normalizeSpace(itemStream.getName()));
if (isInvalidInput(itemStream.getFieldName(), itemStream.getName())) {
return;
}
core/src/main/resources/org/apache/struts2/struts-messages.properties
+2
View File
@@ -27,6 +27,8 @@ struts.messages.removing.file=Removing file {0} {1}
struts.messages.error.uploading=Error uploading: {0}
struts.messages.error.file.too.large=File {0} is too large to be uploaded. Maximum allowed size is {4} bytes!
struts.messages.upload.error.parameter.too.long=The request parameter "{0}" was too long. Max length allowed is {1}, but found {2}!
struts.messages.upload.error.illegal.characters.field=The multipart upload field name "{0}" contains illegal characters!
struts.messages.upload.error.illegal.characters.name=The multipart upload filename "{0}" contains illegal characters!
struts.messages.error.content.type.not.allowed=Content-Type not allowed: {0} "{1}" "{2}" {3}
struts.messages.error.file.extension.not.allowed=File extension not allowed: {0} "{1}" "{2}" {3}
core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+4 -5
View File
@@ -24,9 +24,6 @@ import com.opensymphony.xwork2.DefaultLocaleProvider;
import com.opensymphony.xwork2.ValidationAwareSupport;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import com.opensymphony.xwork2.mock.MockActionProxy;
import com.opensymphony.xwork2.security.DefaultAcceptedPatternsChecker;
import com.opensymphony.xwork2.security.DefaultExcludedPatternsChecker;
import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.util.ClassLoaderUtil;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.struts2.ServletActionContext;
@@ -693,7 +690,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.intercept(mai);
assertFalse(action.hasActionErrors());
assertThat(action.getActionErrors())
.containsExactly("The multipart upload field name \"top.file\" contains illegal characters!");
assertNull(action.getUploadFiles());
}
@@ -724,7 +722,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.intercept(mai);
assertFalse(action.hasActionErrors());
assertThat(action.getActionErrors())
.containsExactly("The multipart upload filename \"../deleteme.txt\" contains illegal characters!");
assertNull(action.getUploadFiles());
}
core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
+4 -3
View File
@@ -23,7 +23,6 @@ import com.opensymphony.xwork2.ActionSupport;
import com.opensymphony.xwork2.DefaultLocaleProvider;
import com.opensymphony.xwork2.ValidationAwareSupport;
import com.opensymphony.xwork2.mock.MockActionInvocation;
import com.opensymphony.xwork2.security.DefaultNotExcludedAcceptedPatternsChecker;
import com.opensymphony.xwork2.util.ClassLoaderUtil;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.apache.struts2.ServletActionContext;
@@ -755,7 +754,8 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.intercept(mai);
assertFalse(action.hasActionErrors());
assertThat(action.getActionErrors())
.containsExactly("The multipart upload field name \"top.file\" contains illegal characters!");
assertNull(action.getUploadFiles());
}
@@ -786,7 +786,8 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
interceptor.intercept(mai);
assertFalse(action.hasActionErrors());
assertThat(action.getActionErrors())
.containsExactly("The multipart upload filename \"../deleteme.txt\" contains illegal characters!");
assertNull(action.getUploadFiles());
}
plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java
+2 -1
View File
@@ -20,6 +20,7 @@ package org.apache.struts2.dispatcher.multipart;
import com.opensymphony.xwork2.inject.Inject;
import http.utils.multipartrequest.ServletMultipartRequest;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
@@ -42,7 +43,7 @@ public class PellMultiPartRequest extends AbstractMultiPartRequest {
@Inject(value = StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION, required = false)
public PellMultiPartRequest(String dmiValue) {
super(dmiValue);
super(BooleanUtils.toBoolean(dmiValue));
}
/**