You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2017/03/23 07:16:22 UTC

struts git commit: WW-4768 Adds proper validation if request is a multipart request

Repository: struts
Updated Branches:
  refs/heads/master 82f61666f -> 4e9fa8423


WW-4768 Adds proper validation if request is a multipart request


Project: http://git-wip-us.apache.org/repos/asf/struts/repo
Commit: http://git-wip-us.apache.org/repos/asf/struts/commit/4e9fa842
Tree: http://git-wip-us.apache.org/repos/asf/struts/tree/4e9fa842
Diff: http://git-wip-us.apache.org/repos/asf/struts/diff/4e9fa842

Branch: refs/heads/master
Commit: 4e9fa8423931417da8bc60ce220f46935b54c5de
Parents: 82f6166
Author: Lukasz Lenart <lu...@apache.org>
Authored: Thu Mar 23 08:07:21 2017 +0100
Committer: Lukasz Lenart <lu...@apache.org>
Committed: Thu Mar 23 08:07:21 2017 +0100

----------------------------------------------------------------------
 .../org/apache/struts2/StrutsConstants.java     |  2 ++
 .../apache/struts2/dispatcher/Dispatcher.java   | 38 ++++++++++++++++++--
 .../struts2/dispatcher/DispatcherTest.java      |  3 +-
 .../interceptor/FileUploadInterceptorTest.java  |  4 +++
 4 files changed, 44 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/struts/blob/4e9fa842/core/src/main/java/org/apache/struts2/StrutsConstants.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index a868edd..b41f7e6 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -139,6 +139,8 @@ public final class StrutsConstants {
      */
     public static final String STRUTS_MULTIPART_PARSER = "struts.multipart.parser";
 
+    public static final String STRUTS_MULTIPART_VALIDATION_REGEX = "struts.multipart.validationRegex";
+
     /** How Spring should autowire.  Valid values are 'name', 'type', 'auto', and 'constructor' */
     public static final String STRUTS_OBJECTFACTORY_SPRING_AUTOWIRE = "struts.objectFactory.spring.autoWire";
 

http://git-wip-us.apache.org/repos/asf/struts/blob/4e9fa842/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
index b7714be..280e27b 100644
--- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
+++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java
@@ -66,6 +66,7 @@ import java.io.File;
 import java.io.IOException;
 import java.util.*;
 import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.regex.Pattern;
 
 /**
  * A utility class the actual dispatcher delegates most of its tasks to. Each instance
@@ -82,6 +83,13 @@ public class Dispatcher {
     private static final Logger LOG = LogManager.getLogger(Dispatcher.class);
 
     /**
+     * {@link HttpServletRequest#getMethod()}
+     */
+    public static final String REQUEST_POST_METHOD = "POST";
+
+    public static final String MULTIPART_FORM_DATA_REGEX = "^multipart\\/form-data(; boundary=[a-zA-Z0-9]{1,70})?";
+
+    /**
      * Provide a thread local instance.
      */
     private static ThreadLocal<Dispatcher> instance = new ThreadLocal<>();
@@ -122,6 +130,11 @@ public class Dispatcher {
     private String multipartHandlerName;
 
     /**
+     * A regular expression used to validate if request is a multipart/form-data request
+     */
+    private Pattern multipartValidationPattern = Pattern.compile(MULTIPART_FORM_DATA_REGEX);
+
+    /**
      * Provide list of default configuration files.
      */
     private static final String DEFAULT_CONFIGURATION_PATHS = "struts-default.xml,struts-plugin.xml,struts.xml";
@@ -264,6 +277,11 @@ public class Dispatcher {
         multipartHandlerName = val;
     }
 
+    @Inject(value = StrutsConstants.STRUTS_MULTIPART_VALIDATION_REGEX, required = false)
+    public void setMultipartValidationRegex(String multipartValidationRegex) {
+        this.multipartValidationPattern = Pattern.compile(multipartValidationRegex);
+    }
+
     @Inject
     public void setValueStackFactory(ValueStackFactory valueStackFactory) {
         this.valueStackFactory = valueStackFactory;
@@ -781,8 +799,7 @@ public class Dispatcher {
             return request;
         }
 
-        String content_type = request.getContentType();
-        if (content_type != null && content_type.contains("multipart/form-data")) {
+        if (isMultipartRequest(request)) {
             MultiPartRequest multiPartRequest = getMultiPartRequest();
             LocaleProviderFactory localeProviderFactory = getContainer().getInstance(LocaleProviderFactory.class);
 
@@ -801,6 +818,23 @@ public class Dispatcher {
     }
 
     /**
+     * Checks if request is a multipart request (a file upload request)
+     *
+     * @param request current servlet request
+     * @return true if it is a multipart request
+     *
+     * @since 2.5.11
+     */
+    protected boolean isMultipartRequest(HttpServletRequest request) {
+        String httpMethod = request.getMethod();
+        String contentType = request.getContentType();
+
+        return REQUEST_POST_METHOD.equalsIgnoreCase(httpMethod) &&
+                contentType != null &&
+                multipartValidationPattern.matcher(contentType.toLowerCase(Locale.ENGLISH)).matches();
+    }
+
+    /**
      * On each request it must return a new instance as implementation could be not thread safe
      * and thus ensure of resource clean up
      *

http://git-wip-us.apache.org/repos/asf/struts/blob/4e9fa842/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
----------------------------------------------------------------------
diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
index d8f10cc..ffa43e1 100644
--- a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
+++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherTest.java
@@ -138,7 +138,8 @@ public class DispatcherTest extends StrutsInternalTestCase {
         MockHttpServletRequest req = new MockHttpServletRequest();
         MockHttpServletResponse res = new MockHttpServletResponse();
 
-        req.setContentType("multipart/form-data");
+        req.setMethod("post");
+        req.setContentType("multipart/form-data; boundary=asdcvb345asd");
         Dispatcher du = initDispatcher(Collections.<String, String>emptyMap());
         du.prepare(req, res);
         HttpServletRequest wrapped = du.wrapRequest(req);

http://git-wip-us.apache.org/repos/asf/struts/blob/4e9fa842/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
----------------------------------------------------------------------
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 8e68cef..a3e3143 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
@@ -229,6 +229,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
 
         req.setCharacterEncoding("text/html");
         req.setContentType("text/xml"); // not a multipart contentype
+        req.setMethod("post");
         req.addHeader("Content-type", "multipart/form-data");
 
         MyFileupAction action = container.inject(MyFileupAction.class);
@@ -249,6 +250,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
         MockHttpServletRequest req = new MockHttpServletRequest();
 
         req.setCharacterEncoding("text/html");
+        req.setMethod("post");
         req.addHeader("Content-type", "multipart/form-data");
         req.setContent(null); // there is no content
 
@@ -269,6 +271,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
     public void testSuccessUploadOfATextFileMultipartRequest() throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest();
         req.setCharacterEncoding("text/html");
+        req.setMethod("post");
         req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
 
         // inspired by the unit tests for jakarta commons fileupload
@@ -374,6 +377,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
     public void testMultipartRequestLocalizedError() throws Exception {
         MockHttpServletRequest req = new MockHttpServletRequest();
         req.setCharacterEncoding("text/html");
+        req.setMethod("post");
         req.addHeader("Content-type", "multipart/form-data; boundary=---1234");
 
         // inspired by the unit tests for jakarta commons fileupload