[
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17504829#comment-17504829
]
Robert Scholte edited comment on VELOCITY-952 at 3/11/22, 9:46 AM:
-------------------------------------------------------------------
{code}
public class ModuleVisibilityTest extends BaseTestCase {
public ModuleVisibilityTest(String name) {
super(name);
}
@Override
protected void setUpContext(VelocityContext context) {
context.put("timeZone", TimeZone.getTimeZone("UTC"));
}
public void testModuleVisibility() {
assertEvalEquals("0","$timeZone.getOffset(0)");
}
}
{code}
A small testcase to reproduce it. And I think the assumption is right.
In that case, what should be improved is respecting the method return type
instead of the class of the returned instance.
was (Author: rfscholte):
{code}
public class ModuleVisibilityTest extends BaseTestCase {
public ModuleVisibilityTest(String name) {
super(name);
}
@Override
protected void setUpContext(VelocityContext context) {
context.put("timeZone", TimeZone.getTimeZone("UTC"));
}
public void testModuleVisibilityTest() {
assertEvalEquals("0","$timeZone.getOffset(0)");
}
}
{code}
A small testcase to reproduce it. And I think the assumption is right.
In that case, what should be improved is respecting the method return type
instead of the class of the returned instance.
> Velocity is calling the "wrong" method
> --------------------------------------
>
> Key: VELOCITY-952
> URL: https://issues.apache.org/jira/browse/VELOCITY-952
> Project: Velocity
> Issue Type: Bug
> Components: Engine
> Affects Versions: 2.3
> Reporter: Thomas Mortagne
> Priority: Major
>
> OK, the title is maybe a bit harsh, but it catches the eyes :)
> At XWiki we recently started testing on Java 17 to see if there is any
> problem which leaded us to add things like {{--add-opens
> java.base/java.util=ALL-UNNAMED}} but Velocity happen to be the source of
> another problem related to the new module world.
> When doing something like {{$datetool.timeZone.getOffset(0)}} ($datetool
> being the org.apache.velocity.tools.generic.DateTool) we get the following
> error:
> {noformat}
> ...
> Caused by: java.lang.IllegalAccessException: class
> org.apache.velocity.util.introspection.UberspectImpl$VelMethodImpl cannot
> access class sun.util.calendar.ZoneInfo (in module java.base) because module
> java.base does not export sun.util.calendar to unnamed module @7ca16adc
> at
> java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:392)
> at
> java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:674)
> at java.base/java.lang.reflect.Method.invoke(Method.java:560)
> at
> org.apache.velocity.util.introspection.UberspectImpl$VelMethodImpl.doInvoke(UberspectImpl.java:571)
> at
> org.apache.velocity.util.introspection.UberspectImpl$VelMethodImpl.invoke(UberspectImpl.java:554)
> at
> org.apache.velocity.runtime.parser.node.ASTMethod.execute(ASTMethod.java:221)
> ... 199 more
> {noformat}
> The reason is that while the developer intent/expectation was to call
> "java.util.TimeZone#getOffset(0)" what Velocity really called from JVM point
> of view is "sun.util.calendar.ZoneInfo.getOffset(0)" directly.
> That's because Velocity is doing (I assume, since I did not check the exact
> code) the equivalent of:
> {{noformat}}
> java.util.TimeZone.getDefault().getClass().getMethod("getOffset",
> long.class).invoke(TimeZone.getDefault(), 0);
> {{noformat}}
> which is itself the equivalent of:
> {{noformat}}
> sun.util.calendar.ZoneInfo.class.getMethod("getOffset",
> long.class).invoke(TimeZone.getDefault(), 0);
> {{noformat}}
> I guess the only way to fix this would be to track down the lowest overridden
> Method (I agree, it might not always be easy to choose between two interfaces
> declaring a method with the same signature, but choosing the first one we
> find from the same hierarchy level is still better than nothing) and call
> that one instead. With the use case used as example in this issue, that would
> mean ending up doing the equivalent of:
> {{noformat}}
> java.util.TimeZone.class.getMethod("getOffset",
> long.class).invoke(TimeZone.getDefault(), 0);
> {{noformat}}
> which, from JVM point of view, is covered by the {{--add-opens
> java.base/java.util=ALL-UNNAMED}}.
> It would be a big change, but at least can't think of any retro-compatibility
> problem it might cause.
> One might be tempted to answer "just add {{--add-opens
> java.base/sun.util.calendar=ALL-UNNAMED}}" but it does not seem fair as this
> is not what the API that the script uses was exposing at all, you might need
> to document a different one for each JVM implementation (even if it's
> probably unlikely for this specific example) but more importantly you will
> potentially need quite a lot of those and will only discover it at runtime
> since it's not so easy to guess from an API in which you only know the public
> JVM classes since that's what you manipulate.
> I would be happy to work on this, but I wanted first ask what others think
> about this problem in general and the possible solutions I may have missed.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
