From 835cc7cd096ba274ffd71e54115dbefc63cadffd Mon Sep 17 00:00:00 2001 From: Aleksandr Mashchenko Date: Sat, 15 Oct 2016 15:25:14 +0300 Subject: [PATCH] WW-4636 - File upload error message always in default language --- .../struts2/dispatcher/LocalizedMessage.java | 104 ++++++++++++++++++ .../multipart/JakartaMultiPartRequest.java | 13 ++- .../JakartaStreamMultiPartRequest.java | 13 ++- .../multipart/MultiPartRequest.java | 5 +- .../multipart/MultiPartRequestWrapper.java | 13 ++- .../interceptor/FileUploadInterceptor.java | 6 +- .../FileUploadInterceptorTest.java | 40 +++++++ .../multipart/PellMultiPartRequest.java | 3 +- 8 files changed, 175 insertions(+), 22 deletions(-) create mode 100644 core/src/main/java/org/apache/struts2/dispatcher/LocalizedMessage.java diff --git a/core/src/main/java/org/apache/struts2/dispatcher/LocalizedMessage.java b/core/src/main/java/org/apache/struts2/dispatcher/LocalizedMessage.java new file mode 100644 index 0000000000..9d2304fb32 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/LocalizedMessage.java @@ -0,0 +1,104 @@ +/* + * $Id$ + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.struts2.dispatcher; + +import java.util.Arrays; + +public class LocalizedMessage { + private final Class clazz; + private final String textKey; + private final String defaultMessage; + private final Object[] args; + + public LocalizedMessage(Class clazz, String textKey, String defaultMessage, Object[] args) { + this.clazz = clazz; + this.textKey = textKey; + this.defaultMessage = defaultMessage; + this.args = args; + } + + public Class getClazz() { + return clazz; + } + + public String getTextKey() { + return textKey; + } + + public String getDefaultMessage() { + return defaultMessage; + } + + public Object[] getArgs() { + return args; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + Arrays.hashCode(args); + result = prime * result + ((clazz == null) ? 0 : clazz.hashCode()); + result = prime * result + ((defaultMessage == null) ? 0 : defaultMessage.hashCode()); + result = prime * result + ((textKey == null) ? 0 : textKey.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + LocalizedMessage other = (LocalizedMessage) obj; + if (!Arrays.equals(args, other.args)) { + return false; + } + if (clazz == null) { + if (other.clazz != null) { + return false; + } + } else if (!clazz.equals(other.clazz)) { + return false; + } + if (defaultMessage == null) { + if (other.defaultMessage != null) { + return false; + } + } else if (!defaultMessage.equals(other.defaultMessage)) { + return false; + } + if (textKey == null) { + if (other.textKey != null) { + return false; + } + } else if (!textKey.equals(other.textKey)) { + return false; + } + return true; + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java index 643d73d237..32890c62c5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java @@ -34,6 +34,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; +import org.apache.struts2.dispatcher.LocalizedMessage; import javax.servlet.http.HttpServletRequest; import java.io.File; @@ -56,7 +57,7 @@ public class JakartaMultiPartRequest implements MultiPartRequest { protected Map> params = new HashMap<>(); // any errors while processing this request - protected List errors = new ArrayList<>(); + protected List errors = new ArrayList<>(); protected long maxSize; private Locale defaultLocale = Locale.ENGLISH; @@ -85,7 +86,7 @@ public void parse(HttpServletRequest request, String saveDir) throws IOException processUpload(request, saveDir); } catch (FileUploadException e) { LOG.warn("Request exceeded size limit!", e); - String errorMessage = null; + LocalizedMessage errorMessage = null; if(e instanceof FileUploadBase.SizeLimitExceededException) { FileUploadBase.SizeLimitExceededException ex = (FileUploadBase.SizeLimitExceededException) e; @@ -99,7 +100,7 @@ public void parse(HttpServletRequest request, String saveDir) throws IOException } } catch (Exception e) { LOG.warn("Unable to parse request", e); - String errorMessage = buildErrorMessage(e, new Object[]{}); + LocalizedMessage errorMessage = buildErrorMessage(e, new Object[]{}); if (!errors.contains(errorMessage)) { errors.add(errorMessage); } @@ -112,10 +113,10 @@ protected void setLocale(HttpServletRequest request) { } } - protected String buildErrorMessage(Throwable e, Object[] args) { + protected LocalizedMessage buildErrorMessage(Throwable e, Object[] args) { String errorKey = "struts.messages.upload.error." + e.getClass().getSimpleName(); LOG.debug("Preparing error message for key: [{}]", errorKey); - return LocalizedTextUtil.findText(this.getClass(), errorKey, defaultLocale, e.getMessage(), args); + return new LocalizedMessage(this.getClass(), errorKey, e.getMessage(), args); } protected void processUpload(HttpServletRequest request, String saveDir) throws FileUploadException, UnsupportedEncodingException { @@ -315,7 +316,7 @@ public String[] getParameterValues(String name) { /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors() */ - public List getErrors() { + public List getErrors() { return errors; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java index f4c4665ad7..b1aa563db1 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java @@ -12,6 +12,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.StrutsConstants; +import org.apache.struts2.dispatcher.LocalizedMessage; import javax.servlet.http.HttpServletRequest; import java.io.*; @@ -48,7 +49,7 @@ public class JakartaStreamMultiPartRequest implements MultiPartRequest { /** * Internal list of raised errors to be passed to the the Struts2 framework. */ - private List errors = new ArrayList<>(); + private List errors = new ArrayList<>(); /** * Internal list of non-critical messages to be passed to the Struts2 framework. @@ -130,7 +131,7 @@ public String[] getContentType(String fieldName) { /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors() */ - public List getErrors() { + public List getErrors() { return errors; } @@ -239,7 +240,7 @@ public void parse(HttpServletRequest request, String saveDir) throws IOException processUpload(request, saveDir); } catch (Exception e) { LOG.warn("Error occurred during parsing of multi part request", e); - String errorMessage = buildErrorMessage(e, new Object[]{}); + LocalizedMessage errorMessage = buildErrorMessage(e, new Object[]{}); if (!errors.contains(errorMessage)) { errors.add(errorMessage); } @@ -349,7 +350,7 @@ private long getRequestSize(HttpServletRequest request) { private void addFileSkippedError(String fileName, HttpServletRequest request) { String exceptionMessage = "Skipped file " + fileName + "; request size limit exceeded."; FileSizeLimitExceededException exception = new FileUploadBase.FileSizeLimitExceededException(exceptionMessage, getRequestSize(request), maxSize); - String message = buildErrorMessage(exception, new Object[]{fileName, getRequestSize(request), maxSize}); + LocalizedMessage message = buildErrorMessage(exception, new Object[]{fileName, getRequestSize(request), maxSize}); if (!errors.contains(message)) { errors.add(message); } @@ -507,10 +508,10 @@ private String getCanonicalName(String fileName) { * @param args arguments * @return error message */ - private String buildErrorMessage(Throwable e, Object[] args) { + private LocalizedMessage buildErrorMessage(Throwable e, Object[] args) { String errorKey = "struts.message.upload.error." + e.getClass().getSimpleName(); LOG.debug("Preparing error message for key: [{}]", errorKey); - return LocalizedTextUtil.findText(this.getClass(), errorKey, defaultLocale, e.getMessage(), args); + return new LocalizedMessage(this.getClass(), errorKey, e.getMessage(), args); } /** diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java index 52e415e922..a230a60dfc 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java @@ -22,6 +22,9 @@ package org.apache.struts2.dispatcher.multipart; import javax.servlet.http.HttpServletRequest; + +import org.apache.struts2.dispatcher.LocalizedMessage; + import java.io.File; import java.io.IOException; import java.util.Enumeration; @@ -112,7 +115,7 @@ public interface MultiPartRequest { * * @return a list of Strings that represent various errors during parsing */ - public List getErrors(); + public List getErrors(); /** * Cleans up all uploaded file, should be called at the end of request diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java index 160664ba1e..05798e3e9d 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java @@ -25,6 +25,7 @@ import com.opensymphony.xwork2.util.LocalizedTextUtil; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.dispatcher.LocalizedMessage; import org.apache.struts2.dispatcher.StrutsRequestWrapper; import javax.servlet.http.HttpServletRequest; @@ -60,7 +61,7 @@ public class MultiPartRequestWrapper extends StrutsRequestWrapper { protected static final Logger LOG = LogManager.getLogger(MultiPartRequestWrapper.class); - private Collection errors; + private Collection errors; private MultiPartRequest multi; private Locale defaultLocale = Locale.ENGLISH; @@ -83,7 +84,7 @@ public MultiPartRequestWrapper(MultiPartRequest multiPartRequest, HttpServletReq setLocale(request); try { multi.parse(request, saveDir); - for (String error : multi.getErrors()) { + for (LocalizedMessage error : multi.getErrors()) { addError(error); } } catch (IOException e) { @@ -102,10 +103,10 @@ protected void setLocale(HttpServletRequest request) { } } - protected String buildErrorMessage(Throwable e, Object[] args) { + protected LocalizedMessage buildErrorMessage(Throwable e, Object[] args) { String errorKey = "struts.messages.upload.error." + e.getClass().getSimpleName(); LOG.debug("Preparing error message for key: [{}]", errorKey); - return LocalizedTextUtil.findText(this.getClass(), errorKey, defaultLocale, e.getMessage(), args); + return new LocalizedMessage(this.getClass(), errorKey, e.getMessage(), args); } /** @@ -234,7 +235,7 @@ public boolean hasErrors() { * * @return the error Collection. */ - public Collection getErrors() { + public Collection getErrors() { return errors; } @@ -243,7 +244,7 @@ public Collection getErrors() { * * @param anErrorMessage the error message to report. */ - protected void addError(String anErrorMessage) { + protected void addError(LocalizedMessage anErrorMessage) { if (!errors.contains(anErrorMessage)) { errors.add(anErrorMessage); } diff --git a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java index 680f8279d6..be97a315bf 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java @@ -26,10 +26,12 @@ import com.opensymphony.xwork2.inject.Inject; import com.opensymphony.xwork2.interceptor.AbstractInterceptor; import com.opensymphony.xwork2.interceptor.ValidationAware; +import com.opensymphony.xwork2.util.LocalizedTextUtil; import com.opensymphony.xwork2.util.TextParseUtil; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.ServletActionContext; +import org.apache.struts2.dispatcher.LocalizedMessage; import org.apache.struts2.dispatcher.Parameter; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; import org.apache.struts2.util.ContentTypeMatcher; @@ -256,9 +258,9 @@ public String intercept(ActionInvocation invocation) throws Exception { MultiPartRequestWrapper multiWrapper = (MultiPartRequestWrapper) request; if (multiWrapper.hasErrors()) { - for (String error : multiWrapper.getErrors()) { + for (LocalizedMessage error : multiWrapper.getErrors()) { if (validation != null) { - validation.addActionError(error); + validation.addActionError(LocalizedTextUtil.findText(error.getClazz(), error.getTextKey(), ActionContext.getContext().getLocale(), error.getDefaultMessage(), error.getArgs())); } } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java index c7cb2368d9..6668b4c682 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java @@ -41,8 +41,10 @@ import java.io.IOException; import java.net.URI; import java.net.URL; +import java.util.Collection; import java.util.HashMap; import java.util.List; +import java.util.Locale; import java.util.Map; @@ -335,6 +337,44 @@ public void testMultipleAccept() throws Exception { assertNotNull("test1.html", fileRealFilenames[0]); } + public void testMultipartRequestLocalizedError() throws Exception { + MockHttpServletRequest req = new MockHttpServletRequest(); + req.setCharacterEncoding("text/html"); + req.addHeader("Content-type", "multipart/form-data; boundary=---1234"); + + // inspired by the unit tests for jakarta commons fileupload + String content = ("-----1234\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename=\"deleteme.txt\"\r\n" + + "Content-Type: text/html\r\n" + + "\r\n" + + "Unit test of FileUploadInterceptor" + + "\r\n" + + "-----1234--\r\n"); + req.setContent(content.getBytes("US-ASCII")); + + MyFileupAction action = new MyFileupAction(); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + Map param = new HashMap<>(); + ActionContext.getContext().setParameters(HttpParameters.create(param).build()); + // set German locale + ActionContext.getContext().setLocale(Locale.GERMAN); + ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 10)); + + interceptor.intercept(mai); + + assertTrue(action.hasActionErrors()); + + Collection errors = action.getActionErrors(); + assertEquals(1, errors.size()); + String msg = errors.iterator().next(); + // the error message should contain at least this test + assertTrue(msg.startsWith("Der Request übertraf die maximal erlaubte Größe")); + } + private String encodeTextFile(String bondary, String endline, String name, String filename, String contentType, String content) { final StringBuilder sb = new StringBuilder(64); sb.append(endline); diff --git a/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java b/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java index c18277c601..4a5f7c0f1b 100644 --- a/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java +++ b/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java @@ -27,6 +27,7 @@ import org.apache.logging.log4j.LogManager; import http.utils.multipartrequest.ServletMultipartRequest; import org.apache.struts2.StrutsConstants; +import org.apache.struts2.dispatcher.LocalizedMessage; import javax.servlet.http.HttpServletRequest; import java.io.File; @@ -131,7 +132,7 @@ public String[] getParameterValues(String name) { return values.toArray(new String[values.size()]); } - public List getErrors() { + public List getErrors() { return Collections.emptyList(); }