[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method

2023-10-05 Thread Christopher Schultz (Jira)


[ 
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17772341#comment-17772341
 ] 

Christopher Schultz commented on VELOCITY-952:
--

I reported this as VELOCITY-968 and offered the following as a suggestion. I 
don't know how the introspector works, so I'm just waving my hands, here:

"

Maybe 
org.apache.velocity.util.introspection.MethodMap.getBestMatch(List, 
Class[]) can choose a superclass method over a subclass method if they are 
otherwise equivalent?

"

I've read the documentation for MethodOverrideUberspector.java and I think if 
you just always use the most-superclass-or-superinterface method available, 
many of these issues can be avoided without any additional overhead of 
remembering the return-type of certain "get" invocations. This will also help 
when Velocity wasn't responsible for performing the original access, for 
example when the object is just dropped into the context by Java code and has a 
runtime type different from the declared type in the code.

> 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 

[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method

2023-04-12 Thread Thomas Mortagne (Jira)


[ 
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17711273#comment-17711273
 ] 

Thomas Mortagne commented on VELOCITY-952:
--

bq. an uberspector on XWiki side

Implemented on 
https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki-commons-velocity/src/main/java/org/xwiki/velocity/introspection/MethodOverrideUberspector.java

> 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.10#820010)

-
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org



[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method

2023-04-11 Thread Thomas Mortagne (Jira)


[ 
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17710993#comment-17710993
 ] 

Thomas Mortagne commented on VELOCITY-952:
--

{quote}
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.
{quote}

So I finally did some digging on what it would take to implement this idea, and 
it's indeed not trivial. From what I understood, the problem is that here is no 
way to associate metadata to a value in the context right now, so all the APIs 
which manipulate the value of a variable don't have any way to pass/return more 
info about this value (thinking especially about associating a Type with the 
value in the Context and how to return a Type along with the value returned by 
ASTMethod#execute.

I started by refactoring a bit {{org.apache.velocity.context.Context}} to 
support associating metadata to an entry without breaking existing API, but 
then the number of things to change in pretty critical places to take this into 
account is a bit overwhelming for me and a huge challenge in terms of retro 
compatibly without really knowing if that's really the direction that you guys 
want to take. 

Problem is that this bug makes pretty much impossible to say that XWiki 
supports Java 17 because of the many unforeseen IllegalAccessException failures 
you can end up with depending on which apparently standard Java API you are 
manipulating in a script. So for now my plan is to workaround this with an 
uberspector on XWiki side doing what I initially proposed:  try to find the 
lowest overwritten Method (which, I agree, is not the best and not something I 
would recommend on standard Velocity side compared to the "carry the return 
type around with the value" idea).

> 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 

[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method

2022-03-11 Thread Thomas Mortagne (Jira)


[ 
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] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method

2022-03-11 Thread Robert Scholte (Jira)


[ 
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: 

[jira] [Commented] (VELOCITY-952) Velocity is calling the "wrong" method

2022-03-10 Thread Michael Osipov (Jira)


[ 
https://issues.apache.org/jira/browse/VELOCITY-952?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17504510#comment-17504510
 ] 

Michael Osipov commented on VELOCITY-952:
-

Nice catch, I am clueless. Let's ask an JPMS expert: [~rfscholte], can you help?

> 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