You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2013/08/15 22:51:38 UTC
svn commit: r1514485 - in /tomcat/trunk/java/org/apache:
coyote/ajp/AbstractAjpProcessor.java
coyote/http11/AbstractOutputBuffer.java coyote/spdy/SpdyProcessor.java
tomcat/util/http/HttpMessages.java
Author: markt
Date: Thu Aug 15 20:51:38 2013
New Revision: 1514485
URL: http://svn.apache.org/r1514485
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55399
Have the message in the response line use the locale set for the response.
Modified:
tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java
tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
Modified: tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java?rev=1514485&r1=1514484&r2=1514485&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java Thu Aug 15 20:51:38 2013
@@ -947,7 +947,8 @@ public abstract class AbstractAjpProcess
message = response.getMessage();
}
if (message == null){
- message = HttpMessages.getMessage(response.getStatus());
+ message = HttpMessages.getInstance(
+ response.getLocale()).getMessage(response.getStatus());
}
if (message == null) {
// mod_jk + httpd 2.x fails with a null status message - bug 45026
Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java?rev=1514485&r1=1514484&r2=1514485&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java Thu Aug 15 20:51:38 2013
@@ -133,7 +133,7 @@ public abstract class AbstractOutputBuff
finished = false;
// Cause loading of HttpMessages
- HttpMessages.getMessage(200);
+ HttpMessages.getInstance(response.getLocale()).getMessage(200);
}
@@ -427,7 +427,8 @@ public abstract class AbstractOutputBuff
message = response.getMessage();
}
if (message == null) {
- write(HttpMessages.getMessage(status));
+ write(HttpMessages.getInstance(
+ response.getLocale()).getMessage(status));
} else {
write(message);
}
Modified: tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java?rev=1514485&r1=1514484&r2=1514485&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java Thu Aug 15 20:51:38 2013
@@ -440,7 +440,8 @@ public class SpdyProcessor extends Abstr
message = response.getMessage();
}
if (message == null) {
- message = HttpMessages.getMessage(response.getStatus());
+ message = HttpMessages.getInstance(
+ response.getLocale()).getMessage(response.getStatus());
}
if (message == null) {
// mod_jk + httpd 2.x fails with a null status message - bug
Modified: tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java?rev=1514485&r1=1514484&r2=1514485&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java Thu Aug 15 20:51:38 2013
@@ -16,6 +16,10 @@
*/
package org.apache.tomcat.util.http;
+import java.util.Locale;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
import org.apache.tomcat.util.res.StringManager;
/**
@@ -28,14 +32,24 @@ import org.apache.tomcat.util.res.String
* @author costin@eng.sun.com
*/
public class HttpMessages {
+
+ private static final Map<Locale,HttpMessages> instances =
+ new ConcurrentHashMap<>();
+
+ private static final HttpMessages DEFAULT = getInstance(Locale.getDefault());
+
// XXX move message resources in this package
- private static final StringManager sm =
- StringManager.getManager("org.apache.tomcat.util.http.res");
+ private final StringManager sm;
+
+ private String st_200 = null;
+ private String st_302 = null;
+ private String st_400 = null;
+ private String st_404 = null;
+
+ private HttpMessages(StringManager sm) {
+ this.sm = sm;
+ }
- private static String st_200=null;
- private static String st_302=null;
- private static String st_400=null;
- private static String st_404=null;
/** Get the status string associated with a status code.
* No I18N - return the messages defined in the HTTP spec.
@@ -45,36 +59,53 @@ public class HttpMessages {
* Common messages are cached.
*
*/
- public static String getMessage( int status ) {
+ public String getMessage(int status) {
// method from Response.
// Does HTTP requires/allow international messages or
// are pre-defined? The user doesn't see them most of the time
switch( status ) {
case 200:
- if( st_200==null ) {
- st_200=sm.getString( "sc.200");
+ if(st_200 == null ) {
+ st_200 = sm.getString("sc.200");
}
return st_200;
case 302:
- if( st_302==null ) {
- st_302=sm.getString( "sc.302");
+ if(st_302 == null ) {
+ st_302 = sm.getString("sc.302");
}
return st_302;
case 400:
- if( st_400==null ) {
- st_400=sm.getString( "sc.400");
+ if(st_400 == null ) {
+ st_400 = sm.getString("sc.400");
}
return st_400;
case 404:
- if( st_404==null ) {
- st_404=sm.getString( "sc.404");
+ if(st_404 == null ) {
+ st_404 = sm.getString("sc.404");
}
return st_404;
}
return sm.getString("sc."+ status);
}
+
+ public static HttpMessages getInstance(Locale locale) {
+ HttpMessages result = instances.get(locale);
+ if (result == null) {
+ StringManager sm = StringManager.getManager(
+ "org.apache.tomcat.util.http.res", locale);
+ if (Locale.getDefault().equals(sm.getLocale())) {
+ result = DEFAULT;
+ } else {
+ result = new HttpMessages(sm);
+ }
+ instances.put(locale, result);
+ }
+ return result;
+ }
+
+
/**
* Filter the specified message string for characters that are sensitive
* in HTML. This avoids potential attacks caused by including JavaScript
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1514485 - in /tomcat/trunk/java/org/apache: coyote/ajp/AbstractAjpProcessor.java
coyote/http11/AbstractOutputBuffer.java coyote/spdy/SpdyProcessor.java tomcat/util/http/HttpMessages.java
Posted by Mark Thomas <ma...@apache.org>.
On 11/09/2013 14:13, Mark Thomas wrote:
> On 11/09/2013 01:24, Konstantin Kolinko wrote:
>> 2013/8/16 <ma...@apache.org>:
>>> Author: markt
>>> Date: Thu Aug 15 20:51:38 2013
>>> New Revision: 1514485
>>>
>>> URL: http://svn.apache.org/r1514485
>>> Log:
>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55399
>>> Have the message in the response line use the locale set for the response.
>>>
>>> Modified:
>>> tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
>>> tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java
>>> tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
>>> tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
>>>
>>
>>>
>>> +
>>> + public static HttpMessages getInstance(Locale locale) {
>>> + HttpMessages result = instances.get(locale);
>>> + if (result == null) {
>>> + StringManager sm = StringManager.getManager(
>>> + "org.apache.tomcat.util.http.res", locale);
>>> + if (Locale.getDefault().equals(sm.getLocale())) {
>>> + result = DEFAULT;
>>> + } else {
>>> + result = new HttpMessages(sm);
>>> + }
>>> + instances.put(locale, result);
>>> + }
>>> + return result;
>>> + }
>>> +
>>
>> What a bit bothers me here (and in earlier changes to ErrorReportValve
>> etc. - r1514496) is that locale is provided by client and thus the
>> caches here and in StringManager can grow, instead of being limited to
>> the few locales that are actually bundled with Tomcat.
>
> That is a fair point. I'll take another look.
Because the method is synchronized we can simply use a LinkedHashMap
here. I have a fix I'll be applying shortly (I have a couple of other
fixes sat in my local git repo that I need to commit as well).
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1514485 - in /tomcat/trunk/java/org/apache: coyote/ajp/AbstractAjpProcessor.java
coyote/http11/AbstractOutputBuffer.java coyote/spdy/SpdyProcessor.java tomcat/util/http/HttpMessages.java
Posted by Mark Thomas <ma...@apache.org>.
On 11/09/2013 01:24, Konstantin Kolinko wrote:
> 2013/8/16 <ma...@apache.org>:
>> Author: markt
>> Date: Thu Aug 15 20:51:38 2013
>> New Revision: 1514485
>>
>> URL: http://svn.apache.org/r1514485
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55399
>> Have the message in the response line use the locale set for the response.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
>> tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java
>> tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
>> tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
>>
>
>>
>> +
>> + public static HttpMessages getInstance(Locale locale) {
>> + HttpMessages result = instances.get(locale);
>> + if (result == null) {
>> + StringManager sm = StringManager.getManager(
>> + "org.apache.tomcat.util.http.res", locale);
>> + if (Locale.getDefault().equals(sm.getLocale())) {
>> + result = DEFAULT;
>> + } else {
>> + result = new HttpMessages(sm);
>> + }
>> + instances.put(locale, result);
>> + }
>> + return result;
>> + }
>> +
>
> What a bit bothers me here (and in earlier changes to ErrorReportValve
> etc. - r1514496) is that locale is provided by client and thus the
> caches here and in StringManager can grow, instead of being limited to
> the few locales that are actually bundled with Tomcat.
That is a fair point. I'll take another look.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1514485 - in /tomcat/trunk/java/org/apache:
coyote/ajp/AbstractAjpProcessor.java coyote/http11/AbstractOutputBuffer.java
coyote/spdy/SpdyProcessor.java tomcat/util/http/HttpMessages.java
Posted by Konstantin Kolinko <kn...@gmail.com>.
2013/8/16 <ma...@apache.org>:
> Author: markt
> Date: Thu Aug 15 20:51:38 2013
> New Revision: 1514485
>
> URL: http://svn.apache.org/r1514485
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55399
> Have the message in the response line use the locale set for the response.
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
> tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java
> tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
> tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
>
>
> +
> + public static HttpMessages getInstance(Locale locale) {
> + HttpMessages result = instances.get(locale);
> + if (result == null) {
> + StringManager sm = StringManager.getManager(
> + "org.apache.tomcat.util.http.res", locale);
> + if (Locale.getDefault().equals(sm.getLocale())) {
> + result = DEFAULT;
> + } else {
> + result = new HttpMessages(sm);
> + }
> + instances.put(locale, result);
> + }
> + return result;
> + }
> +
What a bit bothers me here (and in earlier changes to ErrorReportValve
etc. - r1514496) is that locale is provided by client and thus the
caches here and in StringManager can grow, instead of being limited to
the few locales that are actually bundled with Tomcat.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1514485 - in /tomcat/trunk/java/org/apache: coyote/ajp/AbstractAjpProcessor.java
coyote/http11/AbstractOutputBuffer.java coyote/spdy/SpdyProcessor.java tomcat/util/http/HttpMessages.java
Posted by Rainer Jung <ra...@kippdata.de>.
On 15.08.2013 22:51, markt@apache.org wrote:
> Author: markt
> Date: Thu Aug 15 20:51:38 2013
> New Revision: 1514485
>
> URL: http://svn.apache.org/r1514485
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55399
> Have the message in the response line use the locale set for the response.
>
> Modified:
> tomcat/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java
> tomcat/trunk/java/org/apache/coyote/http11/AbstractOutputBuffer.java
> tomcat/trunk/java/org/apache/coyote/spdy/SpdyProcessor.java
> tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java?rev=1514485&r1=1514484&r2=1514485&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/http/HttpMessages.java Thu Aug 15 20:51:38 2013
> @@ -16,6 +16,10 @@
> */
> package org.apache.tomcat.util.http;
>
> +import java.util.Locale;
> +import java.util.Map;
> +import java.util.concurrent.ConcurrentHashMap;
> +
> import org.apache.tomcat.util.res.StringManager;
>
> /**
> @@ -28,14 +32,24 @@ import org.apache.tomcat.util.res.String
> * @author costin@eng.sun.com
> */
> public class HttpMessages {
> +
> + private static final Map<Locale,HttpMessages> instances =
> + new ConcurrentHashMap<>();
> +
> + private static final HttpMessages DEFAULT = getInstance(Locale.getDefault());
Problem here, see below.
> +
> // XXX move message resources in this package
> - private static final StringManager sm =
> - StringManager.getManager("org.apache.tomcat.util.http.res");
> + private final StringManager sm;
> +
> + private String st_200 = null;
> + private String st_302 = null;
> + private String st_400 = null;
> + private String st_404 = null;
> +
> + private HttpMessages(StringManager sm) {
> + this.sm = sm;
> + }
>
> - private static String st_200=null;
> - private static String st_302=null;
> - private static String st_400=null;
> - private static String st_404=null;
>
> /** Get the status string associated with a status code.
> * No I18N - return the messages defined in the HTTP spec.
> @@ -45,36 +59,53 @@ public class HttpMessages {
> * Common messages are cached.
> *
> */
> - public static String getMessage( int status ) {
> + public String getMessage(int status) {
> // method from Response.
>
> // Does HTTP requires/allow international messages or
> // are pre-defined? The user doesn't see them most of the time
> switch( status ) {
> case 200:
> - if( st_200==null ) {
> - st_200=sm.getString( "sc.200");
> + if(st_200 == null ) {
> + st_200 = sm.getString("sc.200");
> }
> return st_200;
> case 302:
> - if( st_302==null ) {
> - st_302=sm.getString( "sc.302");
> + if(st_302 == null ) {
> + st_302 = sm.getString("sc.302");
> }
> return st_302;
> case 400:
> - if( st_400==null ) {
> - st_400=sm.getString( "sc.400");
> + if(st_400 == null ) {
> + st_400 = sm.getString("sc.400");
> }
> return st_400;
> case 404:
> - if( st_404==null ) {
> - st_404=sm.getString( "sc.404");
> + if(st_404 == null ) {
> + st_404 = sm.getString("sc.404");
> }
> return st_404;
> }
> return sm.getString("sc."+ status);
> }
>
> +
> + public static HttpMessages getInstance(Locale locale) {
> + HttpMessages result = instances.get(locale);
> + if (result == null) {
> + StringManager sm = StringManager.getManager(
> + "org.apache.tomcat.util.http.res", locale);
> + if (Locale.getDefault().equals(sm.getLocale())) {
> + result = DEFAULT;
DEFAULT is set above by a call to this method here which already tries
to use it. On one of my systems this leads to "result" being set to null
here and ...
> + } else {
> + result = new HttpMessages(sm);
> + }
> + instances.put(locale, result);
an NPE here because "instances" is a ConcurrentHashMap that doesn't
allow null. After that no request processing is possible, because the
static initializer setting DEFAULT had failed and any further use of
this class leads to a java.lang.ExceptionInInitializerError.
One solution might be setting
private static final HttpMessages DEFAULT = new
HttpMessages(StringManager.getManager("org.apache.tomcat.util.http.res",
Locale.getDefault()));
above. At least this worked for me.
> + }
> + return result;
> + }
> +
> +
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org