[jira] [Updated] (VELOCITY-952) Velocity is calling the "wrong" method
[
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Thomas Mortagne updated VELOCITY-952:
-
Description:
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.
was:
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
[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method
[
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17504830#comment-17504830
]
Thomas Mortagne commented on VELOCITY-952:
--
bq. respecting the method return type
Carrying this information around with the object sounds like a complex thing to
change (but maybe not, I don't know the Velocity AST very well), but I agree
it's probably the most accurate.
> 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: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
[jira] [Comment Edited] (VELOCITY-952) Velocity is calling the "wrong" method
[
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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
[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method
[
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17504829#comment-17504829
]
Robert Scholte commented on VELOCITY-952:
-
{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:
