You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Danno Ferrin <sh...@earthlink.net> on 2000/10/10 07:20:44 UTC

[BUG #40] Attribute Expressions evaluated out of order

The bug is that 2.12.1 of the 1.1 spec says that expressions are
evaluated right to left in attributes of an action.  That is no the case
right now, as can be demonstrated by replacing the tld and foo.jsp in
the simpletag example with the attached files.  The golden page should
be the attached foo.jsp.golden but the execution is quite different (the
attached fo.jsp.htm).  Some test like this needs to be put into watchdog
but I am so out of touch with what is going on in there I probably am
not the best person to do it.

The problem comes in several of the interface methods in
org.apache.jasper.compiler.ParseEventListener (14 methods in all if my
count is correct) use Hashtable to pass attributes, which ruins the
ordering (by definition).  To fix this there are several options, in
order of my preference, of fixing them:

1) Replace Hashtable with org.sax.helpers.AttributesImpl, it maintains
order, name based lookup, and is an existing class.  (possibly
AttributeListImpl, but it is deprecated but then again it exists in
SAX1).

2) Roll our own equivalent of AttributesImpl, the disadvantage is
duplicate code.  The advantage is no non-jasper classes in the
interfaces (even if they are standard APIs such as SAX).

3) pass in a N length array giving the original order of the attributes
passed in.

4) Pass in a 2xN string array, and implement name based lookup by hand
(ugly)


Another question is of we should try and get it into 3.2.  This is a
conformance issue so it may be valuable, but then again the release of
3.2 is so late this would push it back at least another month for
shake-down time.  I plan on dropping it into 3.3 and 4.0 when I get a
vibe on what is the preferred solution.  There will also be a lot of
cascading changes, which may be up to half of the class in the
...jasper.compiler package, so this will be a big patch, hence my
apprehension to putting it into the 3.2 tree.

Please respond with your opinions about what should be done, but I would
like to start writing the code changes on Wednesday so hopefully I could
get it fixed before I go on vacation next week, otherwise it will be the
last week in Oct.  But if the vibe is that it's not going into 3.2 and a
beta/alpha of 3.3 and 4.0 isn't coming until Nov., then it could wait
until after I get back unless someone else does it.

--Danno

Re: [BUG #40] Attribute Expressions evaluated out of order

Posted by Danno Ferrin <sh...@earthlink.net>.
Actually, the JSP1.1 errata a issue 16 fixes the list, it should read

"The following attributes accept request time attribute expressions:
The value attribute of jsp:setProperty (2.13.2).
The beanName attribute of jsp:useBean (2.13.1).
The page attribute of jsp:include (2.13.4).
The page attribute of jsp:forward (2.13.5).
The value attribute of jsp:param (2.13.6)."

So the only thing this affects is the custom actions.  The current count
of files affected by the cascading change of option 1 is 19 (out of
about 60 files).  Plus I haven't run it through watchdog or the
integrated tests yet.  This makes option 3 a bit more palatable, but
some of the things I would need to change in Parser shouts out bubble
gum and duct tape, so it wouldn't hurt to re-factor.  I will probably
check it in on a branch since I won't be able to QA it next week or deal
with breakages... then in late Oct. I would move it over to the main 4.0
line once I can deal with the breakages.

--Danno

Pierre Delisle wrote:
> 
> Danno,
> 
> While most handlers in JspParseEventListener have the
> "Hashtable attrs" argument, many of them do not allow
> request-time attribute values and would not be impacted
> by that bug (evaluation order only applies
> to request-time attributes).
> 
> I have not verified if that is exhaustive, but the spec
> only mentions the following request-time attributes for
> JSP actions:
> 
>     "The following attributes accept request-time attribute expressions:
> 
>     · The value and beanName attributes of jsp:setProperty (2.13.2).
>     · The page attribute of jsp:include (2.13.4).
>     · The page attribute of jsp:forward (2.13.5).
>     · The value attribute of jsp:param (2.13.6)."
> 
> If we discard the ones that only have a single rt-attribute,
> that leaves us only with 'jsp:setProperty' and custom tags.
> [Please correct me if I am wrong].
> 
> This being said, if we still want to clean it all up,
> my preferences are listed below.
> 
> -----
> * Which option to fix the bug
> 
>   I vote for 1): replace hashtable with org.sax.helpers.AttributesImpl.
>   This is already how we get the attributes when handling JSPx
>   (JSP page as XML doc) via the SAX2 parser.
>   We already have the dependency and I don't see it going away
>   in the future.
> 
> * Get it into 3.2?
> 
>   No, to avoid delaying 3.2. I'd be surprised (but may well be wrong)
>   that many JSP pages rely on the attributes evaluation order
>   (probably only while using custom tags).
>   Let's clearly document the bug though.
> 
> * When to fix
> 
>   As soon as you can would be nice for 4.0 at least.
> 
>     -- Pierre
> 
> Danno Ferrin wrote:
> >
> > The bug is that 2.12.1 of the 1.1 spec says that expressions are
> > evaluated right to left in attributes of an action.  That is no the case
> > right now, as can be demonstrated by replacing the tld and foo.jsp in
> > the simpletag example with the attached files.  The golden page should
> > be the attached foo.jsp.golden but the execution is quite different (the
> > attached fo.jsp.htm).  Some test like this needs to be put into watchdog
> > but I am so out of touch with what is going on in there I probably am
> > not the best person to do it.
> >
> > The problem comes in several of the interface methods in
> > org.apache.jasper.compiler.ParseEventListener (14 methods in all if my
> > count is correct) use Hashtable to pass attributes, which ruins the
> > ordering (by definition).  To fix this there are several options, in
> > order of my preference, of fixing them:
> >
> > 1) Replace Hashtable with org.sax.helpers.AttributesImpl, it maintains
> > order, name based lookup, and is an existing class.  (possibly
> > AttributeListImpl, but it is deprecated but then again it exists in
> > SAX1).
> >
> > 2) Roll our own equivalent of AttributesImpl, the disadvantage is
> > duplicate code.  The advantage is no non-jasper classes in the
> > interfaces (even if they are standard APIs such as SAX).
> >
> > 3) pass in a N length array giving the original order of the attributes
> > passed in.
> >
> > 4) Pass in a 2xN string array, and implement name based lookup by hand
> > (ugly)
> >
> > Another question is of we should try and get it into 3.2.  This is a
> > conformance issue so it may be valuable, but then again the release of
> > 3.2 is so late this would push it back at least another month for
> > shake-down time.  I plan on dropping it into 3.3 and 4.0 when I get a
> > vibe on what is the preferred solution.  There will also be a lot of
> > cascading changes, which may be up to half of the class in the
> > ...jasper.compiler package, so this will be a big patch, hence my
> > apprehension to putting it into the 3.2 tree.
> >
> > Please respond with your opinions about what should be done, but I would
> > like to start writing the code changes on Wednesday so hopefully I could
> > get it fixed before I go on vacation next week, otherwise it will be the
> > last week in Oct.  But if the vibe is that it's not going into 3.2 and a
> > beta/alpha of 3.3 and 4.0 isn't coming until Nov., then it could wait
> > until after I get back unless someone else does it.
> >
> > --Danno
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

