tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Konstantin Kolinko <knst.koli...@gmail.com>
Subject Re: svn commit: r791224 - in /tomcat/trunk/java/org/apache/jasper/compiler: Generator.java PageInfo.java
Date Sun, 05 Jul 2009 20:16:36 GMT
2009/7/5 Mark Thomas <markt@apache.org>:
> Konstantin Kolinko wrote:
>> 2009/7/5  <markt@apache.org>:
>>> Author: markt
>>> Date: Sun Jul  5 11:30:22 2009
>>> New Revision: 791224
>>>
>>> URL: http://svn.apache.org/viewvc?rev=791224&view=rev
>>> Log:
>>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=38797
>>> Revert previous fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=37933
and implement an alternative that doesn't have the side effects described in 38797
>>>
>>> Modified:
>>>    tomcat/trunk/java/org/apache/jasper/compiler/Generator.java
>>>    tomcat/trunk/java/org/apache/jasper/compiler/PageInfo.java
>>>
>>> --- tomcat/trunk/java/org/apache/jasper/compiler/Generator.java (original)
>>> +++ tomcat/trunk/java/org/apache/jasper/compiler/Generator.java Sun Jul  5 11:30:22
2009
>>> ...
>>> +            // Add the named objects to the lits of 'introduced' names
to enable
>>> +            // a later test as per JSP.5.3
>>> +            VariableInfo[] infos = n.getVariableInfos();
>>> +            if (infos != null && infos.length > 0) {
>>> +                for (int i = 0; i < infos.length; i++) {
>>> +                    VariableInfo info = infos[i];
>>> +                    if (info != null && info.getVarName()
!= null)
>>> +                    pageInfo.getVarInfoNames().add(info.getVarName());
>>> +                }
>>> +            }
>>
>> I do not think that the above fragment is right.
>>
>> I have not tested it, but it looks like
>> 1. It does not take VariableInfo#scope into account
> What part of JSP.5.3 makes you think that it should?
>
>> 2. I think that it does not work for tag files that declare variables.
> Again, what part of JSP.5.3 makes you think that it should?
>

OK.

I just thought that it is strange, that only VariableInfo is
mentioned, and it is not the only way to introduce variables (there it
says "object").

but now looking at JSP 1.2 spec, the problem was already there: it
explicitly mentions VariableInfo, but not mentions TagVariableInfo.
Also, it is even more restrictive than 2.0 and 2.1 versions: it does
not mention that ignoring this error is allowed.

So, let's do it as written, verbatim. - as you did.


>> Also,
>> 3. Node.UseBean node does not add anything to that set of variable names.
> It doesn't need to. That is what the BeanRepository instance is for.
>

Oh, I see that now.


Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message