From 7b41296165dcd55c943459244831e8724e9af2a8 Mon Sep 17 00:00:00 2001 From: nrmnrm Date: Mon, 17 Jun 2024 09:36:43 +0200 Subject: [PATCH 1/4] Fix html_cancel regression from commit ""STR-286 and STR-1116: ActionForm populate and reset strategy"" --- .../struts/chain/commands/AbstractPopulateActionForm.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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..07d318ad9 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 @@ -59,9 +59,6 @@ protected boolean execute_(ActionContext actionCtx) ActionConfig actionConfig = actionCtx.getActionConfig(); ActionForm actionForm = actionCtx.getActionForm(); - // First determine if the request was cancelled - handleCancel(actionCtx, actionConfig, actionForm); - // Is there a form bean for this request? if (actionForm == null) { return (false); @@ -79,6 +76,9 @@ protected boolean execute_(ActionContext actionCtx) populate(actionCtx, actionConfig, actionForm); } + // Always handle cancel last, as it needs populate first + handleCancel(actionCtx, actionConfig, actionForm); + return CONTINUE_PROCESSING; } @@ -198,4 +198,4 @@ protected void handleCancel(ActionContext context, context.setCancelled(Boolean.FALSE); } } -} \ No newline at end of file +} From 8694df95754356c3fe037bd95152c48ca1526306 Mon Sep 17 00:00:00 2001 From: ste-gr <84541644+ste-gr@users.noreply.github.com> Date: Fri, 28 Jun 2024 04:41:51 +0200 Subject: [PATCH 2/4] Fix for html:cancel taglib not working in case of ... MultipartRequestWrapper --- CHANGELOG.md | 3 +- .../struts/webapp/upload/UploadAction.java | 16 +++--- .../webapp/WEB-INF/upload/struts-config.xml | 9 +++- .../src/main/webapp/upload/upload-multi.jsp | 2 +- .../src/main/webapp/upload/upload-utf8.jsp | 3 +- .../src/main/webapp/upload/upload.jsp | 2 +- .../src/main/webapp/upload/upload2.jsp | 2 +- .../commands/AbstractPopulateActionForm.java | 44 +++++++++++----- .../commands/servlet/PopulateActionForm.java | 5 +- .../org/apache/struts/util/RequestUtils.java | 52 +++++++++++++++---- src/site/xdoc/userGuide/release-notes.xml | 1 + 11 files changed, 104 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11ddd4f76..fce5bf7fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Changes -## 1.5.0 / YYYY-MM-DD +## 1.5.0-RC3 / YYYY-MM-DD +* 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..f52a6333f 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,15 +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 jakarta.servlet.http.HttpServletRequest; +import jakarta.servlet.http.HttpServletResponse; + /** @@ -59,8 +59,12 @@ public ActionForward execute(ActionMapping mapping, HttpServletResponse response) throws Exception { - if (form instanceof UploadForm) { + // Was this transaction cancelled? + if (isCancelled(request)) { + return mapping.findForward("home"); + } + if (form instanceof UploadForm) { //this line is here for when the input page is upload-utf8.jsp, //it sets the correct character encoding for the response String encoding = request.getCharacterEncoding(); @@ -150,8 +154,8 @@ public ActionForward execute(ActionMapping mapping, request.setAttribute("size", size); request.setAttribute("data", data); - //destroy the temporary file created - file.destroy(); + //destroy temporary files + form.getMultipartRequestHandler().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/chain/commands/AbstractPopulateActionForm.java b/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java index 07d318ad9..614f53586 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,14 @@ 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.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,25 +63,41 @@ protected boolean execute_(ActionContext actionCtx) ActionConfig actionConfig = actionCtx.getActionConfig(); ActionForm actionForm = actionCtx.getActionForm(); - // Is there a form bean for this request? - if (actionForm == null) { - return (false); - } + ServletActionContext saContext = (ServletActionContext) actionCtx; + HttpServletRequest request = saContext.getRequest(); + boolean isRequestMultipartPost = + RequestUtils.isRequestMultipartPost(request); - // Reset the form bean only if configured so - if (isReset(actionCtx, actionConfig)) { + if (isRequestMultipartPost) { + // Processing for multipart-post-requests 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); - } - // Always handle cancel last, as it needs populate first - handleCancel(actionCtx, actionConfig, actionForm); + handleCancel(actionCtx, actionConfig, actionForm); + } else { + // First determine if the request was cancelled + handleCancel(actionCtx, actionConfig, actionForm); + + // Is there a form bean for this request? + if (actionForm == null) { + return CONTINUE_PROCESSING; + } + + // Reset the form bean only if configured so + if (isReset(actionCtx, actionConfig)) { + log.debug("Reseting form bean '{}'", actionConfig.getName()); + reset(actionCtx, actionConfig, actionForm); + } + + // Populate the form bean only if configured so + if (isRequestMultipartPost || isPopulate(actionCtx, actionConfig)) { + log.debug("Populating form bean '{}'", actionConfig.getName()); + populate(actionCtx, actionConfig, actionForm); + } + } return CONTINUE_PROCESSING; } 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..f140d18b3 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 @@ -55,10 +55,13 @@ protected void populate(ActionContext context, ActionConfig actionConfig, protected void reset(ActionContext context, ActionConfig actionConfig, ActionForm actionForm) { + ServletActionContext saContext = (ServletActionContext) context; HttpServletRequest request = saContext.getRequest(); - actionForm.reset((ActionMapping) actionConfig, request); + if (actionForm != null) { + actionForm.reset((ActionMapping) actionConfig, request); + } // Set the multipart class if (actionConfig.getMultipartClass() != null) { 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..0e5832bda 100644 --- a/core/src/main/java/org/apache/struts/util/RequestUtils.java +++ b/core/src/main/java/org/apache/struts/util/RequestUtils.java @@ -392,8 +392,6 @@ 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; if (bean instanceof ActionForm) { @@ -401,15 +399,13 @@ public static void populate(Object bean, String prefix, String suffix, } MultipartRequestHandler multipartHandler = null; - if ((contentType != null) - && (contentType.startsWith("multipart/form-data")) - && (method.equalsIgnoreCase("POST"))) { + if (isRequestMultipartPost(request)) { // Get the ActionServletWrapper from the form bean - ActionServletWrapper servlet; + ActionServletWrapper servlet = null; if (bean instanceof ActionForm) { servlet = ((ActionForm) bean).getServletWrapper(); - } else { + } else if (bean != null) { throw new ServletException("bean that's supposed to be " + "populated from a multipart request is not of type " + "\"org.apache.struts.action.ActionForm\", but type " @@ -422,20 +418,27 @@ public static void populate(Object bean, String prefix, String suffix, 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); + if (servlet == null) { + multipartHandler.rollback(); + return; + } + + // Set servlet and mapping info + servlet.setServletFor(multipartHandler); + //stop here if the maximum length has been exceeded Boolean maxLengthExceeded = (Boolean) request.getAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED); if ((maxLengthExceeded != null) && (maxLengthExceeded.booleanValue())) { + ((ActionForm) bean).setMultipartRequestHandler(multipartHandler); return; } @@ -452,6 +455,7 @@ public static void populate(Object bean, String prefix, String suffix, names = request.getParameterNames(); } + boolean isNotCancelled = true; while (names.hasMoreElements()) { String name = names.nextElement(); String stripped = name; @@ -494,12 +498,21 @@ public static void populate(Object bean, String prefix, String suffix, // such as 'org.apache.struts.action.CANCEL' if (!(stripped.startsWith("org.apache.struts."))) { properties.put(stripped, parameterValue); + } else + // Test the cancellation attribute + if (Globals.CANCEL_PROPERTY.equals(stripped) + || Globals.CANCEL_PROPERTY_X.equals(stripped)) { + + isNotCancelled = false; + break; } } // Set the corresponding properties of our bean try { - BeanUtils.populate(bean, properties); + if (isNotCancelled) { + BeanUtils.populate(bean, properties); + } } catch (Exception e) { throw new ServletException("BeanUtils.populate", e); } finally { @@ -1197,4 +1210,23 @@ public static boolean isRequestIncluded(HttpServletRequest request) { public static boolean isRequestChained(HttpServletRequest request) { return (request.getAttribute(Globals.CHAIN_KEY) != null); } + + /** + * 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") + && method.equalsIgnoreCase("POST"); + } } \ No newline at end of file diff --git a/src/site/xdoc/userGuide/release-notes.xml b/src/site/xdoc/userGuide/release-notes.xml index 68400bf34..d6a148f5f 100644 --- a/src/site/xdoc/userGuide/release-notes.xml +++ b/src/site/xdoc/userGuide/release-notes.xml @@ -97,6 +97,7 @@

