Bug 66235 - Java Beans Standard not honoured in EL expressions when running under Graal VM (JVM Runtime Mode)
Summary: Java Beans Standard not honoured in EL expressions when running under Graal V...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Jasper (show other bugs)
Version: 9.0.x
Hardware: PC All
: P2 normal (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-08-23 16:34 UTC by matthias.hiller
Modified: 2022-08-24 12:23 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description matthias.hiller 2022-08-23 16:34:28 UTC
I recently tried to run an existing application under tomcat 9 with the Graal VM in JVM Runtime Mode, that means not as an Native exceutable, see https://www.graalvm.org/22.2/docs/introduction/#runtime-modes ).

I have a class similar like

public class MyBean {
  public String getText(Locale locale) { ... }
  public String getText() { ... }
}

When I use an EL expression like ${pageScope.mybean.text} it fails (returns null/an empty string) under certain conditions.

I think the problem is the following:

a) the JasperELResolver uses an GraalBeanELResolver when it thinks it runs under graal (see https://github.com/apache/tomcat/blob/main/java/org/apache/jasper/el/JasperELResolver.java#L72 ), I'm not sure if the GraalBeanELResolver would be really needed under Graal VM in JVM Runtime mode as this one should support reflection. But at the moment the JasperELResolver/JspRuntimeLibrary.GRAAL is configured to to so. 

b) the GraalBeanELResolver tries to find the getter of my property, see https://github.com/apache/tomcat/blob/79173cfbcaf6e82972cce98fbdc619e227037ab8/java/org/apache/jasper/el/JasperELResolver.java#L243 , to do so it simply iterates over all methods and tries to find "getText", it simply uses the first one. In my example above it depends on the order of beanClass.getMethods() if this will succeed at the end.

I think the GraalBeanELResolver should use the same logic as the default BeanELResolver which uses the java.beans.PropertyDescriptor.getReadMethod, this means to:
  - only search for getters without arguments
  - also honor the special "is" logic for boolean arguments

I only checked the code for reading (getValue) at the moment, but I guess also the setValue might have similar issues.

You might say that the above class MyBean is a bad architecture, but nevertheless with Graal VM the application behaves different than with standard JDK.

Above I wrote that the code fails in my case under "certain conditions". The exact difference was that it only failed when I had an "serletContext.setAttribute(ELInterpreterFactory.EL_INTERPRETER_CLASS_NAME, someCustomElInterpreter);"
in place. Nevertheless, this custom el interpreter did not interprete this special el expression on its own but used JspUtil.interpreterCall (which usees the default mechanism). But while debugging I saw that the main difference was the order of beanClass.getMethods() inside GraalBeanELResolver. I don't know how this order is determined. But in my optinion the GraalBeanELResolver should not rely on any specific order as the javadoc of getMethods says: "The elements in the returned array are not sorted and are not in any particular order."
Comment 1 matthias.hiller 2022-08-23 16:40:56 UTC
By the way the commit integrating the GraalBeanELResolver already had an comment about similar problems but it was ignored.
See https://github.com/apache/tomcat/commit/4f5bdbc95f1ad1152396c615aab65b2137d30c7f#commitcomment-39709814
Comment 2 Remy Maucherat 2022-08-23 17:58:57 UTC
(In reply to matthias.hiller from comment #1)
> By the way the commit integrating the GraalBeanELResolver already had an
> comment about similar problems but it was ignored.
> See
> https://github.com/apache/tomcat/commit/
> 4f5bdbc95f1ad1152396c615aab65b2137d30c7f#commitcomment-39709814

The basic problem is that something like the default Tomcat landing page will no longer work out of the box (there's no tracing last time I tried, it needs horrible manual config). So I think this is unusable and the replacement is always a better option, but there's room for adding the couple extra hacks you mention (like checking the params for get/set and the alternate "is" for get). This will take care of all current uses I think.
Comment 3 Remy Maucherat 2022-08-24 08:42:05 UTC
I added fixes to GraalBeanELResolver and added tests. This will be in 10.1-M18, 10.0.24. 9.0.66.
Comment 4 matthias.hiller 2022-08-24 10:56:55 UTC
Thanks for this quick fix.
When looking at the commit I have 2 remarks. I think there are still some small differences, that might result in different behavior:

BeanELResolver: "is" is only used for boolean or null type
GraalBeanELResolver: "is" is used for all types

BeanELResolver: if "get" and "is" exists both, "is" is preferred over "get"
GraalBeanELResolver: "get" is preferred over "is"
Comment 5 Remy Maucherat 2022-08-24 12:23:05 UTC
Thanks for the additional tweaks.