Velocity
  1. Velocity
  2. VELOCITY-761

Can not reference a property declared in a super-interface and implemented in a non-public class

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.6.3
    • Fix Version/s: None
    • Component/s: Engine
    • Labels:
      None

      Description

      Consider the following:

      public interface MyUser extends java.security.Principal

      { String getEmailAddress(); }

      class MyUserImpl implements MyUser {
      public String getName()

      { ... }
      public String getEmailAddress() { ... }

      }

      If I put a MyUserImpl in my Velocity context, $user.emailAddress will resolve, but $user.name will not.

      This is a problem with ClassMap#createMethodCache(). It ignores methods declared on the MyUserImpl class because the class is non-public, and it only looks up one level in the Interface hierarchy for methods defined on interfaces: so it will go up as far as the MyUser interface but not as far as the Principal interface.

        Issue Links

          Activity

          Hide
          Charles Miller added a comment - - edited

          Workarounds:

          1. Declare MyUserImpl as a public class
          2. Have MyUserImpl directly implement both MyUser and Principal
          3. Explicitly re-declare the methods on the Principal interface on the MyUser sub-interface.

          Show
          Charles Miller added a comment - - edited Workarounds: 1. Declare MyUserImpl as a public class 2. Have MyUserImpl directly implement both MyUser and Principal 3. Explicitly re-declare the methods on the Principal interface on the MyUser sub-interface.
          Hide
          Jarkko Viinamäki added a comment -

          This doesn't look like a bug to me.

          http://wiki.apache.org/velocity/VelocityFAQ

          Show
          Jarkko Viinamäki added a comment - This doesn't look like a bug to me. http://wiki.apache.org/velocity/VelocityFAQ
          Hide
          Nathan Bubna added a comment -

          createMethodCache gets the interfaces of the class and calls populateMethodCacheWithInterface(...), which most certainly gets the super interfaces and recurses up the tree that way too. We even test this out in the Velocity689TestCase as part of our build process. See VELOCITY-689.

          Show
          Nathan Bubna added a comment - createMethodCache gets the interfaces of the class and calls populateMethodCacheWithInterface(...), which most certainly gets the super interfaces and recurses up the tree that way too. We even test this out in the Velocity689TestCase as part of our build process. See VELOCITY-689 .
          Show
          Nathan Bubna added a comment - http://svn.apache.org/repos/asf/velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity689TestCase.java
          Hide
          Charles Miller added a comment -

          Ah, oops. I originally encountered this bug in 1.6.1 and downloaded the source to 1.6.3 to see if it was still there, but I might have not looked closely enough.

          I'll check again in the morning and apologise profusely if I wasted your time.

          Show
          Charles Miller added a comment - Ah, oops. I originally encountered this bug in 1.6.1 and downloaded the source to 1.6.3 to see if it was still there, but I might have not looked closely enough. I'll check again in the morning and apologise profusely if I wasted your time.
          Hide
          Matt Ryall added a comment - - edited

          To avoid a whole class of bugs in this area, you could consider using a utility that has well-tested functionality for class introspection like MethodUtils in Commons BeanUtils:

          http://commons.apache.org/beanutils/apidocs/org/apache/commons/beanutils/MethodUtils.html

          Show
          Matt Ryall added a comment - - edited To avoid a whole class of bugs in this area, you could consider using a utility that has well-tested functionality for class introspection like MethodUtils in Commons BeanUtils: http://commons.apache.org/beanutils/apidocs/org/apache/commons/beanutils/MethodUtils.html
          Hide
          Nathan Bubna added a comment -

          That's not really feasible, we have different method mapping rules and caching goals.

          Show
          Nathan Bubna added a comment - That's not really feasible, we have different method mapping rules and caching goals.
          Hide
          William R. Zwicky added a comment -

          I found a related issue. I'm trying to make get("property") work, and it has the same restriction – If the class is not public, the method is not visible. Since an instance of this class has been placed directly into the Velocity context, it seems very odd when methods go missing like this. Velocity should either allow access to all public methods, or crash when the class is not public.

          You also need a section in the documentation on how to build a context object, collecting into one place all the ways properties can be accessed. This would be a good place to document these public/non-public rules.

          Below is the class I'm trying to use. It currently hides inside VelocityGetter.java, thus can't be public.

          class MapList<K,V> extends AbstractList<V> {
          private Map<K,Integer> map;
          private List<V> values;

          public MapList(List<K> schema)

          { map = new HashMap<K, Integer>(); for(int i=0; i<schema.size(); i++) map.put(schema.get(i), i); }

          public MapList(List<K> schema, List<V> values)

          { this(schema); this.values = values; }

          // === List interface === //

          @Override
          public V get(int index)

          { return values.get(index); }

          @Override
          public int size()

          { return values.size(); }

          @Override
          public V set(int index, V value)

          { return values.set(index,value); }

          // === Extra Magic === //

          public void setValues(List<V> values)

          { this.values = values; }

          // public V get(K key)

          { // return values.get(map.get(key)); // }

          public String get(String key)

          { return (String) values.get(map.get(key)); }

          }

          Show
          William R. Zwicky added a comment - I found a related issue. I'm trying to make get("property") work, and it has the same restriction – If the class is not public, the method is not visible. Since an instance of this class has been placed directly into the Velocity context, it seems very odd when methods go missing like this. Velocity should either allow access to all public methods, or crash when the class is not public. You also need a section in the documentation on how to build a context object, collecting into one place all the ways properties can be accessed. This would be a good place to document these public/non-public rules. Below is the class I'm trying to use. It currently hides inside VelocityGetter.java, thus can't be public. class MapList<K,V> extends AbstractList<V> { private Map<K,Integer> map; private List<V> values; public MapList(List<K> schema) { map = new HashMap<K, Integer>(); for(int i=0; i<schema.size(); i++) map.put(schema.get(i), i); } public MapList(List<K> schema, List<V> values) { this(schema); this.values = values; } // === List interface === // @Override public V get(int index) { return values.get(index); } @Override public int size() { return values.size(); } @Override public V set(int index, V value) { return values.set(index,value); } // === Extra Magic === // public void setValues(List<V> values) { this.values = values; } // public V get(K key) { // return values.get(map.get(key)); // } public String get(String key) { return (String) values.get(map.get(key)); } }
          Hide
          Christopher Schultz added a comment -

          William,

          Which "get" method are you trying to call? There are two:

          public String get(String key) // defined in package-protected class MapList
          public Object get(int index) // defined in public interface java.util.List

          I think you probably can't access your get(String) method, since it's not actually visible publicly. This seems perfectly reasonable to me, since the list of accessible methods should be:

          • all public methods from public interfaces
            +
          • all public methods from public classes

          You'll just have to modify your class structure or visibility in order to make this method callable from Velocity.

          Show
          Christopher Schultz added a comment - William, Which "get" method are you trying to call? There are two: public String get(String key) // defined in package-protected class MapList public Object get(int index) // defined in public interface java.util.List I think you probably can't access your get(String) method, since it's not actually visible publicly. This seems perfectly reasonable to me, since the list of accessible methods should be: all public methods from public interfaces + all public methods from public classes You'll just have to modify your class structure or visibility in order to make this method callable from Velocity.
          Hide
          William R. Zwicky added a comment -

          You're missing the point, the problem is one of consistency. The methods we (myself and the original poster) are trying to access are public, but Velocity is getting confused over the status of the class itself. Java doesn't have this problem; if a class is not accessible, then NO part of it can be read. If the class is accessible, then ALL public parts can be read.

          Velocity is different: If the class is not public, then it is PARTIALLY accessible.

          I believe this needs to be brought into sync with Java (forbid all access to non-public classes) or brought into sync with common sense (allow access to all public methods, regardless of the visibility of the class.)

          Show
          William R. Zwicky added a comment - You're missing the point, the problem is one of consistency. The methods we (myself and the original poster) are trying to access are public, but Velocity is getting confused over the status of the class itself. Java doesn't have this problem; if a class is not accessible, then NO part of it can be read. If the class is accessible, then ALL public parts can be read. Velocity is different: If the class is not public, then it is PARTIALLY accessible. I believe this needs to be brought into sync with Java (forbid all access to non-public classes) or brought into sync with common sense (allow access to all public methods, regardless of the visibility of the class.)
          Hide
          Nathan Bubna added a comment -

          Wait, i missed something. What parts of a non-public class does Velocity make accessible? It should not do that, as far as i can tell. The goal has always been only public methods declared in public classes.

          Show
          Nathan Bubna added a comment - Wait, i missed something. What parts of a non-public class does Velocity make accessible? It should not do that, as far as i can tell. The goal has always been only public methods declared in public classes.
          Hide
          William R. Zwicky added a comment -

          Velocity allows access to public methods in non-public classes if the method is also listed in one of the class's interfaces, and that interface is public.

          Show
          William R. Zwicky added a comment - Velocity allows access to public methods in non-public classes if the method is also listed in one of the class's interfaces, and that interface is public.
          Hide
          Nathan Bubna added a comment -

          I don't see that as inconsistent at all. The method was still declared in a public class, even if the implementation was provided elsewhere. A publicly declared public method is all we need to keep the reflection working without doing things that a java security manager might frown upon.

          Show
          Nathan Bubna added a comment - I don't see that as inconsistent at all. The method was still declared in a public class, even if the implementation was provided elsewhere. A publicly declared public method is all we need to keep the reflection working without doing things that a java security manager might frown upon.
          Hide
          Christopher Schultz added a comment -

          William, you didn't answer my question: which method are you trying to call? get(int) should be accessible while get(String) should not be.

          Show
          Christopher Schultz added a comment - William, you didn't answer my question: which method are you trying to call? get(int) should be accessible while get(String) should not be.
          Hide
          William R. Zwicky added a comment -

          Looks like Cristopher is correct, my comment contains no bug. I'm not sure why this confused me; maybe I'm used to other scripting languages with very loose access rules.

          Maybe Velocity could put a reminder in the error message ("Type MapList is not visible to Velocity").

          Show
          William R. Zwicky added a comment - Looks like Cristopher is correct, my comment contains no bug. I'm not sure why this confused me; maybe I'm used to other scripting languages with very loose access rules. Maybe Velocity could put a reminder in the error message ("Type MapList is not visible to Velocity").
          Hide
          Christopher Schultz added a comment -

          William, it's hard to tell why data wasn't available... it might not actually be possible to give an error message that is as specific as "type is not visible".

          Show
          Christopher Schultz added a comment - William, it's hard to tell why data wasn't available... it might not actually be possible to give an error message that is as specific as "type is not visible".

            People

            • Assignee:
              Unassigned
              Reporter:
              Charles Miller
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development