EL processing enhancements for TC 7 (re: [Bug 43819] thread)

[Konstantin Kolinko <kn...@gmail.com>]

Replying to DO NOT REPLY [Bug 43819] Support latest JSR245 proposal to
make EL "self-contained"
http://markmail.org/message/gx3zg5h47x32xdgt

2010/1/4 Mark Thomas <ma...@apache.org>:
>
> My first attempt to add method invocation support hit an issue in that
> the Jasper parser thought it was a function. In an effort of avoid
> adding method invocation support to the Jasper parser and the EL parser,
> I am now looking at this re-factoring.
>
> There is an issue in that the EL parser does not provide a mechanism to
> distinguish between ${...} and #{...} so Jasper will have to do a little
> parsing. I've refactored this part and am currently looking to see if I
> can refactor the function validation code to use the EL parser. My
> current ideas are around using a function mapper that checks the TLDs
> when resolve() is called during the parsing process but I haven't had a
> chance to try coding it yet. Hopefully, I'll get some time early this week.
>
> Mark
>

Will you change the parsing, or it will affect validation only? I was
not looking in the sources, so I have some difficulty to grasp what
you are mentioning above.

The following is my somewhat half-baked opinion based on my investigation
around the ${a}${b} composite expressions bug, which itself was caused
by a fix for quotes processing bugs.


1. There are certain differences in escaping rules between JSP pages,
JSP documents and EL expressions.

In the EL expressions themselves there are differences in escaping
between Literal-expressions (EL.1.2.2) (the chapter numbers are from
"Expression Language 2.2" spec) and Eval-expressions (EL.1.2.1).

The escaping rules in EL.1.3 apply to the string literals in
Eval-expressions, but not to Literal expressions.

Specifically, the following test in TestELEvaluation.java is wrong:

  assertEquals("\\", evaluateExpression("\\\\"));

It should be
  assertEquals("\\\\", evaluateExpression("\\\\"));
  assertEquals("\\", evaluateExpression("${'\\\\'}"));

See also
https://jsp-spec-public.dev.java.net/issues/show_bug.cgi?id=165
and
JSP.10.1.11 Request-Time Attribute Expressions

2. I am hesitant to try fixing the EL parser right now (at least until
we release the next 6.0.x), because I suspect that it leverages
certain Jasper behavior.

3. Testing compliance with the escaping rules should involve JSP pages
and Jasper.

TCK helps here, but there are cases that it does not catch. I do not
have TCK access, but I see from bug reports that were fixed thus far.

Also the behavior triggered by
org.apache.jasper.compiler.Parser.STRICT_QUOTE_ESCAPING property is
not tested by TCK.

4.
> parsing. I've refactored this part and am currently looking to see if I
> can refactor the function validation code to use the EL parser. My

Currently when generating Java code Jasper evaluates the composite
expressions in the following way:
a) It parses an expression and splits it into Eval and Literal
expressions, according to JSP escaping rules.
b) Literal expressions are processed as is, and only Eval expressions
are passed to the EL interpreter.
c) The result is concatenated, and has to be coerced to the expected type.

This becomes a bit complicated.

