burtonrhodes commented on code in PR #861:
URL: https://github.com/apache/struts/pull/861#discussion_r1468547208


##########
core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java:
##########
@@ -21,375 +21,116 @@
 import jakarta.servlet.http.HttpServletRequest;
 import org.apache.commons.fileupload2.core.DiskFileItem;
 import org.apache.commons.fileupload2.core.DiskFileItemFactory;
-import org.apache.commons.fileupload2.core.FileItem;
-import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException;
-import org.apache.commons.fileupload2.core.FileUploadContentTypeException;
-import org.apache.commons.fileupload2.core.FileUploadException;
-import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException;
-import org.apache.commons.fileupload2.core.FileUploadSizeException;
-import org.apache.commons.fileupload2.core.RequestContext;
-import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload;
+import 
org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.apache.struts2.dispatcher.LocalizedMessage;
 
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.UncheckedIOException;
 import java.nio.charset.Charset;
+import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Enumeration;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 
 /**
- * Multipart form data request adapter for Jakarta Commons Fileupload package.
+ * Multipart form data request adapter for Jakarta Commons FileUpload package.
  */
-public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
+public class JakartaMultiPartRequest extends AbstractMultiPartRequest<File> {
 
-    static final Logger LOG = 
LogManager.getLogger(JakartaMultiPartRequest.class);
+    private static final Logger LOG = 
LogManager.getLogger(JakartaMultiPartRequest.class);
 
-    // maps parameter name -> List of FileItem objects
-    protected Map<String, List<FileItem>> files = new HashMap<>();
+    @Override
+    protected void processUpload(HttpServletRequest request, String saveDir) 
throws IOException {
+        String charset = StringUtils.isBlank(request.getCharacterEncoding())
+                ? defaultEncoding
+                : request.getCharacterEncoding();
 
-    // maps parameter name -> List of param values
-    protected Map<String, List<String>> params = new HashMap<>();
+        JakartaServletDiskFileUpload servletFileUpload =
+                prepareServletFileUpload(Charset.forName(charset), 
Path.of(saveDir));
 
-    /**
-     * Creates a new request wrapper to handle multipart data using methods 
adapted from Jason Pell's
-     * multipart classes (see class description).
-     *
-     * @param saveDir the directory to save off the file
-     * @param request the request containing the multipart
-     * @throws java.io.IOException is thrown if encoding fails.
-     */
-    public void parse(HttpServletRequest request, String saveDir) throws 
IOException {
-        try {
-            setLocale(request);
-            processUpload(request, saveDir);
-        } catch (FileUploadException e) {
-            LOG.debug("Request exceeded size limit!", e);
-            LocalizedMessage errorMessage;
-            if (e instanceof FileUploadByteCountLimitException) {
-                FileUploadByteCountLimitException ex = 
(FileUploadByteCountLimitException) e;
-                errorMessage = buildErrorMessage(e, new Object[]{
-                        ex.getFieldName(), ex.getFileName(), 
ex.getPermitted(), ex.getActualSize()
-                });
-            } else if (e instanceof FileUploadFileCountLimitException) {
-                FileUploadFileCountLimitException ex = 
(FileUploadFileCountLimitException) e;
-                errorMessage = buildErrorMessage(e, new Object[]{
-                        ex.getPermitted(), ex.getActualSize()
-                });
-            } else if (e instanceof FileUploadSizeException) {
-                FileUploadSizeException ex = (FileUploadSizeException) e;
-                errorMessage = buildErrorMessage(e, new Object[]{
-                        ex.getPermitted(), ex.getActualSize()
-                });
-            } else if (e instanceof FileUploadContentTypeException) {
-                FileUploadContentTypeException ex = 
(FileUploadContentTypeException) e;
-                errorMessage = buildErrorMessage(e, new Object[]{
-                        ex.getContentType()
-                });
+        for (DiskFileItem item : servletFileUpload.parseRequest(request)) {

Review Comment:
   In thinking about it more, that might be a security issue, as the 
serletFileUpload object would upload everything first (something that the 
setting is trying to prevent), only to perform the check after the processing 
is done.  Ok, never mind. ;-)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to