Allow Thymeleaf to check for allowed method access of Java supertypes

[jkuipers]

What does this PR do?

To prevent Thymeleaf templates from accessing methods on built-in Java types that are not allowed, it ships with a list of allowed supertypes. Its org.thymeleaf.util.ExpressionUtils type has an isMemberAllowed method that calls getDeclaredMethods() on those types, so that needs to work for all these types in a native image. This PR ensures that.

Recent versions of Thymeleaf have expanded the list with two additional types, which is why the M2 version has 7 new entries while the RC1 / latest has 9. For details, check out
the source code.

Code sections where the PR accesses files, network, docker or some external service

None.

Checklist before merging

• I have considered including reachability metadata directly in the library or the framework (see our contributing document)
• I am the original author of all content provided in the pull request, and I did not copy the content from any other source (see the tests section from our contributing document)
• For all tests where I am not the sole author, I have added a comment that proves I may publish them under the specified license (see the tests section from our contributing document)
• I have properly formatted metadata files (see our formatting guide)
• I have added thorough tests (see the tests section from our contributing document)
• I have filled all places where my pull request accesses files, network, docker, or any other external service (see the section above)

[fniephaus]

Thanks a lot for contributing, @jkuipers!
Would be great if you could make sure the relevant code is exercised in the corresponding tests, so that this cannot accidentally break: https://github.com/oracle/graalvm-reachability-metadata/tree/master/tests/src/org.thymeleaf/thymeleaf

[jkuipers]

Happy to add some tests. Note that this code is only exercised by the framework when you use Spring or OGNL expressions, but the required changes have to go in the core project. @fniephaus is it sufficient to simply add some tests that invoke the org.thymeleaf.util.ExpressionUtils#isMemberAllowed method with objects of the relevant types? If not, the tests for the core framework would require additional dependencies to bring in OGNL so that I can call the OGNLVariableExpressionEvaluator, which to me seems like overkill and unnecessary complexity.

[fniephaus]

No need to pull in addition dependencies if you can invoke the code somehow directly, that is without accessing any private APIs reflectively or so.

[jkuipers]

I added tests. Note that I had to update the existing date-based test, because it failed on my local machine (which uses a Dutch locale) without an explicit format.

[jkuipers]

Hey, is anything still missing that's causing this to stay open? I was hoping that this could get merged in its current state.

[melix]

I order to approve or not, I need to understand a bit better why this is needed. This adds reflection metadata for all public methods of types from the JDK (guarded by the ExpressionUtils access), which can be quite expensive. Is this really needed or can we narrow more the methods which would be accessed reflectively?

[jkuipers]

I order to approve or not, I need to understand a bit better why this is needed. This adds reflection metadata for all public methods of types from the JDK (guarded by the ExpressionUtils access), which can be quite expensive. Is this really needed or can we narrow more the methods which would be accessed reflectively?

Hey, thanks for getting back to me. The Thymeleaf ExpressionUtils class is used when a template uses an expression (via Spring expression language or OGNL) to check whether that's an allowed call. It contains a hardcoded list of Java types for which it allows calls by reflectively iterating over all their declared methods and comparing the resulting names to the memberName that's being passed to the method.
Without the declared methods being available in the GraalVM metadata, the check will simply always fail when those methods are called from a template and won't allow the method to be accessed. I ran into this while iterating over the entries of a map and rendering their keys and values. Something like this then fails:

 <tr th:each="entry: ${queuesByName}">
  <td th:text="${entry.key}">queue name</td>

Of course I cannot predict what declared methods of those types would be used in practice by all possible Thymeleaf users, so my PR is based on wanting the ExpressionUtils to simply behave the same as when running on a JVM.
Note that I am myself not associated with the Thymeleaf project: I'm just a user that expected the Spring/Thymeleaf combination to work with GraalVM, as the Spring team has already provided some basic reachability data for both Thymeleaf itself as well as the Spring support for it here. However, that's not sufficient when you use basic functionality like what I showed here. When I ran into this I analyzed why this failed and what Thymeleaf needs to make this work in a native image and then created this PR by hand (i.e. not using any agents or other tools) based on the contents of the ExpressionUtils class. I don't see how you could limit this further based on the code that's in the ExpressionUtils class.
My understanding is that GraalVM won't include the ExpressionUtils when it's not actually used by the code being compiled, so this seemed like a good guarding clause.

[jkuipers]

@melix Hey, did you manage to have a look at my explanation and the Thymeleaf code that this PR allows to work in a native image? Without this, most real-world applications using Thymeleaf will not work correctly, which is extra confusing since this repo does already contain reachability metadata for it (I had assumed that this would've worked out the box when I started to use this).

In general I think that for a templating solution that relies on reflection it seems reasonable that some additional metadata is included, since you cannot predict what access templates will require, but in this particular case Thymeleaf even actively denies calls to common Java operations (esp. calls to collections, which are used all over the place in templates) without this metadata: it took me some time to figure this out and implement a fix in my own applications, it seems better to have this work out of the box and perhaps file an issue with the Thymeleaf team to optimize this for GraalVM in future releases so that the overhead can be reduced then.

[melix]

@jkuipers sorry for the late answer, I'm back from a break. The explanation makes sense to me.

[jkuipers]

@jkuipers sorry for the late answer, I'm back from a break. The explanation makes sense to me.

I'm a bit confused: I was assuming that that means that this can be merged, but the issue stays open and has been for two months now. Is anything else expected from my side?

[melix]

Thanks for your patience, I just merged it.