Bug

    +
  • 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:
      From 8460403d7626fff6447bcc8f34113ef9c92b1dfe Mon Sep 17 00:00:00 2001 From: ste-gr <84541644+ste-gr@users.noreply.github.com> Date: Sun, 14 Jul 2024 16:42:25 +0200 Subject: [PATCH 3/4] Add Method RequestUtils.isRequestCancelled --- CHANGELOG.md | 1 + .../java/org/apache/struts/action/Action.java | 16 ++++++++-------- .../org/apache/struts/util/RequestUtils.java | 16 ++++++++++++++++ .../struts/extras/actions/ActionDispatcher.java | 12 ++++++------ src/site/xdoc/userGuide/release-notes.xml | 1 + 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fce5bf7fa..e6232a19d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.5.0-RC3 / YYYY-MM-DD +* 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 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/util/RequestUtils.java b/core/src/main/java/org/apache/struts/util/RequestUtils.java index 0e5832bda..7977c765b 100644 --- a/core/src/main/java/org/apache/struts/util/RequestUtils.java +++ b/core/src/main/java/org/apache/struts/util/RequestUtils.java @@ -1211,6 +1211,22 @@ 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. * 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 d6a148f5f..f2a1d76af 100644 --- a/src/site/xdoc/userGuide/release-notes.xml +++ b/src/site/xdoc/userGuide/release-notes.xml @@ -118,6 +118,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
          • From 8182c60ec94e19310b17e2864befb9e5c3e4d3c0 Mon Sep 17 00:00:00 2001 From: ste-gr <84541644+ste-gr@users.noreply.github.com> Date: Sun, 14 Jul 2024 17:05:50 +0200 Subject: [PATCH 4/4] Second attempt to fix for #32 - html:cancel taglib not working in ... case of MultipartRequestWrapper --- CHANGELOG.md | 1 + .../struts/webapp/upload/UploadAction.java | 24 ++-- .../struts/action/RequestProcessor.java | 16 +-- .../commands/AbstractPopulateActionForm.java | 56 ++++---- .../commands/servlet/PopulateActionForm.java | 21 ++- .../org/apache/struts/util/RequestUtils.java | 122 ++++++++++-------- src/site/xdoc/userGuide/release-notes.xml | 1 + 7 files changed, 132 insertions(+), 109 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6232a19d..71f77ea55 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 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 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 f52a6333f..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 @@ -33,6 +33,7 @@ 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; @@ -59,12 +60,17 @@ public ActionForward execute(ActionMapping mapping, HttpServletResponse response) throws Exception { - // Was this transaction cancelled? - if (isCancelled(request)) { - return mapping.findForward("home"); - } - 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 String encoding = request.getCharacterEncoding(); @@ -73,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(); @@ -88,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; } @@ -155,7 +159,7 @@ public ActionForward execute(ActionMapping mapping, request.setAttribute("data", data); //destroy temporary files - form.getMultipartRequestHandler().finish(); + multipartHandler.finish(); //return a forward to display.jsp return mapping.findForward("display"); 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 614f53586..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 @@ -27,6 +27,7 @@ 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; @@ -65,37 +66,41 @@ protected boolean execute_(ActionContext actionCtx) ServletActionContext saContext = (ServletActionContext) actionCtx; HttpServletRequest request = saContext.getRequest(); - boolean isRequestMultipartPost = - RequestUtils.isRequestMultipartPost(request); - if (isRequestMultipartPost) { - // Processing for multipart-post-requests - log.debug("Reseting form bean '{}'", actionConfig.getName()); - reset(actionCtx, actionConfig, actionForm); + final boolean isReset = isReset(actionCtx, actionConfig); - log.debug("Populating form bean '{}'", actionConfig.getName()); - populate(actionCtx, actionConfig, actionForm); + // Set the multipart class + if (isReset && actionConfig.getMultipartClass() != null) { + saContext.getRequestScope().put(Globals.MULTIPART_KEY, + actionConfig.getMultipartClass()); + } - handleCancel(actionCtx, actionConfig, actionForm); - } else { - // First determine if the request was cancelled - handleCancel(actionCtx, actionConfig, actionForm); + final MultipartRequestHandler multipartHandler = RequestUtils.populateMultipartPost(request); - // Is there a form bean for this request? - if (actionForm == null) { - return CONTINUE_PROCESSING; - } + // First determine if the request was cancelled + handleCancel(actionCtx, actionConfig, actionForm); - // Reset the form bean only if configured so - if (isReset(actionCtx, actionConfig)) { - log.debug("Reseting form bean '{}'", actionConfig.getName()); - reset(actionCtx, actionConfig, actionForm); + // Is there a form bean for this request? + if (actionForm == null) { + if (multipartHandler != null) { + multipartHandler.rollback(); } + return CONTINUE_PROCESSING; + } - // Populate the form bean only if configured so - if (isRequestMultipartPost || isPopulate(actionCtx, actionConfig)) { - log.debug("Populating form bean '{}'", actionConfig.getName()); - populate(actionCtx, actionConfig, actionForm); + // Reset the form bean only if configured so + 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, multipartHandler); + } else { + if (multipartHandler != null) { + multipartHandler.rollback(); } } @@ -150,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 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 f140d18b3..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,30 +44,21 @@ 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, ActionForm actionForm) { - ServletActionContext saContext = (ServletActionContext) context; HttpServletRequest request = saContext.getRequest(); - if (actionForm != null) { - actionForm.reset((ActionMapping) actionConfig, request); - } - - // Set the multipart class - if (actionConfig.getMultipartClass() != null) { - saContext.getRequestScope().put(Globals.MULTIPART_KEY, - actionConfig.getMultipartClass()); - } + actionForm.reset((ActionMapping) actionConfig, request); } // ---------------------------------------------------------- Helper Methods @@ -148,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 7977c765b..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,70 +438,49 @@ public static void populate(Object bean, String prefix, String suffix, // Map for multipart parameters Map multipartParameters = null; - boolean isMultipart = false; + final boolean isMultipart = multipartHandler != null; if (bean instanceof ActionForm) { ((ActionForm) bean).setMultipartRequestHandler(null); } - MultipartRequestHandler multipartHandler = null; - if (isRequestMultipartPost(request)) { + if (isMultipart) { // Get the ActionServletWrapper from the form bean ActionServletWrapper servlet = null; if (bean instanceof ActionForm) { servlet = ((ActionForm) bean).getServletWrapper(); - } else if (bean != null) { + } else { throw new ServletException("bean that's supposed to be " + "populated from a multipart request is not of type " + "\"org.apache.struts.action.ActionForm\", but type " + "\"" + bean.getClass().getName() + "\""); } - // Obtain a MultipartRequestHandler - multipartHandler = getMultipartHandler(request); + // Set servlet and mapping info + servlet.setServletFor(multipartHandler); - if (multipartHandler != null) { - isMultipart = true; + //stop here if the maximum length has been exceeded + Boolean maxLengthExceeded = + (Boolean) request.getAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED); - multipartHandler.setMapping((ActionMapping) request - .getAttribute(Globals.MAPPING_KEY)); - - // Initialize multipart request class handler - multipartHandler.handleRequest(request); - - if (servlet == null) { - multipartHandler.rollback(); - return; - } - - // Set servlet and mapping info - servlet.setServletFor(multipartHandler); - - //stop here if the maximum length has been exceeded - Boolean maxLengthExceeded = - (Boolean) request.getAttribute(MultipartRequestHandler.ATTRIBUTE_MAX_LENGTH_EXCEEDED); - - 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 ((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) { names = request.getParameterNames(); } - boolean isNotCancelled = true; while (names.hasMoreElements()) { String name = names.nextElement(); String stripped = name; @@ -498,25 +523,16 @@ public static void populate(Object bean, String prefix, String suffix, // such as 'org.apache.struts.action.CANCEL' if (!(stripped.startsWith("org.apache.struts."))) { properties.put(stripped, parameterValue); - } else - // Test the cancellation attribute - if (Globals.CANCEL_PROPERTY.equals(stripped) - || Globals.CANCEL_PROPERTY_X.equals(stripped)) { - - isNotCancelled = false; - break; } } // Set the corresponding properties of our bean try { - if (isNotCancelled) { - BeanUtils.populate(bean, properties); - } + BeanUtils.populate(bean, properties); } 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 @@ -1243,6 +1259,6 @@ public static boolean isRequestMultipartPost(HttpServletRequest request) { return contentType != null && contentType.startsWith("multipart/form-data") - && method.equalsIgnoreCase("POST"); + && "POST".equalsIgnoreCase(method); } } \ No newline at end of file diff --git a/src/site/xdoc/userGuide/release-notes.xml b/src/site/xdoc/userGuide/release-notes.xml index f2a1d76af..81a99cfac 100644 --- a/src/site/xdoc/userGuide/release-notes.xml +++ b/src/site/xdoc/userGuide/release-notes.xml @@ -97,6 +97,7 @@

          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: