Patch for bug report #362 (and a patch for a logging error)

[Serge Knystautas <se...@lokitech.com>]

Ok, I submitted the bug report this afternoon (#362) about how jasper was
mishandling a custom tag of mine.  You can read the report if you want, but
basically here's what happens... say your custom tag has an attribute to be
rtexprvalue and because of the flexibility of the tag, you make the setter
method take a java.lang.Object value.  When you then set a non-dynamic value
in your JSP (rather than passing some object, just write attr="value"),
jasper throws this funny message that says it "can't cast String to
java.lang.Object."

Not a high priority thing, but I think pretty useful (especially since I'd
like to use these custom tags I wrote!!)  I read over the JSP spec several
times and don't see how this would be a violation of the spec.  Included
below is the 2 line patch.

In testing this patch, I also found a bug in the Logger that was ignoring
the verbosity level setting and just dumping *everything* to the log.  So
there's a second one line patch as well.

These are diff'd off of the CVS tree as of this evening (8 Nov 00)...


Index: jasper/compiler/TagBeginGenerator.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/jasper/compiler/TagBegin
Generator.java,v
retrieving revision 1.16
diff -r1.16 TagBeginGenerator.java
269a270,271
>   } else if (c == Object.class) {
>    return writer.quoteString(s);


Index: tomcat/util/log/Logger.java
===================================================================
RCS file:
/home/cvspublic/jakarta-tomcat/src/share/org/apache/tomcat/util/log/Logger.j
ava,v
retrieving revision 1.1
diff -r1.1 Logger.java
344c344
<  log(message, null, level);
---
>  log(message, null, verbosityLevel);


Serge Knystautas
Loki Technologies
http://www.lokitech.com/

Re: Patch for bug report #362 (and a patch for a logging error)

[Serge Knystautas <se...@lokitech.com>]

"Craig R. McClanahan" wrote:
>
> Serge Knystautas wrote:
>
> > [snip]
> > The way I got this to work was to overload the setter method to take
both a
> > String and an Object... this got it working through Jasper Servlet but
not
> > jspc.  I don't know if that's a violation of the spec or the way to
handle
> > this issue.
>
> Yep, it is, and again this is made explicit in JSP 1.2 :-(.

I've been agonizing over this days reading over the spec again and again and
immersing myself in the bean API.  First, I really don't think the spec
intends to require attr='<%="value"%>' and in fact in JSP 1.2 it specifies
how you write your tag to support attr="value".

I endeavored to use the bean API as the JSP spec tells me to.  After I
finish my tag, I created a <Tagname>BeanInfo which returns an array of
PropertyDescriptor objects defining my properties.  Then I write a
<Tagname><Propertyname>PropertyEditor class and set this on the
PropertyDescriptor for that property that is a java.lang.Object.  My
<Tagname><Propertyname>PropertyEditor class overrides setAsText(String)
which according to section 4.2 of the JSP 1.2 spec, should allow me to pass
a String constant to this property.

So my problem is... there's no way in Tomcat 3.2b7 to do attr="value" if
your attr is an Object property.  I can use Tomcat 3.1 and could use Tomcat
4.0 (assuming this part is implemented), but not Tomcat 3.2b7.  The "rigor"
was added to prevent you from passing a String constant directly to an
Object property, but the way to support this through a PropertyEditor class
is not added.

I see this as an isolated but critical flaw in the implementation of the
spec.  As I see it, you can either relax the recent restriction made (by
allowing a String to convert to an Object... my 2 line patch) or you can add
support for tags with a PropertyEditor.  I don't really want to hold up
Tomcat 3.2, and since it doesn't really need to support JSP 1.2 just yet, I
would suggest making it so that tags can pass a String constant directly to
an Object property (which in my opinion complies with JSP 1.1, even though
not JSP 1.2).

Anyway, I hope this sheds some light on my plight, and maybe someone can
decide on a remedy.  I believe the patch to add support for PropertyEditors
is non-trivial, otherwise I'd try to write it myself and submit.  Maybe
Eduardo can also shed light on this issue and how he envisions the spec
migration happening.

> >  Anyway, seems that if spec compliance is an issue, we should
> > have Tomcat 3.2 be uniform in both jspc and the Jasper Servlet.
>
> That is *definitely* true.  Unfortunately, I'm not the person most versed
with
> the JSP compiler.  Pierre Delisle is, and he will also be back on Monday.

I haven't had the chance to test this, as I've been spending my time trying
to figure a way out for my tags rather than make sure they don't work.  ;)

Serge Knystautas
Loki Technologies
http://www.lokitech.com/

Re: Patch for bug report #362 (and a patch for a logging error)

[Pierre Delisle <pi...@sun.com>]

Serge,

> > The way I got this to work was to overload the setter method to take both a
> > String and an Object... this got it working through Jasper Servlet but not
> > jspc.  I don't know if that's a violation of the spec or the way to handle
> > this issue.

Tried to make this work under 3.2b7, and it always does return the
proper exception. Please send a sample test case so I can debug.

    -- Pierre


When I try yo

"Craig R. McClanahan" wrote:
> 
> Serge Knystautas wrote:
> 
> > [snip]
> > The way I got this to work was to overload the setter method to take both a
> > String and an Object... this got it working through Jasper Servlet but not
> > jspc.  I don't know if that's a violation of the spec or the way to handle
> > this issue.
> 
> Yep, it is, and again this is made explicit in JSP 1.2 :-(.
> 
> I was trying something similar (overloaded setters) originally with Struts, and
> it went down in flames on most servlet containers because many of them just use
> the standard JavaBeans introspector -- and the introspector returns a property
> setter only if there is a single method.  On one of the other containers, you'd
> start getting compiler errors like "this is a read-only property" because the
> introspector returned neither setter.
> 
> >  Anyway, seems that if spec compliance is an issue, we should
> > have Tomcat 3.2 be uniform in both jspc and the Jasper Servlet.
> 
> That is *definitely* true.  Unfortunately, I'm not the person most versed with
> the JSP compiler.  Pierre Delisle is, and he will also be back on Monday.
> 
> >  I looked
> > through the code some and didn't understand why it would compile from the
> > Jasper Servlet but not jspc (before my patch), but that's what's happening
> > for me with Tomcat 3.2b6.
> >
> 
> I will look at this as well, and see if anything obvious stands out.
> 
> >
> > Serge Knystautas
> > Loki Technologies
> > http://www.lokitech.com/
> >
> 
> Craig
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

Re: Patch for bug report #362 (and a patch for a logging error)

["Craig R. McClanahan" <Cr...@eng.sun.com>]

Serge Knystautas wrote:

> [snip]
> The way I got this to work was to overload the setter method to take both a
> String and an Object... this got it working through Jasper Servlet but not
> jspc.  I don't know if that's a violation of the spec or the way to handle
> this issue.

Yep, it is, and again this is made explicit in JSP 1.2 :-(.

I was trying something similar (overloaded setters) originally with Struts, and
it went down in flames on most servlet containers because many of them just use
the standard JavaBeans introspector -- and the introspector returns a property
setter only if there is a single method.  On one of the other containers, you'd
start getting compiler errors like "this is a read-only property" because the
introspector returned neither setter.

>  Anyway, seems that if spec compliance is an issue, we should
> have Tomcat 3.2 be uniform in both jspc and the Jasper Servlet.

That is *definitely* true.  Unfortunately, I'm not the person most versed with
the JSP compiler.  Pierre Delisle is, and he will also be back on Monday.

>  I looked
> through the code some and didn't understand why it would compile from the
> Jasper Servlet but not jspc (before my patch), but that's what's happening
> for me with Tomcat 3.2b6.
>

I will look at this as well, and see if anything obvious stands out.

>
> Serge Knystautas
> Loki Technologies
> http://www.lokitech.com/
>

Craig

Re: Patch for bug report #362 (and a patch for a logging error)

[Serge Knystautas <se...@lokitech.com>]

----- Original Message -----
From: "Craig R. McClanahan" <Cr...@eng.sun.com>


> I'm afraid that I disagree with you about whether this change would be
spec
> compliant or not.
[good points trimmed]
> Thus, you also have a workaround.  Assume you've got a custom tag "foo"
that has
> a property (of class java.lang.Object) named "bar".  Your patch would
enable
> saying:
>
>     <mytags:foo bar="abc"/>
>
> which the page compiler will currently complain about.  However, the
following
> will work just fine (assuming you've defined this attribute to accept
runtime
> expressions:
>
>     <mytags:foo bar='<%= "abc" %>'/>
>
> which accomplishes the same thing.

Thanks for shedding light the finer points of the spec... I definitely want
to make my tags so that they are upward compatible.  As for the workaround,
I can't believe that's the desired way to write JSPs as envisioned by the
spec writers, and unfortunately my company has 4 sites up already with 150+
JSP pages that are using the tag in question... I'd rather correct the tag
class rather than those JSPs if at all possible.

> We can ask Eduardo (the spec lead for JSP) about this next week when he
returns
> from vacation, but the intent is pretty clear -- doing this conversion is
> explicitly illegal in JSP 1.2 -- so I'd prefer not to put this change into
> Tomcat 3.2.

Fair enough... hopefully Eduardo can shed some light on the matter.  An
important note though is that my patch only "fixed" the behavior when using
jspc... Jasper servlet compiles the page without a problem, allowing me to
do:

<mytags:foo bar="abc" />

The way I got this to work was to overload the setter method to take both a
String and an Object... this got it working through Jasper Servlet but not
jspc.  I don't know if that's a violation of the spec or the way to handle
this issue.  Anyway, seems that if spec compliance is an issue, we should
have Tomcat 3.2 be uniform in both jspc and the Jasper Servlet.  I looked
through the code some and didn't understand why it would compile from the
Jasper Servlet but not jspc (before my patch), but that's what's happening
for me with Tomcat 3.2b6.

Serge Knystautas
Loki Technologies
http://www.lokitech.com/

Re: Patch for bug report #362 (and a patch for a logging error)

["Craig R. McClanahan" <Cr...@eng.sun.com>]

Serge Knystautas wrote:

> Ok, I submitted the bug report this afternoon (#362) about how jasper was
> mishandling a custom tag of mine.  You can read the report if you want, but
> basically here's what happens... say your custom tag has an attribute to be
> rtexprvalue and because of the flexibility of the tag, you make the setter
> method take a java.lang.Object value.  When you then set a non-dynamic value
> in your JSP (rather than passing some object, just write attr="value"),
> jasper throws this funny message that says it "can't cast String to
> java.lang.Object."
>
> Not a high priority thing, but I think pretty useful (especially since I'd
> like to use these custom tags I wrote!!)  I read over the JSP spec several
> times and don't see how this would be a violation of the spec.  Included
> below is the 2 line patch.
>

Serge,

I'm afraid that I disagree with you about whether this change would be spec
compliant or not.  The reasoning is based on an implication (not clearly
specified in JSP 1.1, but made explicit in 1.2) that the conversions performed
when setting a custom tag attribute are the same as those used by
<jsp:setProperty/>, described in Section 2.13.2 of the JSP 1.1 spec.  In
particular:

    "The only types of properties that can be assigned to from
    String constants and request parameter values are those listed
    in Table 2-4; the conversion applied is that shown in the table.
    Request-time expressions can be assigned to properties of
    any type; no automatic conversion will be performed."

Thus, you also have a workaround.  Assume you've got a custom tag "foo" that has
a property (of class java.lang.Object) named "bar".  Your patch would enable
saying:

    <mytags:foo bar="abc"/>

which the page compiler will currently complain about.  However, the following
will work just fine (assuming you've defined this attribute to accept runtime
expressions:

    <mytags:foo bar='<%= "abc" %>'/>

which accomplishes the same thing.

We can ask Eduardo (the spec lead for JSP) about this next week when he returns
from vacation, but the intent is pretty clear -- doing this conversion is
explicitly illegal in JSP 1.2 -- so I'd prefer not to put this change into
Tomcat 3.2.

Craig