Re: [BUG #40] Attribute Expressions evaluated out of order

Posted by Pierre Delisle <pi...@sun.com>.
Danno,

While most handlers in JspParseEventListener have the 
"Hashtable attrs" argument, many of them do not allow
request-time attribute values and would not be impacted
by that bug (evaluation order only applies 
to request-time attributes).

I have not verified if that is exhaustive, but the spec 
only mentions the following request-time attributes for
JSP actions:

    "The following attributes accept request-time attribute expressions:
    
    · The value and beanName attributes of jsp:setProperty (2.13.2).
    · The page attribute of jsp:include (2.13.4).
    · The page attribute of jsp:forward (2.13.5).
    · The value attribute of jsp:param (2.13.6)."

If we discard the ones that only have a single rt-attribute,
that leaves us only with 'jsp:setProperty' and custom tags.
[Please correct me if I am wrong].

This being said, if we still want to clean it all up,
my preferences are listed below.

-----
* Which option to fix the bug

  I vote for 1): replace hashtable with org.sax.helpers.AttributesImpl.
  This is already how we get the attributes when handling JSPx 
  (JSP page as XML doc) via the SAX2 parser.
  We already have the dependency and I don't see it going away
  in the future.

* Get it into 3.2?

  No, to avoid delaying 3.2. I'd be surprised (but may well be wrong)
  that many JSP pages rely on the attributes evaluation order
  (probably only while using custom tags).
  Let's clearly document the bug though.

* When to fix

  As soon as you can would be nice for 4.0 at least.


    -- Pierre


