Summary: | Prevent illegal reflective access warnings / errors from BeanELResolver | ||
---|---|---|---|
Product: | Tomcat 9 | Reporter: | stefan.ocke |
Component: | EL | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | P2 | ||
Version: | 9.0.26 | ||
Target Milestone: | ----- | ||
Hardware: | PC | ||
OS: | All | ||
Attachments: | Draft patch |
Description
stefan.ocke
2019-09-29 12:15:02 UTC
Created attachment 36807 [details]
Draft patch
The fix for this is rather more involved than it might first appear.
The check in Java is AccessibleObject.canAccess(Object) but that is a new method in Java 9 and the EL API (and implementation) have to work on Java 8. That means introducing a cut-down version of JreCompat into the EL API.
There are a further 9 places in EL API where Modifiers.isPublic() is called. Each of those also need to be checked to see if a similar change is required there. As do the 7 places it is called elsewhere in the Tomcat code base.
I've attached a draft patch that addresses the specific issue raised in this report for review / comment. I intend to use this as the basis for a broader, more complete patch.
Thank you for considering this. Sorry, I did not want to imply it was easy... I has a look at you patch and it looks reasonable to me. Maybe, in Jre9Compat there could be a check for isPublic at first. If not, return false immediately, if it is public, do the (reflective) canAccess call. Just for performance ... I think you highlight a valid point. The patch conflates the test for the type being public and the new Java 9 accessibility. I think those two should be kept separate. I have a fix locally for most of these issues. I'll re-work it to address the issue above before committing. Fixed in: - master for 9.0.27 onwards - 8.5.x for 8.5.47 onwards - 7.0.x for 7.0.97 onwards I tried with 9.0-SNAPSHOT now. Unfortunately, the problem ist still there and now I have twice as muc warnings as before: WARNING: Illegal reflective access by javax.el.Jre9Compat (file:/C:/java/maven/repository/org/apache/tomcat/embed/tomcat-embed-el/9.0-SNAPSHOT/tomcat-embed-el-9.0-SNAPSHOT.jar) to method ... WARNING: Illegal reflective access by javax.el.BeanELResolver (file:/C:/java/maven/repository/org/apache/tomcat/embed/tomcat-embed-el/9.0-SNAPSHOT/tomcat-embed-el-9.0-SNAPSHOT.jar) to method ... I am not yet sure what is going on here, but I will investigate further. There is probably something wrong with the warnings in the JDK (?) Since if I set --illegal-access=deny it works, where it would have failed before with 9.0.26. However, with --illegal-access=warn all the (fixed) warnings appear. Strange. Okay, the explanation is: With --illegal-access=warn , the new canAccess()-check will return true, since the access is still allowed. It will, however, log the warning (which might be a little bit verbose, since we want to CHECK the access, but not do it). Since canAccess returned true, findMethod() will still return the method from the un-exported module. This leads to the second warning. With --illegal-access=deny, the new canAccess()-check will return false and not log anything. Then findMethod() will search further for an exported method. I don't think there is anything more than can be done here. The warnings will persist, but they will never turn into errors. And that is the primary goal. (Maybe there should be some hint in the documentation about this to avoid further bug tickets) |