# tomcat-dev mailing list archives

##### Site index · List index
Message view
Top
Subject Re: EL issues and 6.0.x release
Date Mon, 01 Feb 2010 09:36:31 GMT
2010/1/31 Mark Thomas <markt@apache.org>:
> On 30/01/2010 17:41, Mark Thomas wrote:
>> 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.
>
> Looks like it will work. Just running the TCK to be sure.
>
>>> 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. > > Yep bug. Test cases written. Fixed. Just running the TCK to be sure. > >>> 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.
>
> Test case added. This is already handled by the EL impl.
>

If we  have "#{foo}${bar}" then parseEL() won't be triggered for the${bar} part, and so won't correctly parse quotes etc.

Though probably that makes not much of a difference.  I think
${foo}#{bar will be treated as an error regardless of what garbage or not that bar is . As of now <tags:echo echo="${foo}#{bar}" />
<tags:echo echo="${foo}#{bar" /> or "legal" <tags:echo echo="#{foo}" /> all result in the same compile-time exception: "According to TLD or attribute directive in tag file, attribute echo does not accept any expressions" The "any expressions" part of the message text is wrong. All that is a minor/cosmetic issue. I won't care much if it is postponed for later or not fixed in a while at all. >>> 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. > > Proposed. > > Assuming the TCK passes I'll have a fix for 46827 and > deferredSyntaxAllowedAsLiteral shortly. > One error there (for AttributeParser.java of trunk at rev.904949): In parseEL() the literalQuote variable should be moved outside the while (i < size && !endEL) loop. Currently its value is not preserved among iterations. E.g. <tags:echo echo="${\"foo\"}\\${'bar'}" /> <tags:echo echo="${'foo'}\\${\"bar\"}" /> should both evaluate to foo\bar, but now the first one evaluates to foo${'bar'}

The rest is OK. I do not see any other errors in the implementation.

Best regards,