Danno Ferrin wrote:
> 
> The bug is that 2.12.1 of the 1.1 spec says that expressions are
> evaluated right to left in attributes of an action.  That is no the case
> right now, as can be demonstrated by replacing the tld and foo.jsp in
> the simpletag example with the attached files.  The golden page should
> be the attached foo.jsp.golden but the execution is quite different (the
> attached fo.jsp.htm).  Some test like this needs to be put into watchdog
> but I am so out of touch with what is going on in there I probably am
> not the best person to do it.
> 
> The problem comes in several of the interface methods in
> org.apache.jasper.compiler.ParseEventListener (14 methods in all if my
> count is correct) use Hashtable to pass attributes, which ruins the
> ordering (by definition).  To fix this there are several options, in
> order of my preference, of fixing them:
> 
> 1) Replace Hashtable with org.sax.helpers.AttributesImpl, it maintains
> order, name based lookup, and is an existing class.  (possibly
> AttributeListImpl, but it is deprecated but then again it exists in
> SAX1).
> 
> 2) Roll our own equivalent of AttributesImpl, the disadvantage is
> duplicate code.  The advantage is no non-jasper classes in the
> interfaces (even if they are standard APIs such as SAX).
> 
> 3) pass in a N length array giving the original order of the attributes
> passed in.
> 
> 4) Pass in a 2xN string array, and implement name based lookup by hand
> (ugly)
> 
> Another question is of we should try and get it into 3.2.  This is a
> conformance issue so it may be valuable, but then again the release of
> 3.2 is so late this would push it back at least another month for
> shake-down time.  I plan on dropping it into 3.3 and 4.0 when I get a
> vibe on what is the preferred solution.  There will also be a lot of
> cascading changes, which may be up to half of the class in the
> ...jasper.compiler package, so this will be a big patch, hence my
> apprehension to putting it into the 3.2 tree.
> 
> Please respond with your opinions about what should be done, but I would
> like to start writing the code changes on Wednesday so hopefully I could
> get it fixed before I go on vacation next week, otherwise it will be the
> last week in Oct.  But if the vibe is that it's not going into 3.2 and a
> beta/alpha of 3.3 and 4.0 isn't coming until Nov., then it could wait
> until after I get back unless someone else does it.
> 
> --Danno

Re: [BUG #40] Attribute Expressions evaluated out of order

Posted by "Craig R. McClanahan" <Cr...@eng.sun.com>.
My vote would be to include this as a "Known Issue" in the release notes for
3.2 and ship the blasted thing.  This is way to big a change to just try to
squeeze in at the last minute without a significant delay.

Craig


Danno Ferrin wrote:

> The bug is that 2.12.1 of the 1.1 spec says that expressions are
> evaluated right to left in attributes of an action.  That is no the case
> right now, as can be demonstrated by replacing the tld and foo.jsp in
> the simpletag example with the attached files.  The golden page should
> be the attached foo.jsp.golden but the execution is quite different (the
> attached fo.jsp.htm).  Some test like this needs to be put into watchdog
> but I am so out of touch with what is going on in there I probably am
> not the best person to do it.
>
> The problem comes in several of the interface methods in
> org.apache.jasper.compiler.ParseEventListener (14 methods in all if my
> count is correct) use Hashtable to pass attributes, which ruins the
> ordering (by definition).  To fix this there are several options, in
> order of my preference, of fixing them:
>
> 1) Replace Hashtable with org.sax.helpers.AttributesImpl, it maintains
> order, name based lookup, and is an existing class.  (possibly
> AttributeListImpl, but it is deprecated but then again it exists in
> SAX1).
>
> 2) Roll our own equivalent of AttributesImpl, the disadvantage is
> duplicate code.  The advantage is no non-jasper classes in the
> interfaces (even if they are standard APIs such as SAX).
>
> 3) pass in a N length array giving the original order of the attributes
> passed in.
>
> 4) Pass in a 2xN string array, and implement name based lookup by hand
> (ugly)
>
> Another question is of we should try and get it into 3.2.  This is a
> conformance issue so it may be valuable, but then again the release of
> 3.2 is so late this would push it back at least another month for
> shake-down time.  I plan on dropping it into 3.3 and 4.0 when I get a
> vibe on what is the preferred solution.  There will also be a lot of
> cascading changes, which may be up to half of the class in the
> ...jasper.compiler package, so this will be a big patch, hence my
> apprehension to putting it into the 3.2 tree.
>
> Please respond with your opinions about what should be done, but I would
> like to start writing the code changes on Wednesday so hopefully I could
> get it fixed before I go on vacation next week, otherwise it will be the
> last week in Oct.  But if the vibe is that it's not going into 3.2 and a
> beta/alpha of 3.3 and 4.0 isn't coming until Nov., then it could wait
> until after I get back unless someone else does it.
>
> --Danno
>
>   ------------------------------------------------------------------------
>                          Name: example-taglib.tld
>    example-taglib.tld    Type: unspecified type (application/octet-stream)
>                      Encoding: 7bit
>
>               Name: foo.jsp
>    foo.jsp    Type: application/x-unknown-content-type-jspfile
>           Encoding: quoted-printable
>
>                      Name: foo.jsp.golden
>    foo.jsp.golden    Type: unspecified type (application/octet-stream)
>                  Encoding: quoted-printable
>
>                   Name: foo.jsp.htm
>    foo.jsp.htm    Type: Hypertext Markup Language (text/html)
>               Encoding: quoted-printable
>
>   ------------------------------------------------------------------------
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org

--
====================
See you at ApacheCon Europe <http://www.apachecon.com>!
Session VS01 (23-Oct 13h00-17h00):  Sun Technical Briefing
Session T06  (24-Oct 14h00-15h00):  Migrating Apache JServ
                                    Applications to Tomcat