Bug 49426 - Manager app wrongly localized
Manager app wrongly localized
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Manager
trunk
PC Linux
: P2 normal (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-06-11 03:26 UTC by Marc Guillemot
Modified: 2010-10-16 15:32 UTC (History)
0 users



Attachments
Patch (with unit test) fixing the problem for the ManagerServlet (32.35 KB, text/plain)
2010-06-11 03:26 UTC, Marc Guillemot
Details
Right patch (with unit test) fixing the problem for the ManagerServlet ( (30.03 KB, patch)
2010-06-11 03:31 UTC, Marc Guillemot
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Guillemot 2010-06-11 03:26:07 UTC
Created attachment 25582 [details]
Patch (with unit test) fixing the problem for the ManagerServlet

The manager app shows message using the server's default locale whereas it should react on the request Locale.

The cause is that the ManagerServlet uses StringManager what is fine for messages that are logged but not for those that are sent as responses.

The attached patch fixes the problem for the ManagerServlet but not for the HTMLManagerServlet. It will require a bit more refactoring there because the same messages may be intended for log or for output to the response. I can propose a second patch for it once the problem has been fixed for the ManagerServlet.

For info: same problem occurs in Tomcat 6.
Comment 1 Marc Guillemot 2010-06-11 03:31:50 UTC
Created attachment 25583 [details]
Right patch (with unit test) fixing the problem for the ManagerServlet   (

Oops previous patch contained changes for HTMLManagerServlet that are not ready yet. Attached the right version of the patch.
Comment 2 Mark Thomas 2010-06-13 16:52:36 UTC
-1 to the patch as currently written. It isn't thread safe.
Comment 3 Marc Guillemot 2010-06-14 03:13:31 UTC
OK, you're right. What about holding the ResourceBundle in a ThreadLocal?
Comment 4 Mark Thomas 2010-06-14 17:48:05 UTC
(In reply to comment #3)
> OK, you're right. What about holding the ResourceBundle in a ThreadLocal?

Given all the issues we have seen with memory leaks and ThreadLocals I'd rather not.

Some additional thoughts:
- we could just pass the Locale around with the writer
- I wonder how hard it would be to extend StringManager (efficiently - which may be the slightly tricky bit) to support multiple Locales
- in determining which Locale to use we need to call request.getLocales() and check each in turn until we find a suitable match.
Comment 5 Marc Guillemot 2010-06-15 03:11:56 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > OK, you're right. What about holding the ResourceBundle in a ThreadLocal?
> 
> Given all the issues we have seen with memory leaks and ThreadLocals I'd
> rather not.
> 
> Some additional thoughts:
> - we could just pass the Locale around with the writer

this would be the only solution when ThreadLocal is not wanted. I didn't want to choose this way as it means more changes but I can do it.
Rather than passing writer and Locale (or ResourceBundle) everywhere along the way, I could imagine having a wrapper for both.

> - I wonder how hard it would be to extend StringManager (efficiently - which
> may be the slightly tricky bit) to support multiple Locales

this wouldn't solve previous point: you have to be able to access the Locale each time you need to get a String.
Additionally StringManager is quite useless. StringManager's cache doesn't make sense as java.util.ResourceBundle has already one. This means that the only value of StringManager is the facility method getString(final String key, final Object... args).

> - in determining which Locale to use we need to call request.getLocales() and
> check each in turn until we find a suitable match.

this is correct. I didn't do it to keep the patch smaller in a first time but I can adapt patch (including unit test) for that now if you want.
Comment 6 Mark Thomas 2010-10-16 15:32:32 UTC
Fixed in trunk and will be in 7.0.5 onwards.