Browse Source

fix #2842 Add http.fileupload.max.file.count property and refactor file upload handling logic

Shinsuke Sugaya 10 months ago
parent
commit
9ac9f96948

+ 27 - 0
src/main/java/org/codelibs/fess/mylasta/direction/FessConfig.java

@@ -310,6 +310,9 @@ public interface FessConfig extends FessEnv, org.codelibs.fess.mylasta.direction
     /** The key of the configuration. e.g. 262144 */
     String HTTP_FILEUPLOAD_THRESHOLD_SIZE = "http.fileupload.threshold.size";
 
+    /** The key of the configuration. e.g. 10 */
+    String HTTP_FILEUPLOAD_MAX_FILE_COUNT = "http.fileupload.max.file.count";
+
     /** The key of the configuration. e.g. groovy */
     String CRAWLER_DEFAULT_SCRIPT = "crawler.default.script";
 
@@ -2646,6 +2649,21 @@ public interface FessConfig extends FessEnv, org.codelibs.fess.mylasta.direction
      */
     Integer getHttpFileuploadThresholdSizeAsInteger();
 
+    /**
+     * Get the value for the key 'http.fileupload.max.file.count'. <br>
+     * The value is, e.g. 10 <br>
+     * @return The value of found property. (NotNull: if not found, exception but basically no way)
+     */
+    String getHttpFileuploadMaxFileCount();
+
+    /**
+     * Get the value for the key 'http.fileupload.max.file.count' as {@link Integer}. <br>
+     * The value is, e.g. 10 <br>
+     * @return The value of found property. (NotNull: if not found, exception but basically no way)
+     * @throws NumberFormatException When the property is not integer.
+     */
+    Integer getHttpFileuploadMaxFileCountAsInteger();
+
     /**
      * Get the value for the key 'crawler.default.script'. <br>
      * The value is, e.g. groovy <br>
@@ -8221,6 +8239,14 @@ public interface FessConfig extends FessEnv, org.codelibs.fess.mylasta.direction
             return getAsInteger(FessConfig.HTTP_FILEUPLOAD_THRESHOLD_SIZE);
         }
 
+        public String getHttpFileuploadMaxFileCount() {
+            return get(FessConfig.HTTP_FILEUPLOAD_MAX_FILE_COUNT);
+        }
+
+        public Integer getHttpFileuploadMaxFileCountAsInteger() {
+            return getAsInteger(FessConfig.HTTP_FILEUPLOAD_MAX_FILE_COUNT);
+        }
+
         public String getCrawlerDefaultScript() {
             return get(FessConfig.CRAWLER_DEFAULT_SCRIPT);
         }
@@ -11066,6 +11092,7 @@ public interface FessConfig extends FessEnv, org.codelibs.fess.mylasta.direction
             defaultMap.put(FessConfig.HTTP_PROXY_PASSWORD, "");
             defaultMap.put(FessConfig.HTTP_FILEUPLOAD_MAX_SIZE, "262144000");
             defaultMap.put(FessConfig.HTTP_FILEUPLOAD_THRESHOLD_SIZE, "262144");
+            defaultMap.put(FessConfig.HTTP_FILEUPLOAD_MAX_FILE_COUNT, "10");
             defaultMap.put(FessConfig.CRAWLER_DEFAULT_SCRIPT, "groovy");
             defaultMap.put(FessConfig.CRAWLER_HTTP_thread_pool_SIZE, "0");
             defaultMap.put(FessConfig.CRAWLER_DOCUMENT_MAX_SITE_LENGTH, "100");

+ 138 - 108
src/main/java/org/codelibs/fess/mylasta/direction/sponsor/FessMultipartRequestHandler.java

@@ -19,10 +19,11 @@ import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.Serializable;
-import java.util.Hashtable;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
 
@@ -36,9 +37,7 @@ import org.apache.logging.log4j.Logger;
 import org.codelibs.fess.util.ComponentUtil;
 import org.dbflute.helper.message.ExceptionMessageBuilder;
 import org.lastaflute.core.message.UserMessages;
-import org.lastaflute.web.LastaWebKey;
 import org.lastaflute.web.exception.Forced404NotFoundException;
-import org.lastaflute.web.ruts.config.ModuleConfig;
 import org.lastaflute.web.ruts.multipart.MultipartFormFile;
 import org.lastaflute.web.ruts.multipart.MultipartRequestHandler;
 import org.lastaflute.web.ruts.multipart.MultipartRequestWrapper;
@@ -46,6 +45,9 @@ import org.lastaflute.web.ruts.multipart.exception.MultipartExceededException;
 import org.lastaflute.web.util.LaServletContextUtil;
 
 /**
+ * The handler of multipart request (fileupload request). <br>
+ * This instance is created per one multipart request.
+ *
  * @author modified by jflute (originated in Seasar)
  */
 public class FessMultipartRequestHandler implements MultipartRequestHandler {
@@ -54,53 +56,92 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
     //                                                                          Definition
     //                                                                          ==========
     private static final Logger logger = LogManager.getLogger(FessMultipartRequestHandler.class);
-    protected static final String CONTEXT_TEMPDIR_KEY = "javax.servlet.context.tempdir";
-    protected static final String JAVA_IO_TMPDIR_KEY = "java.io.tmpdir";
+
+    // -----------------------------------------------------
+    //                                   Temporary Directory
+    //                                   -------------------
+    // used as repository for requested parameters
+    protected static final String CONTEXT_TEMPDIR_KEY = "javax.servlet.context.tempdir"; // prior
+    protected static final String JAVA_IO_TMPDIR_KEY = "java.io.tmpdir"; // secondary
 
     // ===================================================================================
     //                                                                           Attribute
     //                                                                           =========
-    protected Map<String, Object> elementsAll;
-    protected Map<String, MultipartFormFile> elementsFile;
-    protected Map<String, String[]> elementsText;
+    // keeping parsed request parameters, normal texts or uploaded files
+    // keys are requested parameter names (treated as field name here)
+    protected Map<String, Object> elementsAll; // lazy-loaded, then after not null
+    protected Map<String, MultipartFormFile> elementsFile; // me too
+    protected Map<String, String[]> elementsText; // me too
 
     // ===================================================================================
     //                                                                      Handle Request
     //                                                                      ==============
     @Override
     public void handleRequest(final HttpServletRequest request) throws ServletException {
-        // /- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-        // copied from super's method and extends it
-        // basically for JVN#14876762
-        // thought not all problems are resolved however the main case is safety
-        // - - - - - - - - - -/
         final ServletFileUpload upload = createServletFileUpload(request);
         prepareElementsHash();
         try {
             final List<FileItem> items = parseRequest(request, upload);
             mappingParameter(request, items);
-        } catch (final SizeLimitExceededException e) {
+        } catch (final SizeLimitExceededException e) { // special handling
             handleSizeLimitExceededException(request, e);
-        } catch (final FileUploadException e) {
+        } catch (final FileUploadException e) { // contains fileCount exceeded
             handleFileUploadException(e);
         }
     }
 
-    protected ModuleConfig getModuleConfig(final HttpServletRequest request) {
-        return (ModuleConfig) request.getAttribute(LastaWebKey.MODULE_CONFIG_KEY);
+    protected void prepareElementsHash() { // traditional name
+        // #thinking jflute might lazy-loaded be unneeded? because created per request
+        // (2024/09/08)
+        elementsAll = new HashMap<>();
+        elementsText = new HashMap<>();
+        elementsFile = new HashMap<>();
+    }
+
+    protected List<FileItem> parseRequest(final HttpServletRequest request, final ServletFileUpload upload) throws FileUploadException {
+        return upload.parseRequest(request);
     }
 
     // ===================================================================================
-    //                                                            Create ServletFileUpload
-    //                                                            ========================
+    //                                                                   ServletFileUpload
+    //                                                                   =================
     protected ServletFileUpload createServletFileUpload(final HttpServletRequest request) {
         final DiskFileItemFactory fileItemFactory = createDiskFileItemFactory();
         final ServletFileUpload upload = newServletFileUpload(fileItemFactory);
-        upload.setHeaderEncoding(request.getCharacterEncoding());
-        upload.setSizeMax(getSizeMax());
+        setupServletFileUpload(upload, request);
         return upload;
     }
 
+    // -----------------------------------------------------
+    //                          DiskFileItemFactory Settings
+    //                          ----------------------------
+    protected DiskFileItemFactory createDiskFileItemFactory() {
+        final int sizeThreshold = getSizeThreshold();
+        final File repository = createRepositoryFile();
+        return new DiskFileItemFactory(sizeThreshold, repository);
+    }
+
+    protected int getSizeThreshold() {
+        return ComponentUtil.getFessConfig().getHttpFileuploadThresholdSizeAsInteger().intValue();
+    }
+
+    protected File createRepositoryFile() {
+        return new File(getRepositoryPath());
+    }
+
+    protected String getRepositoryPath() {
+        final ServletContext servletContext = LaServletContextUtil.getServletContext();
+        final File tempDirFile = (File) servletContext.getAttribute(CONTEXT_TEMPDIR_KEY);
+        String tempDir = tempDirFile.getAbsolutePath();
+        if (tempDir == null || tempDir.length() == 0) {
+            tempDir = System.getProperty(JAVA_IO_TMPDIR_KEY);
+        }
+        return tempDir; // must be not null
+    }
+
+    // -----------------------------------------------------
+    //                            ServletFileUpload Settings
+    //                            --------------------------
     protected ServletFileUpload newServletFileUpload(final DiskFileItemFactory fileItemFactory) {
         return new ServletFileUpload(fileItemFactory) {
             @Override
@@ -112,6 +153,10 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
         };
     }
 
+    // #for_now jflute to suppress CVE-2014-0050 even if commons-fileupload is older
+    // than safety version (2024/09/08)
+    // but if you use safety version, this extension is basically unneeded (or you
+    // can use it as double check)
     protected void checkBoundarySize(final String contentType, final byte[] boundary) {
         final int boundarySize = boundary.length;
         final int limitSize = getBoundaryLimitSize();
@@ -130,12 +175,12 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
         final ExceptionMessageBuilder br = new ExceptionMessageBuilder();
         br.addNotice("Too long boundary size so treats it as 404.");
         br.addItem("Advice");
-        br.addElement("Against for JVN14876762.");
+        br.addElement("Against for CVE-2014-0050 (JVN14876762).");
         br.addElement("Boundary size is limited by Framework.");
         br.addElement("Too long boundary is treated as 404 because it's thought of as attack.");
         br.addElement("");
         br.addElement("While, you can override the boundary limit size");
-        br.addElement(" in " + FessMultipartRequestHandler.class.getSimpleName() + ".");
+        br.addElement(" in " + getClass().getSimpleName() + ".");
         br.addItem("Content Type");
         br.addElement(contentType);
         br.addItem("Boundary Size");
@@ -143,31 +188,27 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
         br.addItem("Limit Size");
         br.addElement(limitSize);
         final String msg = br.buildExceptionMessage();
-        throw new Forced404NotFoundException(msg, UserMessages.empty()); // heavy attack!? so give no page to tell wasted action
-    }
-
-    protected DiskFileItemFactory createDiskFileItemFactory() {
-        final File repository = createRepositoryFile();
-        return new DiskFileItemFactory((int) getSizeThreshold(), repository);
+        throw new Forced404NotFoundException(msg, UserMessages.empty()); // heavy attack!? so give no page to tell
+                                                                         // wasted action
     }
 
-    protected File createRepositoryFile() {
-        return new File(getRepositoryPath());
+    protected void setupServletFileUpload(final ServletFileUpload upload, final HttpServletRequest request) {
+        upload.setHeaderEncoding(request.getCharacterEncoding());
+        upload.setSizeMax(getSizeMax());
+        upload.setFileCountMax(getFileCountMax()); // since commons-fileupload-1.5
     }
 
-    // ===================================================================================
-    //                                                                      Handling Parts
-    //                                                                      ==============
-    protected void prepareElementsHash() {
-        elementsText = new Hashtable<>();
-        elementsFile = new Hashtable<>();
-        elementsAll = new Hashtable<>();
+    protected long getSizeMax() {
+        return ComponentUtil.getFessConfig().getHttpFileuploadMaxSizeAsInteger().longValue();
     }
 
-    protected List<FileItem> parseRequest(final HttpServletRequest request, final ServletFileUpload upload) throws FileUploadException {
-        return upload.parseRequest(request);
+    protected long getFileCountMax() {
+        return ComponentUtil.getFessConfig().getHttpFileuploadMaxFileCountAsInteger().longValue();
     }
 
+    // ===================================================================================
+    //                                                                   Parameter Mapping
+    //                                                                   =================
     protected void mappingParameter(final HttpServletRequest request, final List<FileItem> items) {
         showFieldLoggingTitle();
         for (final FileItem item : items) {
@@ -184,8 +225,11 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
         }
     }
 
+    // -----------------------------------------------------
+    //                                     Parameter Logging
+    //                                     -----------------
+    // logging filter cannot show the parameters when multi-part so logging here
     protected void showFieldLoggingTitle() {
-        // logging filter cannot show the parameters when multi-part so logging here
         if (logger.isDebugEnabled()) {
             logger.debug("[Multipart Request Parameter]");
         }
@@ -203,45 +247,11 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
         }
     }
 
-    protected void handleSizeLimitExceededException(final HttpServletRequest request, final SizeLimitExceededException e) {
-        final long actual = e.getActualSize();
-        final long permitted = e.getPermittedSize();
-        final String msg = "Exceeded size of the multipart request: actual=" + actual + " permitted=" + permitted;
-        request.setAttribute(MAX_LENGTH_EXCEEDED_KEY, new MultipartExceededException(msg, actual, permitted, e));
-        try {
-            final InputStream is = request.getInputStream();
-            try {
-                final byte[] buf = new byte[1024];
-                while ((is.read(buf)) != -1) {}
-            } catch (final Exception ignored) {} finally {
-                try {
-                    is.close();
-                } catch (final Exception ignored) {}
-            }
-        } catch (final Exception ignored) {}
-    }
-
-    protected void handleFileUploadException(final FileUploadException e) throws ServletException {
-        // suppress logging because it can be caught by logging filter
-        //log.error("Failed to parse multipart request", e);
-        throw new ServletException("Failed to upload the file.", e);
-    }
-
-    // ===================================================================================
-    //                                                                           Roll-back
-    //                                                                           =========
-    @Override
-    public void rollback() {
-        for (final MultipartFormFile formFile : elementsFile.values()) {
-            formFile.destroy();
-        }
-    }
-
     // ===================================================================================
-    //                                                                            Add Text
-    //                                                                            ========
+    //                                                                       Add Parameter
+    //                                                                       =============
     protected void addTextParameter(final HttpServletRequest request, final FileItem item) {
-        final String name = item.getFieldName();
+        final String fieldName = item.getFieldName();
         final String encoding = request.getCharacterEncoding();
         String value = null;
         boolean haveValue = false;
@@ -260,9 +270,9 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
             haveValue = true;
         }
         if (request instanceof final MultipartRequestWrapper wrapper) {
-            wrapper.setParameter(name, value);
+            wrapper.setParameter(fieldName, value);
         }
-        final String[] oldArray = elementsText.get(name);
+        final String[] oldArray = elementsText.get(fieldName);
         final String[] newArray;
         if (oldArray != null) {
             newArray = new String[oldArray.length + 1];
@@ -271,14 +281,15 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
         } else {
             newArray = new String[] { value };
         }
-        elementsText.put(name, newArray);
-        elementsAll.put(name, newArray);
+        elementsAll.put(fieldName, newArray);
+        elementsText.put(fieldName, newArray);
     }
 
     protected void addFileParameter(final FileItem item) {
+        final String fieldName = item.getFieldName();
         final MultipartFormFile formFile = newActionMultipartFormFile(item);
-        elementsFile.put(item.getFieldName(), formFile);
-        elementsAll.put(item.getFieldName(), formFile);
+        elementsAll.put(fieldName, formFile);
+        elementsFile.put(fieldName, formFile);
     }
 
     protected ActionMultipartFormFile newActionMultipartFormFile(final FileItem item) {
@@ -286,31 +297,50 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
     }
 
     // ===================================================================================
-    //                                                                              Finish
-    //                                                                              ======
-    @Override
-    public void finish() {
-        rollback();
+    //                                                                  Exception Handling
+    //                                                                  ==================
+    protected void handleSizeLimitExceededException(final HttpServletRequest request, final SizeLimitExceededException e) {
+        final long actual = e.getActualSize();
+        final long permitted = e.getPermittedSize();
+        final String msg = "Exceeded size of the multipart request: actual=" + actual + " permitted=" + permitted;
+        request.setAttribute(MAX_LENGTH_EXCEEDED_KEY, new MultipartExceededException(msg, actual, permitted, e));
+        try {
+            final InputStream is = request.getInputStream();
+            try {
+                final byte[] buf = new byte[1024];
+                @SuppressWarnings("unused")
+                int len = 0;
+                while ((len = is.read(buf)) != -1) {}
+            } catch (final Exception ignored) {} finally {
+                try {
+                    is.close();
+                } catch (final Exception ignored) {}
+            }
+        } catch (final Exception ignored) {}
     }
 
-    // ===================================================================================
-    //                                                                        Small Helper
-    //                                                                        ============
-    protected long getSizeMax() {
-        return ComponentUtil.getFessConfig().getHttpFileuploadMaxSizeAsInteger();
+    protected void handleFileUploadException(final FileUploadException e) throws ServletException {
+        // suppress logging because it can be caught by logging filter
+        // log.error("Failed to parse multipart request", e);
+        throw new ServletException("Failed to upload the file.", e);
     }
 
-    protected long getSizeThreshold() {
-        return ComponentUtil.getFessConfig().getHttpFileuploadThresholdSizeAsInteger();
+    // ===================================================================================
+    //                                                                           Roll-back
+    //                                                                           =========
+    @Override
+    public void rollback() {
+        for (final MultipartFormFile formFile : elementsFile.values()) {
+            formFile.destroy();
+        }
     }
 
-    protected String getRepositoryPath() {
-        final File tempDirFile = (File) LaServletContextUtil.getServletContext().getAttribute(CONTEXT_TEMPDIR_KEY);
-        String tempDir = tempDirFile.getAbsolutePath();
-        if (tempDir == null || tempDir.length() == 0) {
-            tempDir = System.getProperty(JAVA_IO_TMPDIR_KEY);
-        }
-        return tempDir;
+    // ===================================================================================
+    //                                                                              Finish
+    //                                                                              ======
+    @Override
+    public void finish() {
+        rollback();
     }
 
     // ===================================================================================
@@ -353,11 +383,11 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
 
         protected String getBaseFileName(final String filePath) {
             final String fileName = new File(filePath).getName();
-            int colonIndex = fileName.indexOf(':');
+            int colonIndex = fileName.indexOf(":");
             if (colonIndex == -1) {
                 colonIndex = fileName.indexOf("\\\\"); // Windows SMB
             }
-            final int backslashIndex = fileName.lastIndexOf('\\');
+            final int backslashIndex = fileName.lastIndexOf("\\");
             if (colonIndex > -1 && backslashIndex > -1) {
                 return fileName.substring(backslashIndex + 1);
             }
@@ -379,17 +409,17 @@ public class FessMultipartRequestHandler implements MultipartRequestHandler {
     //                                                                            Accessor
     //                                                                            ========
     @Override
-    public Map<String, Object> getAllElements() {
+    public Map<String, Object> getAllElements() { // not null after parsing
         return elementsAll;
     }
 
     @Override
-    public Map<String, String[]> getTextElements() {
+    public Map<String, String[]> getTextElements() { // me too
         return elementsText;
     }
 
     @Override
-    public Map<String, MultipartFormFile> getFileElements() {
+    public Map<String, MultipartFormFile> getFileElements() { // me too
         return elementsFile;
     }
 }

+ 1 - 0
src/main/resources/fess_config.properties

@@ -195,6 +195,7 @@ http.proxy.username=
 http.proxy.password=
 http.fileupload.max.size=262144000
 http.fileupload.threshold.size=262144
+http.fileupload.max.file.count=10
 
 # ========================================================================================
 #                                                                                   Index