Bug 53792

Summary: EL: AstValue.getTarget() mistakes a method invocation for a property access
Product: Tomcat 7 Reporter: Adrian Moos <adrian.moos>
Component: JasperAssignee: Tomcat Developers Mailing List <dev>
Status: CLOSED FIXED    
Severity: normal    
Priority: P2    
Version: 7.0.27   
Target Milestone: ---   
Hardware: PC   
OS: All   

Description Adrian Moos 2012-08-28 20:24:06 UTC
Trying to evaluate the method expression #{beanFactory.loginBean().init}

Expected behavior: lookup beanFactory, invoke its loginBean method and on its return value, invoke the init method.

Actual behavior: the following exception is thrown: 
javax.el.PropertyNotFoundException: /LoginView.xhtml @9,101 listener="#{viewBeanFactory.loginBean().initIfGetRequest}": Property 'loginBean' not found on type ch.bedag.redacted.InteractionLayer$$EnhancerByCGLIB$$1ea36cb9

Looking at the source, AstValue.getTarget(EvaluationContext) only checks whether the last child is an instanceof AstMethodParameters, but in this expression there is an additional method invocation expression before that ...

Presumably, the while loop 

            while (base != null && i < propCount) {
                property = this.children[i].getValue(ctx);
                base = resolver.getValue(ctx, base, property);

should be something like:

            while (base != null && i < propCount) {
                if (i + 1 < propCount && this.children[i + 1] instanceof AstMethodParameters) {
                    base = // result of method invocation
                    i += 2;
                } else {
                    property = this.children[i].getValue(ctx);
                    base = resolver.getValue(ctx, base, property);
Comment 1 Mark Thomas 2012-08-29 00:11:58 UTC
Having reviewed this against section 1.19 of the EL spec, I can confirm that this should work. The proposed approach looks good to me. I'll take a look at writing some test cases and a fix unless someone beats me to it.
Comment 2 Mark Thomas 2012-08-29 20:49:33 UTC
This works for me and I have added some test cases to prove it. Also, if you want to call the init *method* then the EL should be:

If after reviewing your code, you believe there is still an issue (always a possibility with EL) then the best way to proceed would be to sumbit a patch to the Tomcat unit tests demonstrating the issue.
Comment 3 Adrian Moos 2012-08-30 13:07:27 UTC
That because you have not quite tested the same thing :-)

Yes, #{beanA.setBean(beanB)} works as intended, because its "target" is #{beanA}, which doesn't contain a method invocation expression. That's why I wrote:

> but in this expression there is an additional method invocation expression before that ...

Anyway, here is your test case (to be included in TestMethodExpressionImpl.java):

    public void testBug53792c() {
    	MethodExpression me = factory.createMethodExpression(context, "#{beanA.sayHello().length()}", null, new Class<?>[] {});
    	me.invoke(context, null);

which currently throws: 

javax.el.PropertyNotFoundException: Property 'sayHello' not found on type org.apache.el.TesterBeanA
	at javax.el.BeanELResolver$BeanProperties.get(BeanELResolver.java:237)
	at javax.el.BeanELResolver$BeanProperties.access$1(BeanELResolver.java:234)
	at javax.el.BeanELResolver.property(BeanELResolver.java:325)
	at javax.el.BeanELResolver.getValue(BeanELResolver.java:85)
	at javax.el.CompositeELResolver.getValue(CompositeELResolver.java:67)
	at org.apache.el.parser.AstValue.getTarget(AstValue.java:121)
	at org.apache.el.parser.AstValue.invoke(AstValue.java:245)
	at org.apache.el.MethodExpressionImpl.invoke(MethodExpressionImpl.java:278)
	at org.apache.el.TestMethodExpressionImpl.testBug53792c(TestMethodExpressionImpl.java:473)
Comment 4 Mark Thomas 2012-08-30 18:54:15 UTC
There is no sayHello() method on beanA - it is on beanB.

Looking into the failure with that fix in place...
Comment 5 Mark Thomas 2012-08-30 19:45:51 UTC
Fixed. Thanks for the report and test case.

The fix was a little more extensive than the original suggestion.

Fixed in trunk and 7.0.x and will be included in 7.0.30 onwards.
Comment 6 Adrian Moos 2012-09-03 09:28:42 UTC
Thank you for the speedy response, you set a record for the fastest bugfix I ever saw happen in response to a bug I reported to an open source project :-)