32 htmlcancel taglib not working in case of multipartrequestwrapper

CHANGELOG.md

@@ -1,7 +1,10 @@
 # Changes
 
-## 1.5.0 / YYYY-MM-DD
+## 1.5.0-RC3 / YYYY-MM-DD
 
+* Second attempt to fix for `#32 - html:cancel taglib not working in case of MultipartRequestWrapper`
+* Add Method `RequestUtils.isRequestCancelled`
+* Fix for `#32 - html:cancel taglib not working in case of MultipartRequestWrapper` thanks to nrmnrm
 * Tiles: Correct `I18nFactorySet.initFactory` under windows
 * Set Version to 1.5.0-SNAPSHOT
 * Update documentation to version 1.5.0

apps/examples/src/main/java/org/apache/struts/webapp/upload/UploadAction.java

@@ -28,14 +28,15 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-
 import org.apache.struts.action.Action;
 import org.apache.struts.action.ActionForm;
 import org.apache.struts.action.ActionForward;
 import org.apache.struts.action.ActionMapping;
 import org.apache.struts.upload.FormFile;
+import org.apache.struts.upload.MultipartRequestHandler;
+
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 
 
 
@@ -60,6 +61,15 @@ public ActionForward execute(ActionMapping mapping,
         throws Exception {
 
         if (form instanceof UploadForm) {
+            UploadForm theForm = (UploadForm) form;
+
+            final MultipartRequestHandler multipartHandler = form.getMultipartRequestHandler();
+
+            // Was this transaction cancelled?
+            if (isCancelled(request)) {
+                multipartHandler.rollback();
+                return mapping.findForward("home");
+            }
 
             //this line is here for when the input page is upload-utf8.jsp,
             //it sets the correct character encoding for the response
@@ -69,8 +79,6 @@ public ActionForward execute(ActionMapping mapping,
                 response.setContentType("text/html; charset=utf-8");
             }
 
-            UploadForm theForm = (UploadForm) form;
-
             //retrieve the text data
             String text = theForm.getTheText();
 
@@ -84,11 +92,11 @@ public ActionForward execute(ActionMapping mapping,
 
             // Following is to test fix for STR-3173
             if (file == null) {
-                final FormFile[] files = form.getMultipartRequestHandler().getFileElements().get("otherFile");
+                final FormFile[] files = multipartHandler.getFileElements().get("otherFile");
                 fileCount = files.length;
                 file = fileCount == 0 ? null : files[0];
             } else {
-                final FormFile[] files = form.getMultipartRequestHandler().getFileElements().get("theFile");
+                final FormFile[] files = multipartHandler.getFileElements().get("theFile");
                 fileCount = files.length;
             }
 
@@ -150,8 +158,8 @@ public ActionForward execute(ActionMapping mapping,
             request.setAttribute("size", size);
             request.setAttribute("data", data);
 
-            //destroy the temporary file created
-            file.destroy();
+            //destroy temporary files
+            multipartHandler.finish();
 
             //return a forward to display.jsp
             return mapping.findForward("display");

apps/examples/src/main/webapp/WEB-INF/upload/struts-config.xml

@@ -23,8 +23,12 @@
         <form-bean name="uploadForm"
                    type="org.apache.struts.webapp.upload.UploadForm"/>
     </form-beans>
-    <action-mappings>
 
+    <global-forwards>
+        <forward name="home" path="/welcome.do" module=""/>
+    </global-forwards>
+
+    <action-mappings>
         <action path="/upload" forward="/upload.jsp"/>
 
         <!-- Upload Action -->
@@ -33,6 +37,7 @@
                 name="uploadForm"
                 scope="request"
                 validate="true"
+                cancellable="true"
                 input="input">
             <forward name="input" path="/upload.jsp"/>
             <forward name="display" path="/display.jsp"/>
@@ -47,6 +52,7 @@
                 name="uploadForm"
                 scope="request"
                 validate="true"
+                cancellable="true"
                 input="input">
             <forward name="input" path="/upload2.jsp"/>
             <forward name="display" path="/display.jsp"/>
@@ -61,6 +67,7 @@
                 name="uploadForm"
                 scope="request"
                 validate="true"
+                cancellable="true"
                 input="input">
             <forward name="input" path="/upload-multi.jsp"/>
             <forward name="display" path="/display.jsp"/>

apps/examples/src/main/webapp/upload/upload-multi.jsp

@@ -45,7 +45,7 @@
     <p>If you checked the box to write to a file, please specify the file path here: <br />
     <html:text property="filePath"  errorStyle="background-color: yellow"/></p>
     <p>
-    <html:submit />
+    <html:submit />&nbsp;<html:cancel />
     </p>
     </html:form>
 

apps/examples/src/main/webapp/upload/upload-utf8.jsp

@@ -41,6 +41,7 @@
     <html:text property="filePath" />
     <br />
     <br />
-    <html:submit /></html:form>
+    <html:submit />&nbsp;<html:cancel />
+    </html:form>
   </body>
 </html>

apps/examples/src/main/webapp/upload/upload.jsp

@@ -45,7 +45,7 @@
     <p>If you checked the box to write to a file, please specify the file path here: <br />
     <html:text property="filePath"  errorStyle="background-color: yellow"/></p>
     <p>
-    <html:submit />
+    <html:submit />&nbsp;<html:cancel />
     </p>
     </html:form>
 

apps/examples/src/main/webapp/upload/upload2.jsp

@@ -46,7 +46,7 @@
     <p>If you checked the box to write to a file, please specify the file path here: <br />
     <html:text property="filePath"  errorStyle="background-color: yellow"/></p>
     <p>
-    <html:submit />
+    <html:submit />&nbsp;<html:cancel />
     </p>
     </html:form>
 

core/src/main/java/org/apache/struts/action/Action.java

@@ -23,20 +23,20 @@
 import java.io.Serializable;
 import java.util.Locale;
 
-import jakarta.servlet.ServletContext;
-import jakarta.servlet.ServletRequest;
-import jakarta.servlet.ServletResponse;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-import jakarta.servlet.http.HttpSession;
-
 import org.apache.struts.Globals;
 import org.apache.struts.config.ModuleConfig;
 import org.apache.struts.util.MessageResources;
 import org.apache.struts.util.ModuleUtils;
 import org.apache.struts.util.RequestUtils;
 import org.apache.struts.util.TokenProcessor;
 
+import jakarta.servlet.ServletContext;
+import jakarta.servlet.ServletRequest;
+import jakarta.servlet.ServletResponse;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import jakarta.servlet.http.HttpSession;
+
 /**
  * <p>An <strong>Action</strong> is an adapter between the contents of an
  * incoming HTTP request and the corresponding business logic that should be
@@ -403,7 +403,7 @@ protected MessageResources getResources(HttpServletRequest request,
      *         <code>false</code> otherwise.
      */
     protected boolean isCancelled(HttpServletRequest request) {
-        return (request.getAttribute(Globals.CANCEL_KEY) != null);
+        return RequestUtils.isRequestCancelled(request);
     }
 
     /**

core/src/main/java/org/apache/struts/action/RequestProcessor.java

@@ -25,13 +25,6 @@
 import java.util.HashMap;
 import java.util.Locale;
 
-import jakarta.servlet.RequestDispatcher;
-import jakarta.servlet.ServletContext;
-import jakarta.servlet.ServletException;
-import jakarta.servlet.http.HttpServletRequest;
-import jakarta.servlet.http.HttpServletResponse;
-import jakarta.servlet.http.HttpSession;
-
 import org.apache.struts.Globals;
 import org.apache.struts.config.ActionConfig;
 import org.apache.struts.config.ExceptionConfig;
@@ -43,6 +36,13 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import jakarta.servlet.RequestDispatcher;
+import jakarta.servlet.ServletContext;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import jakarta.servlet.http.HttpSession;
+
 /**
  * <p><strong>RequestProcessor</strong> contains the processing logic that the
  * {@link ActionServlet} performs as it receives each servlet request from the
@@ -805,7 +805,7 @@ protected void processPopulate(HttpServletRequest request,
         }
 
         RequestUtils.populate(form, mapping.getPrefix(), mapping.getSuffix(),
-            request);
+            request, RequestUtils.populateMultipartPost(request));
 
         // Set the cancellation request attribute if appropriate
         if ((request.getParameter(Globals.CANCEL_PROPERTY) != null)

core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java

@@ -25,10 +25,15 @@
 import org.apache.struts.Globals;
 import org.apache.struts.action.ActionForm;
 import org.apache.struts.chain.contexts.ActionContext;
+import org.apache.struts.chain.contexts.ServletActionContext;
 import org.apache.struts.config.ActionConfig;
+import org.apache.struts.upload.MultipartRequestHandler;
+import org.apache.struts.util.RequestUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import jakarta.servlet.http.HttpServletRequest;
+
 /**
  * <p>Populate the form bean (if any) for this request.</p>
  *
@@ -59,24 +64,44 @@ protected boolean execute_(ActionContext actionCtx)
         ActionConfig actionConfig = actionCtx.getActionConfig();
         ActionForm actionForm = actionCtx.getActionForm();
 
+        ServletActionContext saContext = (ServletActionContext) actionCtx;
+        HttpServletRequest request = saContext.getRequest();
+
+        final boolean isReset = isReset(actionCtx, actionConfig);
+
+        // Set the multipart class
+        if (isReset && actionConfig.getMultipartClass() != null) {
+            saContext.getRequestScope().put(Globals.MULTIPART_KEY,
+                actionConfig.getMultipartClass());
+        }
+
+        final MultipartRequestHandler multipartHandler = RequestUtils.populateMultipartPost(request);
+
         // First determine if the request was cancelled
         handleCancel(actionCtx, actionConfig, actionForm);
 
         // Is there a form bean for this request?
         if (actionForm == null) {
-            return (false);
+            if (multipartHandler != null) {
+                multipartHandler.rollback();
+            }
+            return CONTINUE_PROCESSING;
         }
 
         // Reset the form bean only if configured so
-        if (isReset(actionCtx, actionConfig)) {
+        if (isReset) {
             log.debug("Reseting form bean '{}'",  actionConfig.getName());
             reset(actionCtx, actionConfig, actionForm);
         }
 
         // Populate the form bean only if configured so
         if (isPopulate(actionCtx, actionConfig)) {
             log.debug("Populating form bean '{}'", actionConfig.getName());
-            populate(actionCtx, actionConfig, actionForm);
+            populate(actionCtx, actionConfig, actionForm, multipartHandler);
+        } else {
+            if (multipartHandler != null) {
+                multipartHandler.rollback();
+            }
         }
 
         return CONTINUE_PROCESSING;
@@ -130,7 +155,8 @@ protected abstract void reset(ActionContext context,
      * @throws Exception On an unexpected error
      */
     protected abstract void populate(ActionContext context,
-        ActionConfig actionConfig, ActionForm actionForm)
+        ActionConfig actionConfig, ActionForm actionForm,
+        MultipartRequestHandler multipartHandler)
         throws Exception;
 
     // original implementation casting context to WebContext is not safe
@@ -198,4 +224,4 @@ protected void handleCancel(ActionContext context,
             context.setCancelled(Boolean.FALSE);
         }
     }
-}
+}

core/src/main/java/org/apache/struts/chain/commands/servlet/PopulateActionForm.java

@@ -20,14 +20,14 @@
  */
 package org.apache.struts.chain.commands.servlet;
 
-import org.apache.struts.Globals;
 import org.apache.struts.action.ActionForm;
 import org.apache.struts.action.ActionMapping;
 import org.apache.struts.chain.commands.AbstractPopulateActionForm;
 import org.apache.struts.chain.contexts.ActionContext;
 import org.apache.struts.chain.contexts.ServletActionContext;
 import org.apache.struts.config.ActionConfig;
 import org.apache.struts.config.PopulateEvent;
+import org.apache.struts.upload.MultipartRequestHandler;
 import org.apache.struts.util.RequestUtils;
 
 import jakarta.servlet.http.HttpServletRequest;
@@ -44,13 +44,13 @@ public class PopulateActionForm extends AbstractPopulateActionForm {
     // ------------------------------------------------------- Protected Methods
 
     protected void populate(ActionContext context, ActionConfig actionConfig,
-        ActionForm actionForm)
+        ActionForm actionForm, MultipartRequestHandler multipartHandler)
         throws Exception {
         ServletActionContext saContext = (ServletActionContext) context;
         HttpServletRequest request = saContext.getRequest();
 
         RequestUtils.populate(actionForm, actionConfig.getPrefix(),
-            actionConfig.getSuffix(), request);
+            actionConfig.getSuffix(), request, multipartHandler);
     }
 
     protected void reset(ActionContext context, ActionConfig actionConfig,
@@ -59,12 +59,6 @@ protected void reset(ActionContext context, ActionConfig actionConfig,
         HttpServletRequest request = saContext.getRequest();
 
         actionForm.reset((ActionMapping) actionConfig, request);
-
-        // Set the multipart class
-        if (actionConfig.getMultipartClass() != null) {
-            saContext.getRequestScope().put(Globals.MULTIPART_KEY,
-                actionConfig.getMultipartClass());
-        }
     }
 
     // ---------------------------------------------------------- Helper Methods
@@ -145,6 +139,10 @@ private boolean getResetOrPopulate(ActionContext context, String[] events) {
                 if (RequestUtils.isRequestChained(request)) {
                     return true;
                 }
+            } else if (attribute.equals(PopulateEvent.CANCEL)) {
+                if (RequestUtils.isRequestCancelled(request)) {
+                    return true;
+                }
             } else if (attribute.equals(PopulateEvent.REQUEST)) {
                 if (RequestUtils.isRequestForwarded(request)) {
                     continue;