Bug 28875

Created attachment 11496 [details]
The proposed patch to fix this bug in ErrorReportValve.

Created attachment 11497 [details]
The ErrorReportValve class file that applied the patch.

Kan,

I thought revision 1.11 of CoyoteResponse.java already fixed this issue.

CVS log:

revision 1.11
date: 2004/01/20 19:39:00;  author: luehe;  state: Exp;  lines: +8 -5
Fix for Bugtraq 4655010: Method sendError() of class HttpServletResponse
                         does not send multi byte data

Use response encoding when generating error report, so that the following
code works as expected:

      response.setCharacterEncoding(<charset>);
      response.sendError(<errorCode>, <message>);


Here are the diffs for that revision:

Index: CoyoteResponse.java
===================================================================
RCS file:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5/CoyoteResponse.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- CoyoteResponse.java 15 Oct 2003 18:47:49 -0000      1.10
+++ CoyoteResponse.java 20 Jan 2004 19:39:00 -0000      1.11
@@ -1,7 +1,7 @@
 /*
- * $Header:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5/CoyoteResponse.java,v
1.10 2003/10/15 18:47:49 jfarcand Exp $
- * $Revision: 1.10 $
- * $Date: 2003/10/15 18:47:49 $
+ * $Header:
/home/cvs/jakarta-tomcat-catalina/catalina/src/share/org/apache/coyote/tomcat5/CoyoteResponse.java,v
1.11 2004/01/20 19:39:00 luehe Exp $
+ * $Revision: 1.11 $
+ * $Date: 2004/01/20 19:39:00 $
  *
  * ====================================================================
  *
@@ -107,7 +107,7 @@
  *
  * @author Remy Maucherat
  * @author Craig R. McClanahan
- * @version $Revision: 1.10 $ $Date: 2003/10/15 18:47:49 $
+ * @version $Revision: 1.11 $ $Date: 2004/01/20 19:39:00 $
  */
 
 public class CoyoteResponse
@@ -529,9 +529,12 @@
      * unexpected exception thrown during the servlet processing
      * (and only in that case), null will be returned if the response stream
      * has already been used.
+     *
+     * @exception IOException if an input/output error occurs
      */
-    public PrintWriter getReporter() {
+    public PrintWriter getReporter() throws IOException {
         if (outputBuffer.isNew()) {
+            outputBuffer.checkConverter();
             return writer;
         } else {
             return null;


Do you have any test case to prove that this is still an issue? If so, please
attach.

Thanks,

Jan

Jan,

I attach screenshot of a test case that returns HTTP 404 status simply.
Until my first proposed patch is applied, this issue always occurs.


Regards,

Created attachment 11507 [details]
A test case that returns HTTP 404 status simply, without the first patch.

Created attachment 11508 [details]
A test case that returns HTTP 404 status simply, with the first patch.

This problem isn't yet solved in Tomcat 5.0.27 release.

It doesn't occur in 4.1.30 release, but always happens in 5.0.x release.
Any multi-byte character is not correctly printed into response reporter, 
independently of the response locale setting.

Therefore, it is impossible to read every multi-byte error messages which is 
displayed in default error page.
(And many Japanese users are in trouble very much.)

According to current ErrorReportValve source code, I have re-written my first 
proposed patch (see the following diff code).
Please apply it.

Index: jakarta-tomcat-
catalina/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java
===================================================================
RCS file: /home/cvspublic/jakarta-tomcat-
catalina/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java,v
retrieving revision 1.21
diff -u -w -r1.21 ErrorReportValve.java
--- jakarta-tomcat-
catalina/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java
	27 Jun 2004 23:56:23 -0000	1.21
+++ jakarta-tomcat-
catalina/catalina/src/share/org/apache/catalina/valves/ErrorReportValve.java
	17 Jul 2004 20:59:20 -0000
@@ -284,19 +284,17 @@
 
         try {
 
-            Writer writer = response.getReporter();
-
-            if (writer != null) {
-
-                Locale locale = Locale.getDefault();
-
                 try {
                 	response.setContentType("text/html");
-                	response.setLocale(locale);
+                response.setCharacterEncoding("utf-8");
                 } catch (Throwable t) {
                     if (container.getLogger().isDebugEnabled())
                         container.getLogger().debug("status.setContentType", 
t);
                 }
+
+            Writer writer = response.getReporter();
+
+            if (writer != null) {
 
                 // If writer is null, it's an indication that the response has
                 // been hard committed already, which should never happen

Created attachment 12178 [details]
The re-written patch to fix this problem in ErrorReportValve.

I haven't been looking into this problem much and maybe I'm wrong somewhat.

Regarding the latest patch, I'm wondering if there is any reason to drop setLocale()
from that portion.

Invocation of either setCharacterEncoding() or setContentType() that precedes setLocale()
seems to prevent setLocale() from changing the character encoding for the request.

Thanks, Moriyoshi.

Pondering carefully, the root cause is only that response.getReporter() 
method has been invoked before content type and locale setter methods of 
response are called.
Once the response.getReporter() method is invoked, the setLocale() method and 
the setCharacterEncoding() method of the response will process nothing.

Therefore, I think that so far my submitted patches are basically no wrong.
When the response.getReporter() method is invoked after setting content type 
and 
appropriate character encoding into this response, this issue surely will be 
solved.


Regards,

Kan Ogawa

I've applied the re-written patch.  I think it looks OK, but I haven't re-built 
and re-run your test cases yet.  If you could do that when you get a chance, 
that'd be great.  Thanks for contributing the patch.

Yoav, 

I downloaded Tomcat 5 nightly distribution (2004-07-28 Build), and have proved 
that this problem was fixed in Japanese language environment.

Thanks,
Kan Ogawa

Good, thanks for checking.  I'm verifying this issue.

*** Bug 30691 has been marked as a duplicate of this bug. ***