Changeset 35720 for other-projects/the-macronizer/trunk/src/java/web/servlets

Timestamp:

2021-11-03T16:18:48+13:00 (2 years ago)

Author:

cstephen

Message:

Refactor FileUpload logic

Location:

other-projects/the-macronizer/trunk/src/java/web/servlets

Files:

• DirectInput.java (modified) (1 diff)
• FileUpload.java (modified) (14 diffs)

other-projects/the-macronizer/trunk/src/java/web/servlets/DirectInput.java

@@ -85 +85 @@
 
         final String fragment = request.getParameter("fragment");
-        final Boolean showAdvancedOptions = request.getParameter("options") != null && request.getParameter("options").equals("true");
-        final Boolean shouldPreserveMacrons = request.getParameter("preserveExistingMacrons") != null && request.getParameter("preserveExistingMacrons").equals("true");
+        final Boolean showAdvancedOptions = Boolean.parseBoolean(request.getParameter("options"));
+        final Boolean shouldPreserveMacrons = Boolean.parseBoolean(request.getParameter("preserveExistingMacrons"));
         final OutputType outputType = OutputType.parse(request.getParameter("o"));
 
         // Note that the lang parameter might be null if the request hasn't been made from an internal JSP page
-        final String lang = request.getParameter("lang");
-        final String forwardPath = lang != null ? "/jsp/" + lang : "/jsp/mi";
+        final String forwardPath = request.getParameter("lang") != null
+            ? "/jsp/" + request.getParameter("lang")
+            : "/jsp/mi";
 
         try

other-projects/the-macronizer/trunk/src/java/web/servlets/FileUpload.java

@@ -72 +72 @@
         throws IllegalStateException, IOException, ServletException
     {
-        final OutputType outputType = OutputType.parse(request.getParameter("o")); // TODO: This probably won't work with the multipart request
-        
+        List<FileItem> formItems = null;
+        Properties properties = null;
         File restoredFile = null;
         PluginConfiguration configuration = null;
-        Properties properties = null;
-        String forwardPath = "/jsp/mi";
 
         try
         {
-            properties = handleRequest(request);
-            forwardPath = properties.getLanguage() != null ? "/jsp/" + properties.getLanguage() : forwardPath;
+            formItems = parseFormItems(request);
+            properties = getRequestProperties(formItems);
         }
         catch (Exception ex)
         {
-            setError(outputType, forwardPath, "INVALID_REQUEST", request, response);
+            throw new ServletException("Expected a multipart/form-data request.", ex);
         }
 
         try
         {
+            Boolean foundFile = getRequestFile(formItems, properties);
+            if (!foundFile)
+            {
+                setError(properties, "FILE_NOT_FOUND_ERROR", request, response, true);
+                return;
+            }
+        }
+        catch (Exception ex)
+        {
+            ex.printStackTrace();
+            logger.error("Failed to get uploaded file", ex);
+            setError(properties, "UNEXPECTED_ERROR", request, response, false);
+        }
+
+        try
+        {
             configuration = configure(properties);
 
@@ -96 +110 @@
             {
                 configuration.getFile().delete();
-                setError(outputType, forwardPath, "FILE_NOT_FOUND_ERROR", request, response);
+                setError(properties, "FILE_NOT_FOUND_ERROR", request, response, false);
                 return;
             }
@@ -106 +120 @@
             request.setAttribute("filename", properties.getFilename());
             request.setAttribute("preserveMacrons", properties.getPreserveExistingMacrons());
-            request.setAttribute("options", properties.getOptions());
-            forward(forwardPath + "/main.jsp", request, response);
+            request.setAttribute("options", properties.getShowAdvancedOptions());
+
+            forward(properties.getJspForwardPath() + "/main.jsp", request, response);
         }
         catch (UnsupportedOperationException uoex)
@@ -113 +128 @@
             FileUtil.deleteFile(restoredFile);
             logger.error("Failed to restore macrons on a file", uoex);
-            setError(outputType, forwardPath, "FILE_TYPE_NOT_SUPPORTED_ERROR", request, response);
+            setError(properties, "FILE_TYPE_NOT_SUPPORTED_ERROR", request, response, false);
         }
         catch (Exception ex)
@@ -120 +135 @@
             ex.printStackTrace();
             logger.error("Failed to restore macrons on a file", ex);
-            setError(outputType, forwardPath, "UNEXPECTED_ERROR", request, response);
+            setError(properties, "UNEXPECTED_ERROR", request, response, false);
         }
         finally
