diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ddd4f76..71f77ea55 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/apps/examples/src/main/java/org/apache/struts/webapp/upload/UploadAction.java b/apps/examples/src/main/java/org/apache/struts/webapp/upload/UploadAction.java index 637005408..dee5fb244 100644 --- a/apps/examples/src/main/java/org/apache/struts/webapp/upload/UploadAction.java +++ b/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"); diff --git a/apps/examples/src/main/webapp/WEB-INF/upload/struts-config.xml b/apps/examples/src/main/webapp/WEB-INF/upload/struts-config.xml index 1f5279eb1..91f26b4e3 100644 --- a/apps/examples/src/main/webapp/WEB-INF/upload/struts-config.xml +++ b/apps/examples/src/main/webapp/WEB-INF/upload/struts-config.xml @@ -23,8 +23,12 @@ - + + + + + @@ -33,6 +37,7 @@ name="uploadForm" scope="request" validate="true" + cancellable="true" input="input"> @@ -47,6 +52,7 @@ name="uploadForm" scope="request" validate="true" + cancellable="true" input="input"> @@ -61,6 +67,7 @@ name="uploadForm" scope="request" validate="true" + cancellable="true" input="input"> diff --git a/apps/examples/src/main/webapp/upload/upload-multi.jsp b/apps/examples/src/main/webapp/upload/upload-multi.jsp index dd790ec7f..4b4640cb3 100644 --- a/apps/examples/src/main/webapp/upload/upload-multi.jsp +++ b/apps/examples/src/main/webapp/upload/upload-multi.jsp @@ -45,7 +45,7 @@

If you checked the box to write to a file, please specify the file path here:

- +  

diff --git a/apps/examples/src/main/webapp/upload/upload-utf8.jsp b/apps/examples/src/main/webapp/upload/upload-utf8.jsp index 621d4a12f..47d93b0e2 100644 --- a/apps/examples/src/main/webapp/upload/upload-utf8.jsp +++ b/apps/examples/src/main/webapp/upload/upload-utf8.jsp @@ -41,6 +41,7 @@

- +   + diff --git a/apps/examples/src/main/webapp/upload/upload.jsp b/apps/examples/src/main/webapp/upload/upload.jsp index 2d75465b8..71adfb28c 100644 --- a/apps/examples/src/main/webapp/upload/upload.jsp +++ b/apps/examples/src/main/webapp/upload/upload.jsp @@ -45,7 +45,7 @@

If you checked the box to write to a file, please specify the file path here:

- +  

diff --git a/apps/examples/src/main/webapp/upload/upload2.jsp b/apps/examples/src/main/webapp/upload/upload2.jsp index c611f836d..1fdfbf415 100644 --- a/apps/examples/src/main/webapp/upload/upload2.jsp +++ b/apps/examples/src/main/webapp/upload/upload2.jsp @@ -46,7 +46,7 @@

If you checked the box to write to a file, please specify the file path here:

- +  

