This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch fix/WW-5501-revert
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 11e791b9f3c3910858726cf6ad20035b8bd3200d
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Sun Feb 16 07:56:12 2025 +0100
Reverts all changes related to WW-5501
---
.../multipart/AbstractMultiPartRequest.java | 76 ++++------------------
.../multipart/JakartaMultiPartRequest.java | 25 ++-----
.../multipart/JakartaStreamMultiPartRequest.java | 23 ++-----
.../org/apache/struts2/struts-messages.properties | 2 -
.../ActionFileUploadInterceptorTest.java | 64 ------------------
.../interceptor/FileUploadInterceptorTest.java | 64 ------------------
.../dispatcher/multipart/PellMultiPartRequest.java | 8 ---
7 files changed, 23 insertions(+), 239 deletions(-)
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
index 88d6e991b..23d879ba4 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java
@@ -20,9 +20,6 @@ package org.apache.struts2.dispatcher.multipart;
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.io.FilenameUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.StrutsConstants;
@@ -33,29 +30,21 @@ 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 =
"^(.*[<>&\"'|;\\\\/?*:]+.*|.*\\.\\..*)$";
- private static final String EXCLUDED_FILE_PATTERN_WITH_DMI_SUPPORT =
"^(?!action:[^<>&\"'|;\\\\/?*:]+(![^<>&\"'|;\\\\/?*:]+)?$)(.*[<>&\"'|;\\\\/?*:]+.*|.*\\.\\..*)$\n";
-
/**
* Defines the internal buffer size used during streaming operations.
*/
public static final int BUFFER_SIZE = 10240;
/**
- * Internal list of raised errors to be passed to the Struts2 framework.
+ * Internal list of raised errors to be passed to the the Struts2
framework.
*/
protected List<LocalizedMessage> errors = new ArrayList<>();
@@ -91,18 +80,6 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
*/
protected Locale defaultLocale = Locale.ENGLISH;
- private final ExcludedPatternsChecker patternsChecker;
-
- 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;
- }
-
/**
* @param bufferSize Sets the buffer size to be used.
*/
@@ -146,7 +123,7 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
/**
* @param request Inspect the servlet request and set the locale if one
wasn't provided by
- * the Struts2 framework.
+ * the Struts2 framework.
*/
protected void setLocale(HttpServletRequest request) {
if (defaultLocale == null) {
@@ -157,7 +134,7 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
/**
* Build error message.
*
- * @param e the Throwable/Exception
+ * @param e the Throwable/Exception
* @param args arguments
* @return error message
*/
@@ -170,7 +147,7 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
/* (non-Javadoc)
* @see
org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors()
- */
+ */
public List<LocalizedMessage> getErrors() {
return errors;
}
@@ -180,43 +157,16 @@ public abstract class AbstractMultiPartRequest implements
MultiPartRequest {
* @return the canonical name based on the supplied filename
*/
protected String getCanonicalName(final String originalFileName) {
- return FilenameUtils.getName(originalFileName);
- }
-
- /**
- * @param fileName file name to check
- * @return true if the file name is excluded
- */
- protected boolean isExcluded(String fileName) {
- 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;
+ 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);
}
-
- 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);
+ return fileName;
}
- 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;
- }
}
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
index be6268380..d55e35001 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
@@ -18,7 +18,6 @@
*/
package org.apache.struts2.dispatcher.multipart;
-import com.opensymphony.xwork2.inject.Inject;
import org.apache.commons.fileupload.FileCountLimitExceededException;
import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.FileUploadBase;
@@ -27,11 +26,9 @@ 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;
-import org.apache.struts2.StrutsConstants;
import org.apache.struts2.dispatcher.LocalizedMessage;
import javax.servlet.http.HttpServletRequest;
@@ -62,15 +59,6 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
// maps parameter name -> List of param values
protected Map<String, List<String>> params = new HashMap<>();
- public JakartaMultiPartRequest() {
- super();
- }
-
- @Inject(value = StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION,
required = false)
- public JakartaMultiPartRequest(String dmiValue) {
- super(BooleanUtils.toBoolean(dmiValue));
- }
-
/**
* Creates a new request wrapper to handle multi-part data using methods
adapted from Jason Pell's
* multipart classes (see class description).
@@ -125,7 +113,11 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
}
protected void processFileField(FileItem item) {
- if (isInvalidInput(item.getFieldName(), item.getName())) {
+ LOG.debug("Item is a file upload");
+
+ // 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;
}
@@ -142,12 +134,7 @@ public class JakartaMultiPartRequest extends
AbstractMultiPartRequest {
protected void processNormalFormField(FileItem item, String charset)
throws UnsupportedEncodingException {
try {
- String fieldName = item.getFieldName();
- LOG.debug("Item: {} is a normal form field",
normalizeSpace(fieldName));
-
- if (isInvalidInput(fieldName)) {
- return;
- }
+ LOG.debug("Item is a normal form field");
List<String> values;
if (params.get(item.getFieldName()) != null) {
diff --git
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
index 655112010..70eeb8e2a 100644
---
a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
+++
b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java
@@ -18,7 +18,6 @@
*/
package org.apache.struts2.dispatcher.multipart;
-import com.opensymphony.xwork2.inject.Inject;
import org.apache.commons.fileupload.FileItemIterator;
import org.apache.commons.fileupload.FileItemStream;
import org.apache.commons.fileupload.FileUploadBase;
@@ -26,10 +25,8 @@ 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;
import org.apache.struts2.dispatcher.LocalizedMessage;
import javax.servlet.http.HttpServletRequest;
@@ -209,15 +206,6 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
}
}
- public JakartaStreamMultiPartRequest() {
- super();
- }
-
- @Inject(value = StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION,
required = false)
- public JakartaStreamMultiPartRequest(String dmiValue) {
- super(BooleanUtils.toBoolean(dmiValue));
- }
-
/**
* Processes the upload.
*
@@ -326,9 +314,6 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
*/
protected void processFileItemStreamAsFormField(FileItemStream itemStream)
{
String fieldName = itemStream.getFieldName();
- if (isInvalidInput(fieldName)) {
- return;
- }
try {
List<String> values;
String fieldValue = Streams.asString(itemStream.openStream());
@@ -351,7 +336,9 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
* @param location location
*/
protected void processFileItemStreamAsFileField(FileItemStream itemStream,
String location) {
- if (isInvalidInput(itemStream.getFieldName(), itemStream.getName())) {
+ // 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;
}
@@ -400,9 +387,7 @@ public class JakartaStreamMultiPartRequest extends
AbstractMultiPartRequest {
}
File file = File.createTempFile(prefix + "_", suffix, new
File(location));
- if (LOG.isDebugEnabled()) {
- LOG.debug("Creating temporary file [{}] (originally [{}]).",
file.getName(), normalizeSpace(fileName));
- }
+ LOG.debug("Creating temporary file [{}] (originally [{}]).",
file.getName(), normalizeSpace(fileName));
return file;
}
diff --git
a/core/src/main/resources/org/apache/struts2/struts-messages.properties
b/core/src/main/resources/org/apache/struts2/struts-messages.properties
index b75a4144e..57b7e2b54 100644
--- a/core/src/main/resources/org/apache/struts2/struts-messages.properties
+++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties
@@ -27,8 +27,6 @@ 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}
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
index 85bf5ab4b..869deeed5 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java
@@ -663,70 +663,6 @@ public class ActionFileUploadInterceptorTest extends
StrutsInternalTestCase {
assertTrue(msg.startsWith("Der Request übertraf die maximal erlaubte
Größe"));
}
- public void testUnacceptedFieldName() throws Exception {
- MockHttpServletRequest req = new MockHttpServletRequest();
- req.setCharacterEncoding(StandardCharsets.UTF_8.name());
- req.setMethod("post");
- req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
-
- // inspired by the unit tests for jakarta commons fileupload
- String content = ("-----1234\r\n" +
- "Content-Disposition: form-data; name=\"top.file\";
filename=\"deleteme.txt\"\r\n" +
- "Content-Type: text/html\r\n" +
- "\r\n" +
- "Unit test of ActionFileUploadInterceptor" +
- "\r\n" +
- "-----1234--\r\n");
- req.setContent(content.getBytes(StandardCharsets.US_ASCII));
-
- MyFileUploadAction action = container.inject(MyFileUploadAction.class);
-
- MockActionInvocation mai = new MockActionInvocation();
- mai.setAction(action);
- mai.setResultCode("success");
- mai.setInvocationContext(ActionContext.getContext());
- ActionContext.getContext()
- .withServletRequest(createMultipartRequestMaxSize(req, 2000));
-
- interceptor.intercept(mai);
-
- assertThat(action.getActionErrors())
- .containsExactly("The multipart upload field name \"top.file\"
contains illegal characters!");
- assertNull(action.getUploadFiles());
- }
-
- public void testUnacceptedFileName() throws Exception {
- MockHttpServletRequest req = new MockHttpServletRequest();
- req.setCharacterEncoding(StandardCharsets.UTF_8.name());
- req.setMethod("post");
- req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
-
- // inspired by the unit tests for jakarta commons fileupload
- String content = ("-----1234\r\n" +
- "Content-Disposition: form-data; name=\"file\";
filename=\"../deleteme.txt\"\r\n" +
- "Content-Type: text/html\r\n" +
- "\r\n" +
- "Unit test of ActionFileUploadInterceptor" +
- "\r\n" +
- "-----1234--\r\n");
- req.setContent(content.getBytes(StandardCharsets.US_ASCII));
-
- MyFileUploadAction action = container.inject(MyFileUploadAction.class);
-
- MockActionInvocation mai = new MockActionInvocation();
- mai.setAction(action);
- mai.setResultCode("success");
- mai.setInvocationContext(ActionContext.getContext());
- ActionContext.getContext()
- .withServletRequest(createMultipartRequestMaxSize(req, 2000));
-
- interceptor.intercept(mai);
-
- assertThat(action.getActionErrors())
- .containsExactly("The multipart upload filename
\"../deleteme.txt\" contains illegal characters!");
- assertNull(action.getUploadFiles());
- }
-
private String encodeTextFile(String filename, String contentType, String
content) {
return "\r\n" +
"--" +
diff --git
a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
index fe4c2ef21..c4b03ad0b 100644
---
a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
+++
b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
@@ -727,70 +727,6 @@ public class FileUploadInterceptorTest extends
StrutsInternalTestCase {
assertTrue(msg.startsWith("Der Request übertraf die maximal erlaubte
Größe"));
}
- public void testUnacceptedFieldName() throws Exception {
- MockHttpServletRequest req = new MockHttpServletRequest();
- req.setCharacterEncoding(StandardCharsets.UTF_8.name());
- req.setMethod("post");
- req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
-
- // inspired by the unit tests for jakarta commons fileupload
- String content = ("-----1234\r\n" +
- "Content-Disposition: form-data; name=\"top.file\";
filename=\"deleteme.txt\"\r\n" +
- "Content-Type: text/html\r\n" +
- "\r\n" +
- "Unit test of ActionFileUploadInterceptor" +
- "\r\n" +
- "-----1234--\r\n");
- req.setContent(content.getBytes(StandardCharsets.US_ASCII));
-
- ActionFileUploadInterceptorTest.MyFileUploadAction action =
container.inject(ActionFileUploadInterceptorTest.MyFileUploadAction.class);
-
- MockActionInvocation mai = new MockActionInvocation();
- mai.setAction(action);
- mai.setResultCode("success");
- mai.setInvocationContext(ActionContext.getContext());
- ActionContext.getContext()
- .withServletRequest(createMultipartRequestMaxSize(req, 2000));
-
- interceptor.intercept(mai);
-
- assertThat(action.getActionErrors())
- .containsExactly("The multipart upload field name \"top.file\"
contains illegal characters!");
- assertNull(action.getUploadFiles());
- }
-
- public void testUnacceptedFileName() throws Exception {
- MockHttpServletRequest req = new MockHttpServletRequest();
- req.setCharacterEncoding(StandardCharsets.UTF_8.name());
- req.setMethod("post");
- req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
-
- // inspired by the unit tests for jakarta commons fileupload
- String content = ("-----1234\r\n" +
- "Content-Disposition: form-data; name=\"file\";
filename=\"../deleteme.txt\"\r\n" +
- "Content-Type: text/html\r\n" +
- "\r\n" +
- "Unit test of ActionFileUploadInterceptor" +
- "\r\n" +
- "-----1234--\r\n");
- req.setContent(content.getBytes(StandardCharsets.US_ASCII));
-
- ActionFileUploadInterceptorTest.MyFileUploadAction action =
container.inject(ActionFileUploadInterceptorTest.MyFileUploadAction.class);
-
- MockActionInvocation mai = new MockActionInvocation();
- mai.setAction(action);
- mai.setResultCode("success");
- mai.setInvocationContext(ActionContext.getContext());
- ActionContext.getContext()
- .withServletRequest(createMultipartRequestMaxSize(req, 2000));
-
- interceptor.intercept(mai);
-
- assertThat(action.getActionErrors())
- .containsExactly("The multipart upload filename
\"../deleteme.txt\" contains illegal characters!");
- assertNull(action.getUploadFiles());
- }
-
private String encodeTextFile(String filename, String contentType, String
content) {
return "\r\n" +
"--" +
diff --git
a/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java
b/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java
index a80625758..ef5019d4b 100644
---
a/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java
+++
b/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java
@@ -18,12 +18,9 @@
*/
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;
import javax.servlet.http.HttpServletRequest;
import java.io.IOException;
@@ -41,11 +38,6 @@ public class PellMultiPartRequest extends
AbstractMultiPartRequest {
private ServletMultipartRequest multi;
- @Inject(value = StrutsConstants.STRUTS_ENABLE_DYNAMIC_METHOD_INVOCATION,
required = false)
- public PellMultiPartRequest(String dmiValue) {
- super(BooleanUtils.toBoolean(dmiValue));
- }
-
/**
* Creates a new request wrapper to handle multi-part data using methods
adapted from Jason Pell's
* multipart classes (see class description).
