[
https://issues.apache.org/jira/browse/WW-4971?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16672090#comment-16672090
]
ASF GitHub Bot commented on WW-4971:
------------------------------------
aleksandr-m closed pull request #257: Proposed fix for WW-4971 (broken includes
for non-UTF8 content).
URL: https://github.com/apache/struts/pull/257
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java
b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index 81a4f803a..f7863050c 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 @@
/** 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 @@
/** 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 @@
/** 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 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 @@
/** 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 @@
/** 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 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 aa58ade62..616aa18c9 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 @@
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.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 @@
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 void setDefaultEncoding(String encoding) {
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 boolean end(Writer writer, String body) {
// 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 static String getContextRelativePath(ServletRequest
request, String relat
}
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 static void include( String relativePath, Writer
writer, ServletRequest r
// 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 PageResponse(HttpServletResponse response) {
* @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 fd75c46a1..6c593fd2f 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 @@
*
*/
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 @@ private static void decodeAndWriteOut(Writer writer, byte[]
bytes, int length, B
// 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 @@ private static void decodeAndWriteBuffered(Writer writer,
ByteBuffer in, CharBuf
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 @@ private static void decodeAndWriteBuffered(Writer writer,
ByteBuffer in, CharBuf
// 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 6b9179203..45830854f 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 void testNoURL() throws Exception {
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 void testIncludeNoParam() throws Exception {
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 void testIncludeWithParameters() throws Exception {
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 void testIncludeRelative2Dots() throws Exception {
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());
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
> s:include tag fails with truncated content in certain circumstances
> -------------------------------------------------------------------
>
> Key: WW-4971
> URL: https://issues.apache.org/jira/browse/WW-4971
> Project: Struts 2
> Issue Type: Bug
> Components: Core Tags
> Affects Versions: 2.3.36, 2.5.18
> Environment: Windows 10, Java 7/8 (but issue isn't environment
> specific)
> Reporter: James Chaplin
> Priority: Major
> Fix For: 2.6, 2.5.19
>
> Attachments: WW4791_Reproducer.war
>
>
> Hello Apache Struts Team.
> There is an issue with the Struts include tag (s:include) when processing
> includes on a page that isn't using UTF-8 encoding (e.g. ISO-8859-1 or
> Windows-1252 page encoding).
> In some circumstances the s:include tag results in truncated content from the
> child page (i.e. the parent page including the child page via s:include
> experiences incomplete rendering of the included content). This happens when
> the included page contains certain characters (e.g. 'ç') in a non-UTF8
> encoding (whether directly or from a resource bundle).
> There are no warnings produced in the logs (even in debug mode), so the issue
> can only be detected visually when things fail.
> Changing all the content to UTF-8 is a workaround, but that is not feasible
> in all circumstances. Given the preceding and the lack of warnings I'm
> initially submitting it as a major priority.
> I will attempt to submit a bugfix for consideration shortly.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
