zcwslnh / javamelody

Automatically exported from code.google.com/p/javamelody
0 stars 0 forks source link

The handling of translated tokens is not robust enough. #70

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Have a JVM parameter of your application containing a # 

For example a GC rolling log for an IBM JVM:
-Xverbosegclog:/soft/myapp/myapp38/logs/myapp38Server_stdloggc#.log,10,1000

This will cause an ArrayIndexOutOfBoundsException

What is the expected output? What do you see instead?

I think that's not safe (and not efficient) to parse the user data searching 
for tokens to replace, especially when you explicitely add these tokens 
yourself.

You should better translate tokens before injecting them into the output string 
(using I18N.getString)and no more parse the string.

What version of the product are you using? On what application server, JDK,
operating system?

javamelody 1.21 and 1.24-SNAPHOT
AIX
IBM J9 VM, 2.4, JRE 1.6.0 IBM J9 2.4 AIX ppc-32 
Websphere 7

Please provide any additional information below.

Please find enclosed a quick fix of I18N.java which supports having an single # 
in user data
I'm using it to fix the issue on my side.
But I don't think that's a long term solution.

Original issue reported on code.google.com by patrick....@gmail.com on 29 Nov 2010 at 1:49

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by evernat@free.fr on 30 Nov 2010 at 10:24

GoogleCodeExporter commented 9 years ago
I have created the separate issue 71 for the jvm parameters. And it is now 
fixed.

For the robustness:
I may be wrong, but it seems easier to have "#label_key#" in contents than to 
write the contents and the translated labels separately. For example, lots of 
template libraries come to mind which were made for things like this (jasper, 
freemarker, jelly...) but we probably can't use one of them to avoid any new 
dependency. We may extend some day the single "#label_key#" feature to include 
"$attribute$" (it would be a minimalistic template engine with 2 features).

So I would prefer for now to keep "#label_key#" for the "easier" reason.

And "#" are not supposed to be possible in values of attributes written like 
that: if you have ArrayIndexOutOfBoundsException, it is a bug which *must* be 
fixed.
So I would prefer to have a "blocking" issue which will be fixed with a new jar 
available after some hours in most cases (for example, 4h for issue 30), than 
to use that patch to workaround the bug. (I agree with you: the workaround with 
the patch can't be long term.)

So this issue would be "won't fix"? (except the issue 71 which is fixed)
What do you think?

Original comment by evernat@free.fr on 1 Dec 2010 at 12:20

GoogleCodeExporter commented 9 years ago
I think that the fix you've done only handle this specific case and wont solve 
the generic issue (not sure this generic issue exists either). 

It may be enough for now, but I still think that it would be safer to use 
"I18N.getString" before creating the string to get the right translation. As 
far as you're not expecting the "user data" to contains token to translate (do 
you?) you dont have to parse user data.

But it's just my opinion.

Original comment by patrick....@gmail.com on 1 Dec 2010 at 10:31

GoogleCodeExporter commented 9 years ago
I will let the code like that for now, except specific cases of course.
Just a note: a global change would have a large impact and no, "user data" does 
not contain tokens to translate.

Thanks for the specific issue.

Original comment by evernat@free.fr on 4 Dec 2010 at 8:37