Bug 62971 - Revert the fix for 49464 in tomcat 9
Summary: Revert the fix for 49464 in tomcat 9
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.13
Hardware: PC All
: P2 normal (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-02 08:37 UTC by quaff
Modified: 2020-01-16 18:16 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description quaff 2018-12-02 08:37:53 UTC
I encountered weird issue and I found this
http://tomcat.10.x6.nabble.com/Tomcat-8-5-19-corrupts-static-text-files-encoded-with-UTF-8-td5065877.html
Tomcat introduce such regress in 8.5.19 and reverted it soon in 8.5.20, but this still exists in tomcat 9.0, and not documented in migration guide
https://tomcat.apache.org/migration-9.html
I think it's bad to assume file encoding as Charset.defaultCharset(), especially on windows, normally system is not utf-8 encoding, and files like js are utf-8, if non-ascii characters included, it parse failed by browser.
The change resolved some edge cases but bring a big regression issue, if you insist on it, please update the migration guide.
Comment 1 Remy Maucherat 2018-12-02 11:11:49 UTC

*** This bug has been marked as a duplicate of bug 49464 ***
Comment 2 fuminzhou 2020-01-13 20:55:26 UTC
The fix to bug 49464 potentially missed a scenario in its consideration, and may have introduced a "new" bug. However, we have not been able to create an isolated test scenario to demonstrate such yet.

In comments to bug 49464, it listed 3 scenarios considered: "
There are three scenarios to consider:
a) directly returning a file
b) including a file into an output stream
c) including a file into a writer".

However, a fourth scenario <c:import...> for standard tag library core appears to be impacted.

To my understanding, if a jsp contains a:
<c:import var="xslt" url="/sample.xsl"/>
it is supposed to populate the content from that file "sample.xsl" into the "variable" "xslt", for later code to use.

The issue behavior that we (in certain conditions) observed, is that the "variable" "xslt" would become empty, yet the jsp or the result html would receive the content of "sample.xsl" as part of its own content, making it behaving as if it is a "include" tag instead of an import tag.

We have not identified the exact conditions that would trigger such behavior yet - thus no isolated test case created to demonstrate such yet.  But since the issue behavior looks to match the comments of the bug/fix for 49464, I am adding this comment to start the communication. - We will update when/if we have more specific info.

We are using tomcat 9.0.26
Comment 3 Mark Thomas 2020-01-14 20:13:18 UTC
This sounds more like a concurrency issue with the tag library.

Waiting for more information on how to reproduce this...
Comment 4 fuminzhou 2020-01-15 16:05:58 UTC
I am able to have an isolated case to reproduce the "bug" behavior now.
Still need to clean up a bit more before I submit.  But a quick update: previously in the scenario I couldn't reproduce the "bug", appears to be because locally a newer version of taglib was in effect. Removing the taglibs-standard-impl-1.2.5.jar and taglibs-standard-spec-1.2.5.jar, and let the javax.servlet.jsp.jstl-1.2.3.jar and javax.servlet.jsp.jstl-api-1.2.1.jar to take effect, would cause the behavior to appear.  - Well, I guess it could be said as a javax.servlet.jsp.jstl-1.2.3.jar bug...
-A quick update anyway.
Comment 5 fuminzhou 2020-01-15 16:29:12 UTC
Also it appears to require struts(1) action/tiles to get it happen - a simple jsp with c:import does not show the issue behavior.
Comment 6 fuminzhou 2020-01-15 20:16:17 UTC
It turns out the issue behavior is not related to struts/tiles/action. I am able to develop a simple jsp that encounter the issue - still want to trim down further before I submit the pacakge.  But it does require to use tag lib http://java.sun.com/jsp/jstl/fmt, with javax.servlet.jsp.jstl-1.2.3.jar.

In debug, it looks the fmt:setLocale would cause an CharacterEncoding (for en-US) to be set on the Response, which in turn, at the code for bug 49464, would cause:
outputEncoding != org.apache.coyote.Constants.DEFAULT_BODY_CHARSET.name()
to be true, which triggers the outputEncodingSpecified pw.flush to happen.

and with the ImportSupport from javax.servlet.jsp.jstl-1.2.3.jar looks not able to deal with the flushed irw, while the taglibs-standard-impl-1.2.5 appears to be able to handle it - these I have not debugged into.
Comment 7 fuminzhou 2020-01-15 20:40:17 UTC
Note that in debug that encountered the problem (in my particular case anyway), in the DefaultServlet.java, even when:
outputEncoding != org.apache.coyote.Constants.DEFAULT_BODY_CHARSET.name()
becomes true, the contents of both sides are actually the same.  -This and the flush looks to be the logic for the 49464 fix.

Which makes this fix suspicious.  - Locally changing the "!=" to ".equals(..)" would make the issue go away.
Comment 8 fuminzhou 2020-01-16 12:56:45 UTC
A simple jsp below could demonstrate the issue behavior - when it is used with javax.servlet.jsp.jstl-1.2.3.jar (and javax.servlet.jsp.jstl-api-1.2.1.jar) instead of taglibs-standard-impl-1.2.5.jar and taglibs-standard-spec-1.2.5.jar.  I have also created a simple webapp with such. - Some environment factors might play - I am using a Windows 10 pro, with en-US/Eastern Time zone, which might cause the non-constant CharacterEncoding to be set on the Response.

But now I am wondering maybe I should create a separate bug instead of reopen this one - so that we could move forward instead of considering rolling it back - what do you think?

<!--begin of jsp>
<%@ taglib uri="http://java.sun.com/jsp/jstl/core" prefix="c" %>
<%@ taglib uri="http://java.sun.com/jsp/jstl/fmt" prefix="fmt" %>

<fmt:setLocale value='' scope="session"/>

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<title>ShowBug</title>
</head>
<body>
    <div id="xsltnot"> c:import is not expected to get the contents in between -<br>
    	---------------------------------------------<br>
    	<c:import var="xslt" url="/sample.txt"/><br>
    	---------------------------------------------<br>
    </div>
    <br>
    <br>
    <div id="xslt"> Instead, we expect the c:import to get the contents into the variable, which should be displayed in between *<br>
    ********************************************<br>
    	<c:out value="${xslt}" escapeXml="true"/><br>
    ********************************************<br>
    </div>
</body>
</html>
<!-- end of jsp>
Comment 9 Mark Thomas 2020-01-16 17:10:11 UTC
This situation you describe seems to be wholly unrelated to this bug.

I recommend opening a new bug.
Comment 10 fuminzhou 2020-01-16 18:16:46 UTC
Thanks for the recommendation. New Bug 64081 reported. Closing this one.