This is an automated email from the ASF dual-hosted git repository.
amashchenko pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 9fa53dddb92c6eea59396d9505c467acb1d74d97
Author: Aleksandr Mashchenko <aleksand...@users.noreply.github.com>
AuthorDate: Thu Nov 1 21:53:04 2018 +0200
Merge pull request #257 from JCgH4164838Gh792C124B5/localS2_2_5_x_Branch
Fix WW-4971 (broken includes for non-UTF8 content).
---
.../java/org/apache/struts2/StrutsConstants.java | 65 +++++++++++-----------
.../org/apache/struts2/components/Include.java | 36 +++++++++---
.../struts2/util/FastByteArrayOutputStream.java | 24 ++++++--
.../apache/struts2/views/jsp/IncludeTagTest.java | 60 +++++++++++++++++++-
4 files changed, 138 insertions(+), 47 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index 81a4f80..f786305 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -41,12 +41,15 @@ public final class StrutsConstants {
/** The URL extension to use to determine if the request is meant for a
Struts action */
public static final String STRUTS_ACTION_EXTENSION =
"struts.action.extension";
- /** Comma separated list of patterns (java.util.regex.Pattern) to be
excluded from Struts2-processing */
- public static final String STRUTS_ACTION_EXCLUDE_PATTERN =
"struts.action.excludePattern";
+ /** Comma separated list of patterns (java.util.regex.Pattern) to be
excluded from Struts2-processing */
+ public static final String STRUTS_ACTION_EXCLUDE_PATTERN =
"struts.action.excludePattern";
- /** Whether to use the alterative syntax for the tags or not */
+ /** Whether to use the alternative syntax for the tags or not */
public static final String STRUTS_TAG_ALTSYNTAX = "struts.tag.altSyntax";
+ /** Whether to use the response encoding (JSP page encoding) for s:include
tag processing (false - use STRUTS_I18N_ENCODING - by default) */
+ public static final String STRUTS_TAG_INCLUDETAG_USERESPONSEENCODING =
"struts.tag.includetag.useResponseEncoding";
+
/** The HTTP port used by Struts URLs */
public static final String STRUTS_URL_HTTP_PORT = "struts.url.http.port";
@@ -56,7 +59,7 @@ public final class StrutsConstants {
/** The default includeParams method to generate Struts URLs */
public static final String STRUTS_URL_INCLUDEPARAMS =
"struts.url.includeParams";
- public static final String STRUTS_URL_RENDERER = "struts.urlRenderer";
+ public static final String STRUTS_URL_RENDERER = "struts.urlRenderer";
/** The com.opensymphony.xwork2.ObjectFactory implementation class */
public static final String STRUTS_OBJECTFACTORY = "struts.objectFactory";
@@ -91,7 +94,7 @@ public final class StrutsConstants {
/** The org.apache.struts2.views.freemarker.FreemarkerManager
implementation class */
public static final String STRUTS_FREEMARKER_MANAGER_CLASSNAME =
"struts.freemarker.manager.classname";
- /** Update freemarker templates cache in seconds*/
+ /** Update freemarker templates cache in seconds */
public static final String STRUTS_FREEMARKER_TEMPLATES_CACHE_UPDATE_DELAY
= "struts.freemarker.templatesCache.updateDelay";
/** Cache model instances at BeanWrapper level */
@@ -220,12 +223,12 @@ public final class StrutsConstants {
public static final String STRUTS_LOCALE_PROVIDER_FACTORY =
"struts.localeProviderFactory";
/** The name of the parameter to create when mapping an id (used by some
action mappers) */
- public static final String STRUTS_ID_PARAMETER_NAME =
"struts.mapper.idParameterName";
-
- /** The name of the parameter to determine whether static method access
will be allowed in OGNL expressions or not */
- public static final String STRUTS_ALLOW_STATIC_METHOD_ACCESS =
"struts.ognl.allowStaticMethodAccess";
+ public static final String STRUTS_ID_PARAMETER_NAME =
"struts.mapper.idParameterName";
+
+ /** The name of the parameter to determine whether static method access
will be allowed in OGNL expressions or not */
+ public static final String STRUTS_ALLOW_STATIC_METHOD_ACCESS =
"struts.ognl.allowStaticMethodAccess";
- /** The com.opensymphony.xwork2.validator.ActionValidatorManager
implementation class */
+ /** The com.opensymphony.xwork2.validator.ActionValidatorManager
implementation class */
public static final String STRUTS_ACTIONVALIDATORMANAGER =
"struts.actionValidatorManager";
/** The {@link com.opensymphony.xwork2.util.ValueStackFactory}
implementation class */
@@ -236,7 +239,7 @@ public final class StrutsConstants {
/** The {@link
com.opensymphony.xwork2.util.reflection.ReflectionContextFactory}
implementation class */
public static final String STRUTS_REFLECTIONCONTEXTFACTORY =
"struts.reflectionContextFactory";
-
+
/** The {@link com.opensymphony.xwork2.util.PatternMatcher} implementation
class */
public static final String STRUTS_PATTERNMATCHER = "struts.patternMatcher";
@@ -246,32 +249,32 @@ public final class StrutsConstants {
/** The {@link com.opensymphony.xwork2.UnknownHandlerManager}
implementation class */
public static final String STRUTS_UNKNOWN_HANDLER_MANAGER =
"struts.unknownHandlerManager";
- /** Throw RuntimeException when a property is not found, or the evaluation
of the espression fails*/
+ /** Throw RuntimeException when a property is not found, or the evaluation
of the expression fails */
public static final String STRUTS_EL_THROW_EXCEPTION =
"struts.el.throwExceptionOnFailure";
- /** Logs properties that are not found (very verbose) **/
+ /** Logs properties that are not found (very verbose) */
public static final String STRUTS_LOG_MISSING_PROPERTIES =
"struts.ognl.logMissingProperties";
- /** Enables caching of parsed OGNL expressions **/
+ /** Enables caching of parsed OGNL expressions */
public static final String STRUTS_ENABLE_OGNL_EXPRESSION_CACHE =
"struts.ognl.enableExpressionCache";
- /** Enables evaluation of OGNL expressions **/
+ /** Enables evaluation of OGNL expressions */
public static final String STRUTS_ENABLE_OGNL_EVAL_EXPRESSION =
"struts.ognl.enableOGNLEvalExpression";
- /** Disables {@link org.apache.struts2.dispatcher.StrutsRequestWrapper}
request attribute value stack lookup (JSTL accessibility) **/
+ /** Disables {@link org.apache.struts2.dispatcher.StrutsRequestWrapper}
request attribute value stack lookup (JSTL accessibility) */
public static final String
STRUTS_DISABLE_REQUEST_ATTRIBUTE_VALUE_STACK_LOOKUP =
"struts.disableRequestAttributeValueStackLookup";
- /** The{@link org.apache.struts2.views.util.UrlHelper} implementation
class **/
+ /** The{@link org.apache.struts2.views.util.UrlHelper} implementation
class */
public static final String STRUTS_URL_HELPER = "struts.view.urlHelper";
- /** {@link com.opensymphony.xwork2.conversion.impl.XWorkBasicConverter} **/
+ /** {@link com.opensymphony.xwork2.conversion.impl.XWorkBasicConverter} */
public static final String STRUTS_CONVERTER_COLLECTION =
"struts.converter.collection";
public static final String STRUTS_CONVERTER_ARRAY =
"struts.converter.array";
public static final String STRUTS_CONVERTER_DATE = "struts.converter.date";
public static final String STRUTS_CONVERTER_NUMBER =
"struts.converter.number";
public static final String STRUTS_CONVERTER_STRING =
"struts.converter.string";
- /** Enable handling exceptions by Dispatcher - true by default **/
+ /** Enable handling exceptions by Dispatcher - true by default */
public static final String STRUTS_HANDLE_EXCEPTION =
"struts.handle.exception";
public static final String STRUTS_CONVERTER_PROPERTIES_PROCESSOR =
"struts.converter.properties.processor";
@@ -282,42 +285,42 @@ public final class StrutsConstants {
public static final String STRUTS_EXPRESSION_PARSER =
"struts.expression.parser";
- /** namespaces names' whitelist **/
+ /** Namespace names' whitelist */
public static final String STRUTS_ALLOWED_NAMESPACE_NAMES =
"struts.allowed.namespace.names";
- /** default namespace name to use when namespace didn't match the
whitelist **/
+ /** Default namespace name to use when namespace didn't match the
whitelist */
public static final String STRUTS_DEFAULT_NAMESPACE_NAME =
"struts.default.namespace.name";
- /** actions names' whitelist **/
+ /** Action names' whitelist */
public static final String STRUTS_ALLOWED_ACTION_NAMES =
"struts.allowed.action.names";
- /** default action name to use when action didn't match the whitelist **/
+ /** Default action name to use when action didn't match the whitelist */
public static final String STRUTS_DEFAULT_ACTION_NAME =
"struts.default.action.name";
- /** methods names' whitelist **/
+ /** Method names' whitelist */
public static final String STRUTS_ALLOWED_METHOD_NAMES =
"struts.allowed.method.names";
- /** default method name to use when method didn't match the whitelist **/
+ /** Default method name to use when method didn't match the whitelist */
public static final String STRUTS_DEFAULT_METHOD_NAME =
"struts.default.method.name";
- /** enables action: prefix **/
+ /** Enables action: prefix */
public static final String STRUTS_MAPPER_ACTION_PREFIX_ENABLED =
"struts.mapper.action.prefix.enabled";
- /** enables access to actions in other namespaces than current with
action: prefix **/
+ /** Enables access to actions in other namespaces than current with
action: prefix */
public static final String STRUTS_MAPPER_ACTION_PREFIX_CROSSNAMESPACES =
"struts.mapper.action.prefix.crossNamespaces";
public static final String DEFAULT_TEMPLATE_TYPE_CONFIG_KEY =
"struts.ui.templateSuffix";
- /** Allows override default DispatcherErrorHandler **/
+ /** Allows override default DispatcherErrorHandler */
public static final String STRUTS_DISPATCHER_ERROR_HANDLER =
"struts.dispatcher.errorHandler";
- /** Comma delimited set of excluded classes and package names which cannot
be accessed via expressions **/
+ /** Comma delimited set of excluded classes and package names which cannot
be accessed via expressions */
public static final String STRUTS_EXCLUDED_CLASSES =
"struts.excludedClasses";
public static final String STRUTS_EXCLUDED_PACKAGE_NAME_PATTERNS =
"struts.excludedPackageNamePatterns";
public static final String STRUTS_EXCLUDED_PACKAGE_NAMES =
"struts.excludedPackageNames";
- /** Dedicated services to check if passed string is excluded/accepted **/
+ /** Dedicated services to check if passed string is excluded/accepted */
public static final String STRUTS_EXCLUDED_PATTERNS_CHECKER =
"struts.excludedPatterns.checker";
public static final String STRUTS_ACCEPTED_PATTERNS_CHECKER =
"struts.acceptedPatterns.checker";
- /** Constant is used to override framework's default excluded patterns **/
+ /** Constant is used to override framework's default excluded patterns */
public static final String STRUTS_OVERRIDE_EXCLUDED_PATTERNS =
"struts.override.excludedPatterns";
public static final String STRUTS_OVERRIDE_ACCEPTED_PATTERNS =
"struts.override.acceptedPatterns";
diff --git a/core/src/main/java/org/apache/struts2/components/Include.java
b/core/src/main/java/org/apache/struts2/components/Include.java
index aa58ade..616aa18 100644
--- a/core/src/main/java/org/apache/struts2/components/Include.java
+++ b/core/src/main/java/org/apache/struts2/components/Include.java
@@ -20,6 +20,7 @@ package org.apache.struts2.components;
import com.opensymphony.xwork2.inject.Inject;
import com.opensymphony.xwork2.util.ValueStack;
+
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.apache.struts2.RequestUtils;
@@ -35,6 +36,7 @@ import javax.servlet.ServletRequest;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.http.HttpServletResponseWrapper;
+
import java.io.*;
import java.net.URLEncoder;
import java.util.*;
@@ -90,17 +92,19 @@ public class Include extends Component {
private static final Logger LOG = LogManager.getLogger(Include.class);
- private static String systemEncoding = System.getProperty("file.encoding");
+ private static final String systemEncoding =
System.getProperty("file.encoding");
protected String value;
private HttpServletRequest req;
private HttpServletResponse res;
- private static String defaultEncoding;
+ private String defaultEncoding; // Made non-static (during WW-4971
fix)
+ private boolean useResponseEncoding; // Added with WW-4971 fix (allows
switch between usage of response or default encoding)
public Include(ValueStack stack, HttpServletRequest req,
HttpServletResponse res) {
super(stack);
this.req = req;
this.res = res;
+ useResponseEncoding = false; // By default use defaultEncoding (vs.
response/page encoding)
}
@Inject(StrutsConstants.STRUTS_I18N_ENCODING)
@@ -108,9 +112,25 @@ public class Include extends Component {
defaultEncoding = encoding;
}
+ @Inject(value = StrutsConstants.STRUTS_TAG_INCLUDETAG_USERESPONSEENCODING,
required=false)
+ public void setUseResponseEncoding(String useEncoding) {
+ useResponseEncoding = Boolean.parseBoolean(useEncoding);
+ }
+
public boolean end(Writer writer, String body) {
String page = findString(value, "value", "You must specify the URL to
include. Example: /foo.jsp");
StringBuilder urlBuf = new StringBuilder();
+ String encodingForInclude;
+
+ if (useResponseEncoding) {
+ encodingForInclude = res.getCharacterEncoding(); // Use response
(page) encoding
+ if (encodingForInclude == null || encodingForInclude.length() ==
0) {
+ encodingForInclude = defaultEncoding; // Revert to
defaultEncoding when response (page) encoding is invalid
+ }
+ }
+ else {
+ encodingForInclude = defaultEncoding; // Use default encoding
(when useResponseEncoding is false)
+ }
// Add URL
urlBuf.append(page);
@@ -147,7 +167,7 @@ public class Include extends Component {
// Include
try {
- include(result, writer, req, res, defaultEncoding);
+ include(result, writer, req, res, encodingForInclude);
} catch (ServletException | IOException e) {
LOG.warn("Exception thrown during include of {}", result, e);
}
@@ -209,7 +229,7 @@ public class Include extends Component {
}
public void addParameter(String key, Object value) {
- // don't use the default implementation of addParameter,
+ // Don't use the default implementation of addParameter,
// instead, include tag requires that each parameter be a list of
objects,
// just like the HTTP servlet interfaces are (String[])
if (value != null) {
@@ -256,7 +276,7 @@ public class Include extends Component {
// Use given encoding
pageResponse.getContent().writeTo(writer, encoding);
} else {
- //use the platform specific encoding
+ // Use the platform specific encoding
pageResponse.getContent().writeTo(writer, systemEncoding);
}
}
@@ -349,9 +369,9 @@ public class Include extends Component {
* @throws IOException
*/
public FastByteArrayOutputStream getContent() throws IOException {
- //if we are using a writer, we need to flush the
- //data to the underlying outputstream.
- //most containers do this - but it seems Jetty 4.0.5 doesn't
+ // If we are using a writer, we need to flush the
+ // data to the underlying outputstream.
+ // Most containers do this - but it seems Jetty 4.0.5 doesn't
if (pagePrintWriter != null) {
pagePrintWriter.flush();
}
diff --git
a/core/src/main/java/org/apache/struts2/util/FastByteArrayOutputStream.java
b/core/src/main/java/org/apache/struts2/util/FastByteArrayOutputStream.java
index fd75c46..6c593fd 100644
--- a/core/src/main/java/org/apache/struts2/util/FastByteArrayOutputStream.java
+++ b/core/src/main/java/org/apache/struts2/util/FastByteArrayOutputStream.java
@@ -18,6 +18,9 @@
*/
package org.apache.struts2.util;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
import javax.servlet.jsp.JspWriter;
import java.io.*;
import java.nio.ByteBuffer;
@@ -35,6 +38,9 @@ import java.util.LinkedList;
*
*/
public class FastByteArrayOutputStream extends OutputStream {
+
+ private static final Logger LOG =
LogManager.getLogger(FastByteArrayOutputStream.class);
+
private static final int DEFAULT_BLOCK_SIZE = 8192;
private LinkedList<byte[]> buffers;
@@ -144,7 +150,7 @@ public class FastByteArrayOutputStream extends OutputStream
{
// Append bytes to current buffer
// Previous data maybe partially decoded, this part will appended to
previous
in.put(bytes, 0, length);
- // To begin of data
+ // To begin processing of data
in.flip();
decodeAndWriteBuffered(writer, in, out, decoder, endOfInput);
}
@@ -158,7 +164,7 @@ public class FastByteArrayOutputStream extends OutputStream
{
if (in.hasRemaining()) {
// Move remaining to top of buffer
in.compact();
- if (result.isOverflow() && !result.isError() &&
!result.isMalformed()) {
+ if (result.isOverflow() && !result.isError()) { // isError
covers isMalformed and isUnmappable
// Not all buffer chars decoded, spin it again
// Set to begin
in.flip();
@@ -167,16 +173,24 @@ public class FastByteArrayOutputStream extends
OutputStream {
// Clean up buffer
in.clear();
}
- } while (in.hasRemaining() && result.isOverflow() && !result.isError()
&& !result.isMalformed());
+ } while (in.hasRemaining() && result.isOverflow() &&
!result.isError()); // isError covers isMalformed and isUnmappable
+
+ if (result.isError()) {
+ if (LOG.isWarnEnabled()) {
+ // Provide a log warning when the decoding fails (prior to
2.5.19 it failed silently).
+ // Note: Set FastByteArrayOutputStream's Logger level to error
or higher to suppress this log warning.
+ LOG.warn("Buffer decoding-in-to-out [{}] failed, coderResult
[{}]", decoder.charset().name(), result.toString());
+ }
+ }
}
private static CoderResult decodeAndWrite(Writer writer, ByteBuffer in,
CharBuffer out, CharsetDecoder decoder, boolean endOfInput) throws IOException {
CoderResult result = decoder.decode(in, out, endOfInput);
- // To begin of decoded data
+ // To begin processing of decoded data
out.flip();
// Output
writer.write(out.toString());
- // clear output to avoid infinitive loops, see WW-4383
+ // Clear output to avoid infinite loops, see WW-4383
out.clear();
return result;
}
diff --git
a/core/src/test/java/org/apache/struts2/views/jsp/IncludeTagTest.java
b/core/src/test/java/org/apache/struts2/views/jsp/IncludeTagTest.java
index 6b91792..4583085 100644
--- a/core/src/test/java/org/apache/struts2/views/jsp/IncludeTagTest.java
+++ b/core/src/test/java/org/apache/struts2/views/jsp/IncludeTagTest.java
@@ -51,7 +51,7 @@ public class IncludeTagTest extends AbstractTagTest {
public void testIncludeNoParam() throws Exception {
- // use always matcher as we can not determine the excact objects used
in mock.include(request, response) call
+ // Use always matcher as we can not determine the exact objects used
in mock.include(request, response) call
mockRequestDispatcher.include(anyObject(ServletRequest.class),
anyObject(ServletResponse.class));
expectLastCall().times(1);
@@ -69,7 +69,7 @@ public class IncludeTagTest extends AbstractTagTest {
public void testIncludeWithParameters() throws Exception {
- // use always matcher as we can not determine the excact objects used
in mock.include(request, response) call
+ // Use always matcher as we can not determine the exact objects used
in mock.include(request, response) call
mockRequestDispatcher.include(anyObject(ServletRequest.class),
anyObject(ServletResponse.class));
expectLastCall().times(1);
@@ -90,7 +90,7 @@ public class IncludeTagTest extends AbstractTagTest {
public void testIncludeRelative2Dots() throws Exception {
// TODO: we should test for .. in unit test - is this test correct?
- // use always matcher as we can not determine the exact objects used
in mock.include(request, response) call
+ // Use always matcher as we can not determine the exact objects used
in mock.include(request, response) call
mockRequestDispatcher.include(anyObject(ServletRequest.class),
anyObject(ServletResponse.class));
expectLastCall().times(1);
@@ -108,6 +108,60 @@ public class IncludeTagTest extends AbstractTagTest {
verify(mockRequestDispatcher);
}
+ public void testIncludeSetUseResponseEncodingTrue() throws Exception {
+ // TODO: If possible in future mock-test an actual content-includes
with various encodings
+ // while setting the response encoding to match. Doesn't appear to
be possible
+ // right now in unit-test form.
+ // Seems that the best we can do is verify the
setUseResponseEncoding() doesn't fail...
+
+ // Use always matcher as we can not determine the exact objects used
in mock.include(request, response) call
+ mockRequestDispatcher.include(anyObject(ServletRequest.class),
anyObject(ServletResponse.class));
+ expectLastCall().times(1);
+
+ replay(mockRequestDispatcher);
+
+ tag.setValue("person/create.jsp");
+ response.setCharacterEncoding("UTF-8");
+ tag.doStartTag();
+ // Manipulate after doStartTag to ensure the tag component has
undergone injection
+ Include include = (Include) tag.getComponent();
+ include.setUseResponseEncoding("true");
+ tag.doEndTag();
+
+ assertEquals("UTF-8", response.getCharacterEncoding());
+ assertEquals("/person/create.jsp",
request.getRequestDispatherString());
+ assertEquals("", writer.toString()); // Nothing gets written for
mock-include
+
+ verify(mockRequestDispatcher);
+ }
+
+ public void testIncludeSetUseResponseEncodingFalse() throws Exception {
+ // TODO: If possible in future mock-test an actual content-includes
with various encodings
+ // while setting the response encoding to match. Doesn't appear to
be possible
+ // right now in unit-test form.
+ // Seems that the best we can do is verify the
setUseResponseEncoding() doesn't fail...
+
+ // Use always matcher as we can not determine the exact objects used
in mock.include(request, response) call
+ mockRequestDispatcher.include(anyObject(ServletRequest.class),
anyObject(ServletResponse.class));
+ expectLastCall().times(1);
+
+ replay(mockRequestDispatcher);
+
+ tag.setValue("person/create.jsp");
+ response.setCharacterEncoding("UTF-8");
+ tag.doStartTag();
+ // Manipulate after doStartTag to ensure the tag component has
undergone injection
+ Include include = (Include) tag.getComponent();
+ include.setUseResponseEncoding("false");
+ tag.doEndTag();
+
+ assertEquals("UTF-8", response.getCharacterEncoding());
+ assertEquals("/person/create.jsp",
request.getRequestDispatherString());
+ assertEquals("", writer.toString()); // Nothing gets written for
mock-include
+
+ verify(mockRequestDispatcher);
+ }
+
protected void setUp() throws Exception {
super.setUp();
request.setupGetRequestDispatcher(new MockRequestDispatcher());