diff --git a/core/src/main/java/org/apache/struts/action/Action.java b/core/src/main/java/org/apache/struts/action/Action.java index 2d2bfc437..4c35a510e 100644 --- a/core/src/main/java/org/apache/struts/action/Action.java +++ b/core/src/main/java/org/apache/struts/action/Action.java @@ -23,13 +23,6 @@ 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; @@ -37,6 +30,13 @@ 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; + /** *

An Action 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, * false otherwise. */ protected boolean isCancelled(HttpServletRequest request) { - return (request.getAttribute(Globals.CANCEL_KEY) != null); + return RequestUtils.isRequestCancelled(request); } /** diff --git a/core/src/main/java/org/apache/struts/action/RequestProcessor.java b/core/src/main/java/org/apache/struts/action/RequestProcessor.java index 4c157afbe..b9a2fea44 100644 --- a/core/src/main/java/org/apache/struts/action/RequestProcessor.java +++ b/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; + /** *

RequestProcessor 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) diff --git a/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java b/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java index 6c3431871..16807ae94 100644 --- a/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java +++ b/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; + /** *

Populate the form bean (if any) for this request.

* @@ -59,16 +64,32 @@ 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); } @@ -76,7 +97,11 @@ protected boolean execute_(ActionContext actionCtx) // 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); } } -} \ No newline at end of file +} diff --git a/core/src/main/java/org/apache/struts/chain/commands/servlet/PopulateActionForm.java b/core/src/main/java/org/apache/struts/chain/commands/servlet/PopulateActionForm.java index 1dc752d24..90ebce2a5 100644 --- a/core/src/main/java/org/apache/struts/chain/commands/servlet/PopulateActionForm.java +++ b/core/src/main/java/org/apache/struts/chain/commands/servlet/PopulateActionForm.java @@ -20,7 +20,6 @@ */ 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; @@ -28,6 +27,7 @@ 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; diff --git a/core/src/main/java/org/apache/struts/util/RequestUtils.java b/core/src/main/java/org/apache/struts/util/RequestUtils.java index 2a9e89758..016d6c52c 100644 --- a/core/src/main/java/org/apache/struts/util/RequestUtils.java +++ b/core/src/main/java/org/apache/struts/util/RequestUtils.java @@ -337,6 +337,51 @@ public static Locale getUserLocale(HttpServletRequest request, String locale) { return userLocale; } + /** + *

Populate the properties of the specified JavaBean from the specified + * HTTP request, based on matching each parameter name (plus an optional + * prefix and/or suffix) against the corresponding JavaBeans "property + * setter" methods in the bean's class. Suitable conversion is done for + * argument types as described under setProperties.

+ * + *

If you specify a non-null prefix and a non-null + * suffix, the parameter name must match + * both conditions for its value(s) to be used in + * populating bean properties. If the request's content type is + * "multipart/form-data" and the method is "POST", the + * HttpServletRequest object will be wrapped in a + * MultipartRequestWrapper object.

+ * + * @param bean The JavaBean whose properties are to be set + * @param prefix The prefix (if any) to be prepend to bean property names + * when looking for matching parameters + * @param suffix The suffix (if any) to be appended to bean property + * names when looking for matching parameters + * @param request The HTTP request whose parameters are to be used to + * populate bean properties + * @throws ServletException if an exception is thrown while setting + * property values + */ + public static MultipartRequestHandler populateMultipartPost(HttpServletRequest request) + throws ServletException { + + MultipartRequestHandler multipartHandler = null; + if (isRequestMultipartPost(request)) { + // Obtain a MultipartRequestHandler + multipartHandler = getMultipartHandler(request); + + if (multipartHandler != null) { + multipartHandler.setMapping((ActionMapping) request + .getAttribute(Globals.MAPPING_KEY)); + + // Initialize multipart request class handler + multipartHandler.handleRequest(request); + } + } + + return multipartHandler; + } + /** *

Populate the properties of the specified JavaBean from the specified * HTTP request, based on matching each parameter name against the @@ -352,7 +397,7 @@ public static Locale getUserLocale(HttpServletRequest request, String locale) { */ public static void populate(Object bean, HttpServletRequest request) throws ServletException { - populate(bean, null, null, request); + populate(bean, null, null, request, populateMultipartPost(request)); } /** @@ -381,8 +426,9 @@ public static void populate(Object bean, HttpServletRequest request) * property values */ public static void populate(Object bean, String prefix, String suffix, - HttpServletRequest request) + HttpServletRequest request, MultipartRequestHandler multipartHandler) throws ServletException { + // Build a list of relevant request parameters from this request HashMap properties = new HashMap<>(); @@ -392,20 +438,15 @@ public static void populate(Object bean, String prefix, String suffix, // Map for multipart parameters Map multipartParameters = null; - String contentType = request.getContentType(); - String method = request.getMethod(); - boolean isMultipart = false; + final boolean isMultipart = multipartHandler != null; if (bean instanceof ActionForm) { ((ActionForm) bean).setMultipartRequestHandler(null); } - MultipartRequestHandler multipartHandler = null; - if ((contentType != null) - && (contentType.startsWith("multipart/form-data")) - && (method.equalsIgnoreCase("POST"))) { + if (isMultipart) { // Get the ActionServletWrapper from the form bean - ActionServletWrapper servlet; + ActionServletWrapper servlet = null; if (bean instanceof ActionForm) { servlet = ((ActionForm) bean).getServletWrapper(); @@ -416,36 +457,24 @@ public static void populate(Object bean, String prefix, String suffix, + "\"" + bean.getClass().getName() + "\""); } - // Obtain a MultipartRequestHandler - multipartHandler = getMultipartHandler(request); - - if (multipartHandler != null) { - isMultipart = true; - - // Set servlet and mapping info - servlet.setServletFor(multipartHandler); - multipartHandler.setMapping((ActionMapping) request - .getAttribute(Globals.MAPPING_KEY)); - - // Initialize multipart request class handler - multipartHandler.handleRequest(request); - - //stop here if the maximum length has been exceeded - Boolean maxLengthExceeded = - (Boolean) request.getAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED); + // Set servlet and mapping info + servlet.setServletFor(multipartHandler); - if ((maxLengthExceeded != null) - && (maxLengthExceeded.booleanValue())) { - ((ActionForm) bean).setMultipartRequestHandler(multipartHandler); - return; - } + //stop here if the maximum length has been exceeded + Boolean maxLengthExceeded = + (Boolean) request.getAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED); - //retrieve form values and put into properties - multipartParameters = - getAllParametersForMultipartRequest(request, - multipartHandler); - names = Collections.enumeration(multipartParameters.keySet()); + if ((maxLengthExceeded != null) + && (maxLengthExceeded.booleanValue())) { + ((ActionForm) bean).setMultipartRequestHandler(multipartHandler); + return; } + + //retrieve form values and put into properties + multipartParameters = + getAllParametersForMultipartRequest(request, + multipartHandler); + names = Collections.enumeration(multipartParameters.keySet()); } if (!isMultipart) { @@ -503,7 +532,7 @@ public static void populate(Object bean, String prefix, String suffix, } catch (Exception e) { throw new ServletException("BeanUtils.populate", e); } finally { - if (multipartHandler != null) { + if (isMultipart) { // Set the multipart request handler for our ActionForm. // If the bean isn't an ActionForm, an exception would have been // thrown earlier, so it's safe to assume that our bean is @@ -1197,4 +1226,39 @@ public static boolean isRequestIncluded(HttpServletRequest request) { public static boolean isRequestChained(HttpServletRequest request) { return (request.getAttribute(Globals.CHAIN_KEY) != null); } + + /** + * Verifies whether current request is cancelled or not. + * + * @param request current HTTP request + * @return {@code true} if the request is cancelled; otherwise {@code false} + * @since Struts 1.5 + */ + public static boolean isRequestCancelled(HttpServletRequest request) { + final Object value = request.getAttribute(Globals.CANCEL_KEY); + if (value instanceof Boolean) { + return ((Boolean) value).booleanValue(); + } else { + return false; + } + } + + /** + * Verifies whether the current request is a multipart-post-request. + * + * @param request current HTTP request + * + * @return {@code true} if the request is a multipart-post-request; + * otherwise {@code false} + * + * @since Struts 1.5 + */ + public static boolean isRequestMultipartPost(HttpServletRequest request) { + final String contentType = request.getContentType(); + final String method = request.getMethod(); + + return contentType != null + && contentType.startsWith("multipart/form-data") + && "POST".equalsIgnoreCase(method); + } } \ No newline at end of file diff --git a/extras/src/main/java/org/apache/struts/extras/actions/ActionDispatcher.java b/extras/src/main/java/org/apache/struts/extras/actions/ActionDispatcher.java index 39337bdff..9a76adb6f 100644 --- a/extras/src/main/java/org/apache/struts/extras/actions/ActionDispatcher.java +++ b/extras/src/main/java/org/apache/struts/extras/actions/ActionDispatcher.java @@ -24,11 +24,6 @@ import java.lang.reflect.Method; import java.util.HashMap; -import jakarta.servlet.ServletException; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; - -import org.apache.struts.Globals; import org.apache.struts.action.Action; import org.apache.struts.action.ActionForm; import org.apache.struts.action.ActionForward; @@ -37,9 +32,14 @@ import org.apache.struts.chain.contexts.ServletActionContext; import org.apache.struts.dispatcher.Dispatcher; import org.apache.struts.util.MessageResources; +import org.apache.struts.util.RequestUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import jakarta.servlet.ServletException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + /** *

Action helper class that dispatches to a public method in an * Action.

This class is provided as an alternative mechanism to @@ -496,7 +496,7 @@ protected String getMethodName(ActionMapping mapping, ActionForm form, * @see org.apache.struts.taglib.html.CancelTag */ protected boolean isCancelled(HttpServletRequest request) { - return (request.getAttribute(Globals.CANCEL_KEY) != null); + return RequestUtils.isRequestCancelled(request); } /** diff --git a/src/site/xdoc/userGuide/release-notes.xml b/src/site/xdoc/userGuide/release-notes.xml index 68400bf34..81a99cfac 100644 --- a/src/site/xdoc/userGuide/release-notes.xml +++ b/src/site/xdoc/userGuide/release-notes.xml @@ -97,6 +97,8 @@

Bug

    +
  • Second attempt to fix for "#32 - html:cancel taglib not working in case of MultipartRequestWrapper"
    • +
  • Fix for "#32 - html:cancel taglib not working in case of MultipartRequestWrapper" thanks to nrmnrm
  • Tiles: Correct "I18nFactorySet.initFactory" under windows
  • With Release Candidate 1 & 2:
      @@ -117,6 +119,7 @@

      Improvement

        +
      • Add Method "RequestUtils.isRequestCancelled"
      • With Release Candidate 1 & 2:
        • Replace "maven-bundle-plugin" 5.1.9 with "bnd-maven-plugin" 7.0.0