On 20/12/2009 04:10, Konstantin Kolinko wrote:
> 2009/12/18 <ma...@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
This I disagree with. The messages can end up being used in HTTP headers
and some of them used to violate RFC2616. That needs fixing. I agree
that there are other ways we could fix this.
> 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.
Agreed.
> 2. There are plenty of 3-rd party libraries that call sendError(code.
> message) and those won't be fixed by this patch.
Not our problem although if whatever we end up implementing works around
them great. If our solution doesn't work around broken libraries I am
fine with that too.
> 3. The result of this patch will be that error messages on the Tomcat
> error pages for Japanese language will not be translated.
Correct. Although I note that a number of messages would have been in
English before this patch anyway.
> (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?
Anything is possible ;)
The fundamental issue is that the error message on the error page comes
from response.getMessage() and we use the same text in the HTTP header.
The first should use i18n, the second must comply with RFC2616. Those
requirements are not compatible.
> 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.
The user may not be expecting the performance hit. That would need
adding to the docs.
> I am in favor of a) option.
I don't view an HTTP header where the message is a long string of ????
characters as better. In that case I think the use English text is a
better option. Neither do I think we want too many configuration options
here.
What do you think to the following as an approach:
1. Revert r892293
2. Write an isSafeForHttpHeader() function
3. Replace all instances of
if (org.apache.coyote.Constants.USE_CUSTOM_STATUS_MSG_IN_HEADER) {
with
if (org.apache.coyote.Constants.USE_CUSTOM_STATUS_MSG_IN_HEADER &&
isSafeForHttpHeader(response.getMessage())) {
That would allow i18n to be used in the body of the response and in the
HTTP headers if it were safe. If it were not safe, the default would be
used. The user any pays the performance hit of the isSafeForHttpHeader()
test if they elect to use custom messages.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
