You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by "Julien, Timothy" <TJ...@Bluestone.com> on 2001/03/21 19:04:25 UTC
bug in Jasper - bad lookups to Constants.getString()
I believe there is a bug in Jasper when resources (such as web.xml's dtd)
can't be loaded.
The basic problem is that keys are getting passed into Constants.getString()
which aren't keys in the message resources file. This happens because of
some exception handling, which basically constructs an invalid key - namely,
a *value* in the message resources file.
JspUtil.parseXMLDocJaxp(String, InputStream)
{
try
{
...
}
catch(IOException io)
{
throw new
JasperException(Constants.getString("jsp.error.parse.xml", new Object[] {
uri, io.toString()
}));
}
}
and then lower down the call stack:
public TldLocationsCache(ServletContext ctxt)
{
mappings = new Hashtable();
try
{
processWebDotXml(ctxt);
processJars(ctxt);
}
catch(JasperException ex)
{
Constants.message(ex.getMessage(), 1);
}
}
The trouble is that ex.getMessage() here contains a *value* from the message
resource file, (as looked up in the first caught Exception) not a *key*.
One fix would be in JspUtil.parseXMLDocJaxp(String, InputStream) to pass
only the key (jsp.error.parse.xml) as the JasperException's message - and
delay the lookup. But then you lose some information (i.e., io.String()).
Maybe a better solution is to set a flag on JasperException which marks its
message as being either key or value - so that Constants.getString can be
called only when necessary. I realize this would effect alot of code.
anyway, I'm happy to help in any way with the fix.
Tim Julien
HP Middleware
Re: bug in Jasper - bad lookups to Constants.getString()
Posted by Mel Martinez <me...@yahoo.com>.
--- cmanolache@yahoo.com wrote:
> On Wed, 21 Mar 2001, Mel Martinez wrote:
>
> > StringManager's getString() currently deals with
> the
> > MissingResourceException by simply returning a
> String
> > consisting of a warning message about not being
> able
> > to find the particular key. I don't think that is
> > correct behavior. I think it should throw the
> > MissingResourceException or return null and that
> > calling code should deal with it. Otherwise the
>
> Ignoring exceptions is bad. Having to handle
> exceptions
> for trivial cases is bad too.
>
I think that in this case, the pattern is so similar
to a Collections retrieval that it makes sense to use
'null' as the return value in the event the String
resource is not found rather than an exception. This
should have the least impact on existing client code
and is consistent with most other java usage of
containers.
The gain is that it will now at least be _possible_
for the client code to detect that a String was not
found (by simply checking for null). If the code does
not do this check, in most cases all that would happen
is that "null" would be displayed. In rare cases, a
nullpointerexception may be thrown (if they try to
invoke a String method without first checking for
null). We'll have to track those down as they occur.
> My feeling is that this is not the biggest priority,
> but you are right - StringManager and the whole
> resource
> handling needs a refactoring. Go for it, if you want
> to do it - I'll do whatever I can to help.
>
> ( I think thread pools or jasper or the admin are in
> a much
> bigger need for fixes and cleanup )
>
I'll be starting to tackle tc3.3 Jasper refactoring
this week with a proposal and hopefully some
discussion. Next week I'll have serveral days where I
can devote quite a bit of time to implementing it.
> I am hoping the "commons" will ease our work - this
> is
> one of the things that are used in all jakarta
> projects,
> and it would be a good thing to share it ( in which
> case
> refactoring it and merging with the best out of what
> other
> projects are doing is worth the effort )
>
I'm only going to put this one tweak in the
StringManager.getString() and start to refactor jasper
to use StringManager for now. I'll also look at
putting in a short-term fix for the standalone jasper
classpath problem mentioned in another thread. Small
steps.
mel
__________________________________________________
Do You Yahoo!?
Get email at your own domain with Yahoo! Mail.
http://personal.mail.yahoo.com/
Re: bug in Jasper - bad lookups to Constants.getString()
Posted by cm...@yahoo.com.
On Wed, 21 Mar 2001, Mel Martinez wrote:
> StringManager's getString() currently deals with the
> MissingResourceException by simply returning a String
> consisting of a warning message about not being able
> to find the particular key. I don't think that is
> correct behavior. I think it should throw the
> MissingResourceException or return null and that
> calling code should deal with it. Otherwise the
Ignoring exceptions is bad. Having to handle exceptions
for trivial cases is bad too.
My feeling is that this is not the biggest priority,
but you are right - StringManager and the whole resource
handling needs a refactoring. Go for it, if you want
to do it - I'll do whatever I can to help.
( I think thread pools or jasper or the admin are in a much
bigger need for fixes and cleanup )
I am hoping the "commons" will ease our work - this is
one of the things that are used in all jakarta projects,
and it would be a good thing to share it ( in which case
refactoring it and merging with the best out of what other
projects are doing is worth the effort )
Costin
Re: bug in Jasper - bad lookups to Constants.getString()
Posted by Mel Martinez <me...@yahoo.com>.
--- cmanolache@yahoo.com wrote:
> On Wed, 21 Mar 2001, Mel Martinez wrote:
>
> > Would the following be an acceptable solution?
> >
> > We change the
> > Constants.message(key,args,verbosityLevel) method
> > behavior so that if getString(key,args) throws
> > a missing resource exception that it logs using
> the
> > key itself as the log message. Note that this
> would
> > also require changing the getString() method to
> throw
> > the MissingResourceException instead of the Error
> > object that it currently throws.
>
> +1 ( please check the tomcat side is doing the
> same - we never had the problem but you never know )
>
> Would StringManager be a better place for the fix ?
>
> Costin
Yes. After looking into this, it would seem that
there is no reason Jasper can't make use of
StringManager to handle getting it's resource strings
consistent with the way tomcat packages are doing so.
This doesn't look too difficult to retro into Jasper.
However ...
StringManager's getString() currently deals with the
MissingResourceException by simply returning a String
consisting of a warning message about not being able
to find the particular key. I don't think that is
correct behavior. I think it should throw the
MissingResourceException or return null and that
calling code should deal with it. Otherwise the
calling code really has no reason to suspect anything
is wrong (that the returned string is not what it
wanted). If an exception were to be thrown and caught
(or a null is checked and found) then the calling code
wuld have the option to use the key to construct a
replacement String or whatever. However, I am a bit
leary about changing code to throw an exception where
one has been incorrectly swallowed like this.
Currently unsuspecting code may exist that relies
unknowingly on this swallowing of the exception by
blindly just using whatever string is returned.
Changing this may require a bit of testing to see all
the effects caused by missing resource strings.
Mel
__________________________________________________
Do You Yahoo!?
Get email at your own domain with Yahoo! Mail.
http://personal.mail.yahoo.com/
Re: bug in Jasper - bad lookups to Constants.getString()
Posted by Mel Martinez <me...@yahoo.com>.
--- cmanolache@yahoo.com wrote:
> On Wed, 21 Mar 2001, Mel Martinez wrote:
>
> > Would the following be an acceptable solution?
> >
> > We change the
> > Constants.message(key,args,verbosityLevel) method
> > behavior so that if getString(key,args) throws
> > a missing resource exception that it logs using
> the
> > key itself as the log message. Note that this
> would
> > also require changing the getString() method to
> throw
> > the MissingResourceException instead of the Error
> > object that it currently throws.
>
> +1 ( please check the tomcat side is doing the
> same - we never had the problem but you never know )
>
> Would StringManager be a better place for the fix ?
>
> Costin
Possibly. I'll take a look tonight. I have to run
home now to watch my daughter watch disney movies and
then run around the house carrying her while she
pretends to fly for a few hours. :-)
Later,
Mel
>
> >
> > Looking at this, I am not sure why an Error is
> being
> > thrown here (in Constants.getString()). If Jasper
> is
> > operating in JspServlet mode inside some other
> > container (i.e. WebLogic, JRun, whatever) then
> should
> > it necessarily throw an Error? According to the
> spec,
> > Errors are reserved for conditions that are so
> serious
> > that they should normally not be caught by the
> > application. I.E., they should bring the JVM
> down.
> > The fact that Jasper (a servlet) is unable to find
> a
> > String in it's resource file sounds more like
> enough
> > error to throw a RuntimeException or even a
> > ServletException, but not an Error. That would
> allow
> > the containing ServletEngine to handl the error to
> > Jasper without bringing down other servlets -
> without
> > having to catch 'Error'.
> >
> > I think the right way to handle this is to change
> the
> > getString() to just throw the
> > MissingResourceException. This requires no change
> to
> > the interfaces.
> >
> > Then in org.apache.jasper.Constants change
> message()
> > to:
> >
> > public static final void message(
> > String key,
> > Object[] args,
> > int
> > verbosityLevel){
> > if (jasperLog == null)
> > jasperLog = Log.getLog("JASPER_LOG",
> null);
> >
> > if (jasperLog != null){
> > try{
> > jasperLog.log(
> > getString(key,args),
> > verbosityLevel);
> > }catch(MissingResourceException e){
> > jasperLog.log(
> > key,verbosityLevel);
> > }
> > }
> >
> >
> > This should work 99% of the time the way you
> describe
> > with much less pain in implementation.
> >
> > Feedback welcome. If no one sends a -1 on this, I
> can
> > put this patch in fairly rapidly in tc3.3.
> >
> > Mel
> >
> >
> > --- "Julien, Timothy" <TJ...@Bluestone.com>
> wrote:
> > > I believe there is a bug in Jasper when
> resources
> > > (such as web.xml's dtd)
> > > can't be loaded.
> > > The basic problem is that keys are getting
> passed
> > > into Constants.getString()
> > > which aren't keys in the message resources file.
>
> > > This happens because of
> > > some exception handling, which basically
> constructs
> > > an invalid key - namely,
> > > a *value* in the message resources file.
> > >
> > > JspUtil.parseXMLDocJaxp(String, InputStream)
> > > {
> > > try
> > > {
> > > ...
> > > }
> > > catch(IOException io)
> > > {
> > > throw new
> > >
> >
>
JasperException(Constants.getString("jsp.error.parse.xml",
> > > new Object[] {
> > > uri, io.toString()
> > > }));
> > > }
> > > }
> > > and then lower down the call stack:
> > >
> > > public TldLocationsCache(ServletContext ctxt)
> > > {
> > > mappings = new Hashtable();
> > > try
> > > {
> > > processWebDotXml(ctxt);
> > > processJars(ctxt);
> > > }
> > > catch(JasperException ex)
> > > {
> > > Constants.message(ex.getMessage(), 1);
> > > }
> > > }
> > >
> > > The trouble is that ex.getMessage() here
> contains a
> > > *value* from the message
> > > resource file, (as looked up in the first caught
> > > Exception) not a *key*.
> > >
> > > One fix would be in
> JspUtil.parseXMLDocJaxp(String,
> > > InputStream) to pass
> > > only the key (jsp.error.parse.xml) as the
> > > JasperException's message - and
> > > delay the lookup. But then you lose some
> > > information (i.e., io.String()).
> > >
> > > Maybe a better solution is to set a flag on
> > > JasperException which marks its
> > > message as being either key or value - so that
> > > Constants.getString can be
> > > called only when necessary. I realize this
> would
> > > effect alot of code.
> > >
> > > anyway, I'm happy to help in any way with the
> fix.
> > > Tim Julien
> > > HP Middleware
> > >
> > >
> >
> >
> > __________________________________________________
> > Do You Yahoo!?
> > Get email at your own domain with Yahoo! Mail.
> > http://personal.mail.yahoo.com/
> >
>
__________________________________________________
Do You Yahoo!?
Get email at your own domain with Yahoo! Mail.
http://personal.mail.yahoo.com/
Re: bug in Jasper - bad lookups to Constants.getString()
Posted by cm...@yahoo.com.
On Wed, 21 Mar 2001, Mel Martinez wrote:
> Would the following be an acceptable solution?
>
> We change the
> Constants.message(key,args,verbosityLevel) method
> behavior so that if getString(key,args) throws
> a missing resource exception that it logs using the
> key itself as the log message. Note that this would
> also require changing the getString() method to throw
> the MissingResourceException instead of the Error
> object that it currently throws.
+1 ( please check the tomcat side is doing the
same - we never had the problem but you never know )
Would StringManager be a better place for the fix ?
Costin
>
> Looking at this, I am not sure why an Error is being
> thrown here (in Constants.getString()). If Jasper is
> operating in JspServlet mode inside some other
> container (i.e. WebLogic, JRun, whatever) then should
> it necessarily throw an Error? According to the spec,
> Errors are reserved for conditions that are so serious
> that they should normally not be caught by the
> application. I.E., they should bring the JVM down.
> The fact that Jasper (a servlet) is unable to find a
> String in it's resource file sounds more like enough
> error to throw a RuntimeException or even a
> ServletException, but not an Error. That would allow
> the containing ServletEngine to handl the error to
> Jasper without bringing down other servlets - without
> having to catch 'Error'.
>
> I think the right way to handle this is to change the
> getString() to just throw the
> MissingResourceException. This requires no change to
> the interfaces.
>
> Then in org.apache.jasper.Constants change message()
> to:
>
> public static final void message(
> String key,
> Object[] args,
> int
> verbosityLevel){
> if (jasperLog == null)
> jasperLog = Log.getLog("JASPER_LOG", null);
>
> if (jasperLog != null){
> try{
> jasperLog.log(
> getString(key,args),
> verbosityLevel);
> }catch(MissingResourceException e){
> jasperLog.log(
> key,verbosityLevel);
> }
> }
>
>
> This should work 99% of the time the way you describe
> with much less pain in implementation.
>
> Feedback welcome. If no one sends a -1 on this, I can
> put this patch in fairly rapidly in tc3.3.
>
> Mel
>
>
> --- "Julien, Timothy" <TJ...@Bluestone.com> wrote:
> > I believe there is a bug in Jasper when resources
> > (such as web.xml's dtd)
> > can't be loaded.
> > The basic problem is that keys are getting passed
> > into Constants.getString()
> > which aren't keys in the message resources file.
> > This happens because of
> > some exception handling, which basically constructs
> > an invalid key - namely,
> > a *value* in the message resources file.
> >
> > JspUtil.parseXMLDocJaxp(String, InputStream)
> > {
> > try
> > {
> > ...
> > }
> > catch(IOException io)
> > {
> > throw new
> >
> JasperException(Constants.getString("jsp.error.parse.xml",
> > new Object[] {
> > uri, io.toString()
> > }));
> > }
> > }
> > and then lower down the call stack:
> >
> > public TldLocationsCache(ServletContext ctxt)
> > {
> > mappings = new Hashtable();
> > try
> > {
> > processWebDotXml(ctxt);
> > processJars(ctxt);
> > }
> > catch(JasperException ex)
> > {
> > Constants.message(ex.getMessage(), 1);
> > }
> > }
> >
> > The trouble is that ex.getMessage() here contains a
> > *value* from the message
> > resource file, (as looked up in the first caught
> > Exception) not a *key*.
> >
> > One fix would be in JspUtil.parseXMLDocJaxp(String,
> > InputStream) to pass
> > only the key (jsp.error.parse.xml) as the
> > JasperException's message - and
> > delay the lookup. But then you lose some
> > information (i.e., io.String()).
> >
> > Maybe a better solution is to set a flag on
> > JasperException which marks its
> > message as being either key or value - so that
> > Constants.getString can be
> > called only when necessary. I realize this would
> > effect alot of code.
> >
> > anyway, I'm happy to help in any way with the fix.
> > Tim Julien
> > HP Middleware
> >
> >
>
>
> __________________________________________________
> Do You Yahoo!?
> Get email at your own domain with Yahoo! Mail.
> http://personal.mail.yahoo.com/
>
Re: bug in Jasper - bad lookups to Constants.getString()
Posted by Mel Martinez <me...@yahoo.com>.
Would the following be an acceptable solution?
We change the
Constants.message(key,args,verbosityLevel) method
behavior so that if getString(key,args) throws
a missing resource exception that it logs using the
key itself as the log message. Note that this would
also require changing the getString() method to throw
the MissingResourceException instead of the Error
object that it currently throws.
Looking at this, I am not sure why an Error is being
thrown here (in Constants.getString()). If Jasper is
operating in JspServlet mode inside some other
container (i.e. WebLogic, JRun, whatever) then should
it necessarily throw an Error? According to the spec,
Errors are reserved for conditions that are so serious
that they should normally not be caught by the
application. I.E., they should bring the JVM down.
The fact that Jasper (a servlet) is unable to find a
String in it's resource file sounds more like enough
error to throw a RuntimeException or even a
ServletException, but not an Error. That would allow
the containing ServletEngine to handl the error to
Jasper without bringing down other servlets - without
having to catch 'Error'.
I think the right way to handle this is to change the
getString() to just throw the
MissingResourceException. This requires no change to
the interfaces.
Then in org.apache.jasper.Constants change message()
to:
public static final void message(
String key,
Object[] args,
int
verbosityLevel){
if (jasperLog == null)
jasperLog = Log.getLog("JASPER_LOG", null);
if (jasperLog != null){
try{
jasperLog.log(
getString(key,args),
verbosityLevel);
}catch(MissingResourceException e){
jasperLog.log(
key,verbosityLevel);
}
}
This should work 99% of the time the way you describe
with much less pain in implementation.
Feedback welcome. If no one sends a -1 on this, I can
put this patch in fairly rapidly in tc3.3.
Mel
--- "Julien, Timothy" <TJ...@Bluestone.com> wrote:
> I believe there is a bug in Jasper when resources
> (such as web.xml's dtd)
> can't be loaded.
> The basic problem is that keys are getting passed
> into Constants.getString()
> which aren't keys in the message resources file.
> This happens because of
> some exception handling, which basically constructs
> an invalid key - namely,
> a *value* in the message resources file.
>
> JspUtil.parseXMLDocJaxp(String, InputStream)
> {
> try
> {
> ...
> }
> catch(IOException io)
> {
> throw new
>
JasperException(Constants.getString("jsp.error.parse.xml",
> new Object[] {
> uri, io.toString()
> }));
> }
> }
> and then lower down the call stack:
>
> public TldLocationsCache(ServletContext ctxt)
> {
> mappings = new Hashtable();
> try
> {
> processWebDotXml(ctxt);
> processJars(ctxt);
> }
> catch(JasperException ex)
> {
> Constants.message(ex.getMessage(), 1);
> }
> }
>
> The trouble is that ex.getMessage() here contains a
> *value* from the message
> resource file, (as looked up in the first caught
> Exception) not a *key*.
>
> One fix would be in JspUtil.parseXMLDocJaxp(String,
> InputStream) to pass
> only the key (jsp.error.parse.xml) as the
> JasperException's message - and
> delay the lookup. But then you lose some
> information (i.e., io.String()).
>
> Maybe a better solution is to set a flag on
> JasperException which marks its
> message as being either key or value - so that
> Constants.getString can be
> called only when necessary. I realize this would
> effect alot of code.
>
> anyway, I'm happy to help in any way with the fix.
> Tim Julien
> HP Middleware
>
>
__________________________________________________
Do You Yahoo!?
Get email at your own domain with Yahoo! Mail.
http://personal.mail.yahoo.com/