tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r892293 - in /tomcat/trunk/java: javax/servlet/http/ org/apache/catalina/authenticator/ org/apache/catalina/core/ org/apache/catalina/realm/ org/apache/catalina/valves/ org/apache/jasper/resources/
Date Sun, 20 Dec 2009 04:10:09 GMT
2009/12/18  <markt@apache.org>:
> Author: markt
> Date: Fri Dec 18 16:13:28 2009
> New Revision: 892293
>
> URL: http://svn.apache.org/viewvc?rev=892293&view=rev
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=47963
> Anything that could end up in an HTTP header must meet the requirements of RFC2616.

I think this approach to fixing this issue is wrong.

1. It is not these messages but the ones in
org/apache/tomcat/util/http/res/LocalStrings*.properties
that should adhere to RFC2616.
You should add that compliance comment to those files.

HttpMessages.getMessage(int) returns those strings.

2. There are plenty of 3-rd party libraries that call sendError(code.
message) and those won't be fixed by this patch.

One example that I saw was sendError(code. exception.getMessage()).

3. The result of this patch will be that error messages on the Tomcat
error pages for Japanese language will not be translated.  (Or for
Russian language, if I ever propose it).  I am not happy with it.

Especially these two messages, as error page 503 is hard to customize:

+# Note: The following value may be used in HTTP headers and as such may only
+#       contain TEXT as defined by RFC 2616. Since Japanese language messages
+#       do not meet this requirement, English text is used.
+applicationDispatcher.isUnavailable=Servlet {0} is currently unavailable

+# Note: The following value may be used in HTTP headers and as such may only
+#       contain TEXT as defined by RFC 2616. Since Japanese language messages
+#       do not meet this requirement, English text is used.
+standardContext.isUnavailable=This application is not currently available

4. Regarding alternative approaches.

We already do scanning and replacing '\n' and '\r's in the message. E.g.
write(message.replace('\n', ' ').replace('\r', ' '));

Maybe we can replace this with a custom function that performs safeguarding?

What approach would be better:
- For non-ISO8859-1 characters (code > 255):
a) replace non-ISO8859-1 characters with '?'
b) replace non-ISO8859-1 characters with space
c) throw an exception and fallback to the default message.
d) provide an option, to choose a) vs. c) vs. current legacy behavior

- For ASCII characters with codes 0..32, 127:
replace with spaces

All the above makes sense only if USE_CUSTOM_STATUS_MSG_IN_HEADER
option was explicitly set to true. Otherwise the
HttpMessages.getMessage(int) message is used and no safeguarding takes
place.
So it was user's choice that they need this feature, and are ready to
pay their performance overhead for safeguarding.

I am in favor of a) option.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message