On 30/01/2010 07:33, Konstantin Kolinko wrote:
> Regarding the implementation, AttributeParser.java class. I think
> that, based on the above, we can fix it to solve bug 48627. Other
> parts of the new implementation will remain unchanged.

I'll take another look at this. I thought that this wouldn't work but
that may because I was doing my testing before I fixed the EL parsing.
If this doesn't work I have an alternative plan.

> 1. In JSP 2.1 spec there is an option to selectively disable '#'
> expressions when '$' ones are still enabled. The name of that option
> is "deferredSyntaxAllowedAsLiteral".
> 
> As of now, AttributeParser takes care of isELIgnored option, but does
> not know about deferredSyntaxAllowedAsLiteral one.

Probably a bug. We should write some test cases for this first though to
check.

> 2. There are several places in AttributeParser#parseLiteral() where
> 
>                 if (type == 0) {
>                     type = '$';
>                 }
> 
> That is where the bug 48627 lies. The above code turns non-dynamic
> attribute in a dynamic one.

Agreed.

> 3. EL spec (ch.1.2.3 of EL 2.1 spec) says that "It is illegal to mix
> ${} and #{} constructs in a composite expression." though followed by
> "This restriction may be lifted in future versions".
> 
> AttributeParser#parseLiteral() has the following clause:
> 
> } else if (ch == type){
> 
> I think it has to process '#' and '$' expressions in the same way, and
> the "mix ${} and #{}" rule should be checked either explicitly here,
> or elsewhere. I have not researched the question where it is actually
> checked.

More tests cases required.

> 4. I have not researched this question, and I *can be wrong* in this
> point, but I think that when EL is evaluated, the engine does not know
> about  isELIgnored and deferredSyntaxAllowedAsLiteral options.

The parser does know. Parsing is done in two phases. The first phase
parses directives, the second phase parses everything else. This ensures
when everything else is parsed, the parser knows the correct way to
handle stuff that might be an expression.

See http://svn.apache.org/viewvc?view=revision&revision=708165

> Lastly,
> when Mark was testing TC7 with JSP 2.2 TCK, he caught several minor EL
> evaluation issues. Those are fixed in TC7, and I think some of them
> have to be backported to TC6.

They all need back-porting. I didn't propose them at the time since the
issues had existing for all of the 6.0.x release and no-one had
complained. I didn't want to hold up the 6.0.24 release.

Thanks for the review.

Mark



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

Reply via email to