There could be another implementation: re-escape the Literal
expression (e.g. replacing '\' with ${'\\'}), recombine the
expressions and pass it to the EL interpreter.

I think that you will face these issues, so here is my warning.



Also, in the Java code generated from
/examples/jsp/jsp2/el/composite.jsp (of TC7)
I see some example, where composite EL expression is evaluated as is.
I suspect that it will exhibit the quoting bugs, but I have not payed
it much attention yet:

> org.apache.jasper.runtime.JspRuntimeLibrary.handleSetPropertyExpression(
> _jspx_page_context.findAttribute("values"), "long", "000${1}${7}",
> _jspx_page_context, null);

The above is generated from
<jsp:setProperty name="values" property="long" value="000${1}${7}"/>


We also have https://issues.apache.org/bugzilla/show_bug.cgi?id=48112


Best regards,
Konstantin Kolinko

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

Re: EL processing enhancements for TC 7 (re: [Bug 43819] thread)

[Mark Thomas <ma...@apache.org>]

On 04/01/2010 15:01, Konstantin Kolinko wrote:
> Will you change the parsing, or it will affect validation only? I was
> not looking in the sources, so I have some difficulty to grasp what
> you are mentioning above.

The current parsing is going to have to change to handle the addition of
method invocation to the unifed EL.

> 1. There are certain differences in escaping rules between JSP pages,
> JSP documents and EL expressions.
> 
> In the EL expressions themselves there are differences in escaping
> between Literal-expressions (EL.1.2.2) (the chapter numbers are from
> "Expression Language 2.2" spec) and Eval-expressions (EL.1.2.1).
> 
> The escaping rules in EL.1.3 apply to the string literals in
> Eval-expressions, but not to Literal expressions.
> 
> Specifically, the following test in TestELEvaluation.java is wrong:
> 
>   assertEquals("\\", evaluateExpression("\\\\"));
> 
> It should be
>   assertEquals("\\\\", evaluateExpression("\\\\"));
>   assertEquals("\\", evaluateExpression("${'\\\\'}"));
> 
> See also
> https://jsp-spec-public.dev.java.net/issues/show_bug.cgi?id=165
> and
> JSP.10.1.11 Request-Time Attribute Expressions

I agree with your analysis. I wasn't planning on addressing any of this
at this stage.

> 2. I am hesitant to try fixing the EL parser right now (at least until
> we release the next 6.0.x), because I suspect that it leverages
> certain Jasper behavior.

+1. Past experience is that the EL implementation is very fragile.

> 3. Testing compliance with the escaping rules should involve JSP pages
> and Jasper.

+1. In Tomcat 7 it is much easier to write test cases that use an
embedded Tomcat instance for things like this. I have some test JSPs
kicking around - I'll try and add them to the Tomcat 7 code base.

> TCK helps here, but there are cases that it does not catch. I do not
> have TCK access, but I see from bug reports that were fixed thus far.

That is easy to arrange if you would like TCK access.

> Also the behavior triggered by
> org.apache.jasper.compiler.Parser.STRICT_QUOTE_ESCAPING property is
> not tested by TCK.

Yep. More test cases required...

> 4.
>> parsing. I've refactored this part and am currently looking to see if I
>> can refactor the function validation code to use the EL parser. My
> 
> Currently when generating Java code Jasper evaluates the composite
> expressions in the following way:
> a) It parses an expression and splits it into Eval and Literal
> expressions, according to JSP escaping rules.
> b) Literal expressions are processed as is, and only Eval expressions
> are passed to the EL interpreter.
> c) The result is concatenated, and has to be coerced to the expected type.
> 
> This becomes a bit complicated.

It gets worse as Jasper then parses the Eval expression as well as
passing it to the EL code.

> There could be another implementation: re-escape the Literal
> expression (e.g. replacing '\' with ${'\\'}), recombine the
> expressions and pass it to the EL interpreter.

That is probably worth considering once the other changes are in place
to see if it simplifies things at all.

> Also, in the Java code generated from
> /examples/jsp/jsp2/el/composite.jsp (of TC7)
> I see some example, where composite EL expression is evaluated as is.
> I suspect that it will exhibit the quoting bugs, but I have not payed
> it much attention yet:
> 
>> org.apache.jasper.runtime.JspRuntimeLibrary.handleSetPropertyExpression(
>> _jspx_page_context.findAttribute("values"), "long", "000${1}${7}",
>> _jspx_page_context, null);
> 
> The above is generated from
> <jsp:setProperty name="values" property="long" value="000${1}${7}"/>
> 
> 
> We also have https://issues.apache.org/bugzilla/show_bug.cgi?id=48112

Yep. Need to look at that too.

Mark



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