@@ -127 +142 @@
             {
                 FileUtil.deleteFile(configuration.getFile());
-            }
-        }
-    }
-
-    private void setError(OutputType outputType, String forwardPath, String errorCode, HttpServletRequest request, HttpServletResponse response)
+                // We don't clean up the file held by the properties object,
+                // as the configuration holds a reference to said file
+            }
+        }
+    }
+
+    private void setError(
+        Properties properties,
+        String errorCode,
+        HttpServletRequest request,
+        HttpServletResponse response,
+        Boolean isClientError)
         throws IOException, ServletException
     {
-        switch (outputType) {
+        switch (properties.getOutputType()) {
             case Json:
                 // 500 Internal Server Error
-                response.sendError(500, errorCode);
+                response.sendError(isClientError ? 400 : 500, errorCode);
                 break;
             default:
                 request.setAttribute("errorMessage", errorCode);
-                forward(forwardPath + "/error.jsp", request, response);
+                forward(properties.getJspForwardPath()+ "/error.jsp", request, response);
                 break;
         }
@@ -161 +183 @@
     }
 
-    private Properties handleRequest(HttpServletRequest request)
-        throws Exception, FileUploadException, IOException
-    {
-        Properties requestData = new Properties();
+    /**
+     * Parses multipart items from a request.
+     * @param request The request
+     * @return The parsed multipart items.
+     * @throws FileUploadException
+     */
+    private List<FileItem> parseFormItems(HttpServletRequest request)
+        throws FileUploadException
+    {
         DiskFileItemFactory factory = new DiskFileItemFactory();
         factory.setSizeThreshold(10 * 1024 * 1024);
@@ -174 +201 @@
         }
 
-        for (FileItem item : items)
+        return items;
+    }
+    
+    /**
+     * Gets the properties of the request by checking the submitted form items.
+     * @param formItems The multipart form items.
+     * @return The properties.
+     */
+    private Properties getRequestProperties(List<FileItem> formItems)
+    {
+        Properties properties = new Properties();
+
+        for (FileItem item : formItems)
+        {
+            if (!item.isFormField())
+            {
+                continue;
+            }
+
+            String fieldName = item.getFieldName();
+            String fieldValue = item.getString();
+
+            switch (fieldName)
+            {
+                case "charsetEncoding":
+                    properties.setCharsetEncoding(fieldValue);
+                    break;
+                case "fileType":
+                    properties.setFileType(fieldValue);
+                    break;
+                case "preserveExistingMacrons":
+                    properties.setPreserveExistingMacrons(fieldValue);
+                    break;
+                case "lang":
+                    properties.setLanguage(fieldValue);
+                    break;
+                case "options":
+                    properties.setShowAdvancedOptions(fieldValue);
+                    break;
+                case "o":
+                    properties.setOutputType(fieldValue);
+                    break;
+            }
+        }
+
+        return properties;
+    }
+
+    /**
+     * Gets the first file contained in the submitted form items.
+     * @param formItems The multipart form items.
+     * @return The file, or <code>null</code> if the request contained no files.
+     * @throws Exception
+     * @throws IOException
+     */
+    private Boolean getRequestFile(List<FileItem> formItems, Properties properties)
+        throws Exception, IOException
+    {
+        for (FileItem item : formItems)
         {
             if (item.isFormField())
             {
-                String fieldName = item.getFieldName();
-                String fieldValue = item.getString();
-                if (fieldName.equals("charsetEncoding")) {
-                    requestData.setCharsetEncoding(fieldValue);
-                } else if (fieldName.equals("fileType")) {
-                    requestData.setFileType(fieldValue);
-                } else if (fieldName.equals("preserveExistingMacrons")) {
-                    requestData.setPreserveExistingMacrons(fieldValue);
-                } else if (fieldName.equals("lang")) {
-                    requestData.setLanguage(fieldValue);
-                } else if (fieldName.equals("options")) {
-                    requestData.setOptions(fieldValue);
-                }
-            }
-            else
-            {
-                String fileType = FileUtil.guessFileType(new File(item.getName()));
-                File file = File.createTempFile(FileUtil.TMP_FILE_PREFIX, fileType, tmpdir);
-                item.write(file);
-                requestData.setFile(file);
-                requestData.setFilename(item.getName());
-            }
-        }
-
-        return requestData;
+                continue;
+            }
+
+            String fileType = FileUtil.guessFileType(new File(item.getName()));
+            File file = File.createTempFile(FileUtil.TMP_FILE_PREFIX, fileType, tmpdir);
+            item.write(file);
+
+            properties.setFile(file);
+            properties.setFilename(item.getName());
+
+            return true;
+        }
+
+        return false;
     }
     
@@ -233 +307 @@
         final String fileType = properties.getFileType().equals("(detect automatically)") ? FileUtil.guessFileType(file) : properties.getFileType();
         final String charsetEncoding = properties.getCharsetEncoding().equals("(detect automatically)") ? "utf8" : properties.getCharsetEncoding();
-        final String preserveExistingMacrons = properties.getPreserveExistingMacrons();
 
         final PluginConfiguration configuration = new PluginConfiguration();
@@ -239 +312 @@
         configuration.setFileType(fileType);
         configuration.setCharsetEncoding(charsetEncoding);
-        configuration.setPreserveExistingMacrons(Boolean.getBoolean(preserveExistingMacrons));
+        configuration.setPreserveExistingMacrons(properties.getPreserveExistingMacrons());
 
         return configuration;
@@ -250 +323 @@
         private String fileType;
         private String charsetEncoding;
-        private String preserveExistingMacrons;
+        private Boolean preserveExistingMacrons;
         private String language;
-        private String options;
+        private Boolean showAdvancedOptions;
+        private OutputType outputType;
 
         public File getFile() {
@@ -270 +344 @@
         }
 
-        public String getPreserveExistingMacrons() {
+        public Boolean getPreserveExistingMacrons() {
             return preserveExistingMacrons;
         }
 
-        @SuppressWarnings("unused")
         public String getLanguage() {
             return language;
         }
 
-        public String getOptions() {
-            return options;
+        public Boolean getShowAdvancedOptions() {
+            return showAdvancedOptions;
+        }
+
+        public OutputType getOutputType() {
+            return outputType;
+        }
+
+        public String getJspForwardPath() {
+            return getLanguage() != null
+                ? "/jsp/" + getLanguage()
+                : "/jsp/mi";
         }
 
@@ -300 +383 @@
 
         public void setPreserveExistingMacrons(String preserveExistingMacrons) {
-            this.preserveExistingMacrons = preserveExistingMacrons;
+            this.preserveExistingMacrons = Boolean.parseBoolean(preserveExistingMacrons);
         }
 
@@ -307 +390 @@
         }
 
-        public void setOptions(String options) {
-            this.options = options;
+        public void setShowAdvancedOptions(String options) {
+            this.showAdvancedOptions = Boolean.parseBoolean(options);
+        }
+
+        public void setOutputType(String outputType) {
+            this.outputType = OutputType.parse(outputType);
         }
     }