WW-5501 Reverts all changes related to WW-5501

.github/workflows/codeql.yml

@@ -17,7 +17,8 @@ name: "CodeQL"
 
 on:
   push:
-    branches: [ "master" ]
+    branches:
+      - 'release/*'
   pull_request:
 
 permissions:
@@ -41,15 +42,21 @@ jobs:
       matrix:
         language: [ 'java' ]
     steps:
-    - name: Checkout repository
-      uses: actions/checkout@v4
-    - name: Initialize CodeQL
-      uses: github/codeql-action/init@v3.27.1
-      with:
-        languages: ${{ matrix.language }}
-    - name: Autobuild
-      uses: github/codeql-action/autobuild@v3.27.1
-    - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v3.27.1
-      with:
-        category: "/language:${{matrix.language}}"
+      - name: Checkout repository
+        uses: actions/checkout@v4
+      - name: Setup Java JDK
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: 17
+          cache: 'maven'
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v3.28.8
+        with:
+          languages: ${{ matrix.language }}
+      - name: Autobuild
+        uses: github/codeql-action/autobuild@v3.28.8
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v3.28.8
+        with:
+          category: "/language:${{matrix.language}}"

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

@@ -20,8 +20,6 @@
 
 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;
@@ -33,29 +31,21 @@
 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 +81,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 +124,7 @@ public void setLocaleProviderFactory(LocaleProviderFactory localeProviderFactory
 
     /**
      * @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 +135,7 @@ protected void setLocale(HttpServletRequest request) {
     /**
      * Build error message.
      *
-     * @param e    the Throwable/Exception
+     * @param e the Throwable/Exception
      * @param args arguments
      * @return error message
      */
@@ -170,7 +148,7 @@ protected LocalizedMessage buildErrorMessage(Throwable e, Object[] args) {
 
     /* (non-Javadoc)
      * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors()
-     */
+    */
     public List<LocalizedMessage> getErrors() {
         return errors;
     }
@@ -183,40 +161,4 @@ 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;
-        }
-
-        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

@@ -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.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 @@ protected void processUpload(HttpServletRequest request, String saveDir) throws
     }
 
     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 @@ protected void processFileField(FileItem item) {
 
     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) {

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

@@ -18,18 +18,15 @@
  */
 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;
 import org.apache.commons.fileupload.FileUploadBase.FileSizeLimitExceededException;
 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 void parse(HttpServletRequest request, String saveDir) throws IOException
         }
     }
 
-    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 @@ protected void addFileSkippedError(String fileName, HttpServletRequest request)
      */
     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 @@ protected void processFileItemStreamAsFormField(FileItemStream itemStream) {
      * @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 @@ protected File createTemporaryFile(String fileName, String location) throws IOEx
         }
 
         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;
     }
 

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}
 

core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java

@@ -663,70 +663,6 @@ public void testMultipartRequestLocalizedError() throws Exception {
         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" +
             "--" +