6788531: java.beans.Statement imposes excessive access control

Reviewed-by: peterz, rupashka

jdk/src/share/classes/com/sun/beans/finder/MethodFinder.java

@@ -24,10 +24,14 @@
  */
 package com.sun.beans.finder;
 
+import com.sun.beans.TypeResolver;
 import com.sun.beans.WeakCache;
 
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.util.Arrays;
 
 /**
  * This utility class provides {@code static} methods
@@ -122,30 +126,52 @@ public final class MethodFinder extends AbstractFinder<Method> {
         if (Modifier.isStatic(method.getModifiers())) {
             throw new NoSuchMethodException("Method '" + method.getName() + "' is not accessible");
         }
-        for (Class<?> face : type.getInterfaces()) {
+        for (Type generic : type.getGenericInterfaces()) {
             try {
-                return findAccessibleMethod(method, face);
+                return findAccessibleMethod(method, generic);
             }
             catch (NoSuchMethodException exception) {
                 // try to find in superclass or another interface
             }
         }
-        return findAccessibleMethod(method, type.getSuperclass());
+        return findAccessibleMethod(method, type.getGenericSuperclass());
     }
 
     /**
      * Finds method that accessible from specified class.
      *
      * @param method  object that represents found method
-     * @param type    class that is used to find accessible method
+     * @param generic generic type that is used to find accessible method
      * @return object that represents accessible method
      * @throws NoSuchMethodException if method is not accessible or is not found
      *                               in specified superclass or interface
      */
-    private static Method findAccessibleMethod(Method method, Class<?> type) throws NoSuchMethodException {
+    private static Method findAccessibleMethod(Method method, Type generic) throws NoSuchMethodException {
         String name = method.getName();
         Class<?>[] params = method.getParameterTypes();
-        return findAccessibleMethod(type.getMethod(name, params));
+        if (generic instanceof Class) {
+            Class<?> type = (Class<?>) generic;
+            return findAccessibleMethod(type.getMethod(name, params));
+        }
+        if (generic instanceof ParameterizedType) {
+            ParameterizedType pt = (ParameterizedType) generic;
+            Class<?> type = (Class<?>) pt.getRawType();
+            for (Method m : type.getMethods()) {
+                if (m.getName().equals(name)) {
+                    Class<?>[] pts = m.getParameterTypes();
+                    if (pts.length == params.length) {
+                        if (Arrays.equals(params, pts)) {
+                            return findAccessibleMethod(m);
+                        }
+                        Type[] gpts = m.getGenericParameterTypes();
+                        if (Arrays.equals(params, TypeResolver.erase(TypeResolver.resolve(pt, gpts)))) {
+                            return findAccessibleMethod(m);
+                        }
+                    }
+                }
+            }
+        }
+        throw new NoSuchMethodException("Method '" + name + "' is not accessible");
     }
 
 

jdk/src/share/classes/java/beans/EventHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2008 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 2000-2009 Sun Microsystems, Inc.  All Rights Reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -385,16 +385,16 @@ public class EventHandler implements InvocationHandler {
         try {
             Method getter = null;
             if (target != null) {
-                getter = ReflectionUtils.getMethod(target.getClass(),
+                getter = Statement.getMethod(target.getClass(),
                                       "get" + NameGenerator.capitalize(first),
                                       new Class[]{});
                 if (getter == null) {
-                    getter = ReflectionUtils.getMethod(target.getClass(),
+                    getter = Statement.getMethod(target.getClass(),
                                    "is" + NameGenerator.capitalize(first),
                                    new Class[]{});
                 }
                 if (getter == null) {
-                    getter = ReflectionUtils.getMethod(target.getClass(), first, new Class[]{});
+                    getter = Statement.getMethod(target.getClass(), first, new Class[]{});
                 }
             }
             if (getter == null) {
@@ -462,10 +462,10 @@ public class EventHandler implements InvocationHandler {
                     target = applyGetters(target, action.substring(0, lastDot));
                     action = action.substring(lastDot + 1);
                 }
-                Method targetMethod = ReflectionUtils.getMethod(
+                Method targetMethod = Statement.getMethod(
                              target.getClass(), action, argTypes);
                 if (targetMethod == null) {
-                    targetMethod = ReflectionUtils.getMethod(target.getClass(),
+                    targetMethod = Statement.getMethod(target.getClass(),
                              "set" + NameGenerator.capitalize(action), argTypes);
                 }
                 if (targetMethod == null) {

jdk/src/share/classes/java/beans/ReflectionUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2003-2008 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 2003-2009 Sun Microsystems, Inc.  All Rights Reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -24,21 +24,7 @@
  */
 package java.beans;
 
-import com.sun.beans.finder.PrimitiveWrapperMap;
-
-import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.lang.reflect.Modifier;
-
-import java.lang.ref.Reference;
-import java.lang.ref.SoftReference;
-
-import java.util.*;
-
-import sun.reflect.misc.MethodUtil;
-import sun.reflect.misc.ConstructorUtil;
-import sun.reflect.misc.ReflectUtil;
 
 /**
  * A utility class for reflectively finding methods, constuctors and fields
@@ -46,8 +32,6 @@ import sun.reflect.misc.ReflectUtil;
  */
 class ReflectionUtils {
 
-    private static Reference methodCacheRef;
-
     public static boolean isPrimitive(Class type) {
         return primitiveTypeFor(type) != null;
     }
@@ -65,346 +49,6 @@ class ReflectionUtils {
         return null;
     }
 
-    /**
-     * Tests each element on the class arrays for assignability.
-     *
-     * @param argClasses arguments to be tested
-     * @param argTypes arguments from Method
-     * @return true if each class in argTypes is assignable from the
-     *         corresponding class in argClasses.
-     */
-    private static boolean matchArguments(Class[] argClasses, Class[] argTypes) {
-        return matchArguments(argClasses, argTypes, false);
-    }
-
-    /**
-     * Tests each element on the class arrays for equality.
-     *
-     * @param argClasses arguments to be tested
-     * @param argTypes arguments from Method
-     * @return true if each class in argTypes is equal to the
-     *         corresponding class in argClasses.
-     */
-    private static boolean matchExplicitArguments(Class[] argClasses, Class[] argTypes) {
-        return matchArguments(argClasses, argTypes, true);
-    }
-
-    private static boolean matchArguments(Class[] argClasses,
-                                          Class[] argTypes, boolean explicit) {
-
-        boolean match = (argClasses.length == argTypes.length);
-        for(int j = 0; j < argClasses.length && match; j++) {
-            Class argType = argTypes[j];
-            if (argType.isPrimitive()) {
-                argType = PrimitiveWrapperMap.getType(argType.getName());
-            }
-            if (explicit) {
-                // Test each element for equality
-                if (argClasses[j] != argType) {
-                    match = false;
-                }
-            } else {
-                // Consider null an instance of all classes.
-                if (argClasses[j] != null &&
-                    !(argType.isAssignableFrom(argClasses[j]))) {
-                    match = false;
-                }
-            }
-        }
-        return match;
-    }
-
-    /**
-     * @return the method which best matches the signature or throw an exception
-     *         if it can't be found or the method is ambiguous.
-     */
-    static Method getPublicMethod(Class declaringClass, String methodName,
-                                           Class[] argClasses) throws NoSuchMethodException {
-        Method m;
-
-        m = findPublicMethod(declaringClass, methodName, argClasses);
-        if (m == null)
-            throw new NoSuchMethodException(declaringClass.getName() + "." + methodName);
-        return m;
-    }
-
-    /**
-     * @return the method which best matches the signature or null if it cant be found or
-     *         the method is ambiguous.
-     */
-    public static Method findPublicMethod(Class declaringClass, String methodName,
-                                           Class[] argClasses) {
-        // Many methods are "getters" which take no arguments.
-        // This permits the following optimisation which
-        // avoids the expensive call to getMethods().
-        if (argClasses.length == 0) {
-            try {
-                return MethodUtil.getMethod(declaringClass, methodName, argClasses);
-            }
-            catch (NoSuchMethodException e) {
-                  return null;
-            } catch (SecurityException se) {
-                // fall through
-            }
-        }
-        Method[] methods = MethodUtil.getPublicMethods(declaringClass);
-        List list = new ArrayList();
-        for(int i = 0; i < methods.length; i++) {
-            // Collect all the methods which match the signature.
-            Method method = methods[i];
-            if (method.getName().equals(methodName)) {
-                if (matchArguments(argClasses, method.getParameterTypes())) {
-                    list.add(method);
-                }
-            }
-        }
-        if (list.size() > 0) {
-            if (list.size() == 1) {
-                return (Method)list.get(0);
-            }
-            else {
-                ListIterator iterator = list.listIterator();
-                Method method;
-                while (iterator.hasNext()) {
-                    method = (Method)iterator.next();
-                    if (matchExplicitArguments(argClasses, method.getParameterTypes())) {
-                        return method;
-                    }
-                }
-                // There are more than one method which matches this signature.
-                // try to return the most specific method.
-                return getMostSpecificMethod(list, argClasses);
-            }
-        }
-        return null;
-    }
-
-    /**
-     * Return the most specific method from the list of methods which
-     * matches the args. The most specific method will have the most
-     * number of equal parameters or will be closest in the inheritance
-     * heirarchy to the runtime execution arguments.
-     * <p>
-     * See the JLS section 15.12
-     * http://java.sun.com/docs/books/jls/second_edition/html/expressions.doc.html#20448
-     *
-     * @param methods List of methods which already have the same param length
-     *                and arg types are assignable to param types
-     * @param args an array of param types to match
-     * @return method or null if a specific method cannot be determined
-     */
-    private static Method getMostSpecificMethod(List methods, Class[] args) {
-        Method method = null;
-
-        int matches = 0;
-        int lastMatch = matches;
-
-        ListIterator iterator = methods.listIterator();
-        while (iterator.hasNext()) {
-            Method m = (Method)iterator.next();
-            Class[] mArgs = m.getParameterTypes();
-            matches = 0;
-            for (int i = 0; i < args.length; i++) {
-                Class mArg = mArgs[i];
-                if (mArg.isPrimitive()) {
-                    mArg = PrimitiveWrapperMap.getType(mArg.getName());
-                }
-                if (args[i] == mArg) {
-                    matches++;
-                }
-            }
-            if (matches == 0 && lastMatch == 0) {
-                if (method == null) {
-                    method = m;
-                } else {
-                    // Test existing method. We already know that the args can
-                    // be assigned to all the method params. However, if the
-                    // current method parameters is higher in the inheritance
-                    // hierarchy then replace it.
-                    if (!matchArguments(method.getParameterTypes(),
-                                        m.getParameterTypes())) {
-                        method = m;
-                    }
-                }
-            } else if (matches > lastMatch) {
-                lastMatch = matches;
-                method = m;
-            } else if (matches == lastMatch) {
-                // ambiguous method selection.
-                method = null;
-            }
-        }
-        return method;
-    }
-
-    /**
-     * @return the method or null if it can't be found or is ambiguous.
-     */
-    public static Method findMethod(Class targetClass, String methodName,
-                                    Class[] argClasses) {
-        Method m = findPublicMethod(targetClass, methodName, argClasses);
-        if (m != null && Modifier.isPublic(m.getDeclaringClass().getModifiers())) {
-            return m;
-        }
-
-        /*
-        Search the interfaces for a public version of this method.
-
-        Example: the getKeymap() method of a JTextField
-        returns a package private implementation of the
-        of the public Keymap interface. In the Keymap
-        interface there are a number of "properties" one
-        being the "resolveParent" property implied by the
-        getResolveParent() method. This getResolveParent()
-        cannot be called reflectively because the class
-        itself is not public. Instead we search the class's
-        interfaces and find the getResolveParent()
-        method of the Keymap interface - on which invoke
-        may be applied without error.
-
-        So in :-
-
-            JTextField o = new JTextField("Hello, world");
-            Keymap km = o.getKeymap();
-            Method m1 = km.getClass().getMethod("getResolveParent", new Class[0]);
-            Method m2 = Keymap.class.getMethod("getResolveParent", new Class[0]);
-
-        Methods m1 and m2 are different. The invocation of method
-        m1 unconditionally throws an IllegalAccessException where
-        the invocation of m2 will invoke the implementation of the
-        method. Note that (ignoring the overloading of arguments)
-        there is only one implementation of the named method which
-        may be applied to this target.
-        */
-        for(Class type = targetClass; type != null; type = type.getSuperclass()) {
-            Class[] interfaces = type.getInterfaces();
-            for(int i = 0; i < interfaces.length; i++) {
-                m = findPublicMethod(interfaces[i], methodName, argClasses);
-                if (m != null) {
-                    return m;
-                }
-            }
-        }
-        return null;
-    }
-
-    /**
-     * A class that represents the unique elements of a method that will be a
-     * key in the method cache.
-     */
-    private static class Signature {
-        private Class targetClass;
-        private String methodName;
-        private Class[] argClasses;
-
-        private volatile int hashCode = 0;
-
-        public Signature(Class targetClass, String methodName, Class[] argClasses) {
-            this.targetClass = targetClass;
-            this.methodName = methodName;
-            this.argClasses = argClasses;
-        }
-
-        public boolean equals(Object o2) {
-            if (this == o2) {
-                return true;
-            }
-            Signature that = (Signature)o2;
-            if (!(targetClass == that.targetClass)) {
-                return false;
-            }
-            if (!(methodName.equals(that.methodName))) {
-                return false;
-            }
-            if (argClasses.length != that.argClasses.length) {
-                return false;
-            }
-            for (int i = 0; i < argClasses.length; i++) {
-                if (!(argClasses[i] == that.argClasses[i])) {
-                  return false;
-                }
-            }
-            return true;
-        }
-
-        /**
-         * Hash code computed using algorithm suggested in
-         * Effective Java, Item 8.
-         */
-        public int hashCode() {
-            if (hashCode == 0) {
-                int result = 17;
-                result = 37 * result + targetClass.hashCode();
-                result = 37 * result + methodName.hashCode();
-                if (argClasses != null) {
-                    for (int i = 0; i < argClasses.length; i++) {
-                        result = 37 * result + ((argClasses[i] == null) ? 0 :
-                            argClasses[i].hashCode());
-                    }
-                }
-                hashCode = result;
-            }
-            return hashCode;
-        }
-    }
-
-    /**
-     * A wrapper to findMethod(), which will search or populate the method
-     * in a cache.
-     * @throws exception if the method is ambiguios.
-     */
-    public static synchronized Method getMethod(Class targetClass,
-                                                String methodName,
-                                                Class[] argClasses) {
-        Object signature = new Signature(targetClass, methodName, argClasses);
-
-        Method method = null;
-        Map methodCache = null;
-        boolean cache = false;
-        if (ReflectUtil.isPackageAccessible(targetClass)) {
-            cache = true;
-        }
-
-        if (cache && methodCacheRef != null &&
-            (methodCache = (Map)methodCacheRef.get()) != null) {
-            method = (Method)methodCache.get(signature);
-            if (method != null) {
-                return method;
-            }
-        }
-        method = findMethod(targetClass, methodName, argClasses);
-        if (cache && method != null) {
-            if (methodCache == null) {
-                methodCache = new HashMap();
-                methodCacheRef = new SoftReference(methodCache);
-            }
-            methodCache.put(signature, method);
-        }
-        return method;
-    }
-
-    /**
-     * Return a constructor on the class with the arguments.
-     *
-     * @throws exception if the method is ambiguios.
-     */
-    public static Constructor getConstructor(Class cls, Class[] args) {
-        Constructor constructor = null;
-
-        // PENDING: Implement the resolutuion of ambiguities properly.
-        Constructor[] ctors = ConstructorUtil.getConstructors(cls);
-        for(int i = 0; i < ctors.length; i++) {
-            if (matchArguments(args, ctors[i].getParameterTypes())) {
-                constructor = ctors[i];
-            }
-        }
-        return constructor;
-    }
-
-    public static Object getPrivateField(Object instance, Class cls, String name) {
-        return getPrivateField(instance, cls, name, null);
-    }
-
     /**
      * Returns the value of a private field.
      *

jdk/src/share/classes/java/beans/Statement.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2007 Sun Microsystems, Inc.  All Rights Reserved.
+ * Copyright 2000-2009 Sun Microsystems, Inc.  All Rights Reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -31,6 +31,8 @@ import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 
 import com.sun.beans.finder.ClassFinder;
+import com.sun.beans.finder.ConstructorFinder;
+import com.sun.beans.finder.MethodFinder;
 import sun.reflect.misc.MethodUtil;
 
 /**
@@ -195,13 +197,18 @@ public class Statement {
                     argClasses[0] == String.class) {
                     return new Character(((String)arguments[0]).charAt(0));
                 }
-                m = ReflectionUtils.getConstructor((Class)target, argClasses);
+                try {
+                    m = ConstructorFinder.findConstructor((Class)target, argClasses);
+                }
+                catch (NoSuchMethodException exception) {
+                    m = null;
+                }
             }
             if (m == null && target != Class.class) {
-                m = ReflectionUtils.getMethod((Class)target, methodName, argClasses);
+                m = getMethod((Class)target, methodName, argClasses);
             }
             if (m == null) {
-                m = ReflectionUtils.getMethod(Class.class, methodName, argClasses);
+                m = getMethod(Class.class, methodName, argClasses);
             }
         }
         else {
@@ -224,7 +231,7 @@ public class Statement {
                     return null;
                 }
             }
-            m = ReflectionUtils.getMethod(target.getClass(), methodName, argClasses);
+            m = getMethod(target.getClass(), methodName, argClasses);
         }
         if (m != null) {
             try {
@@ -289,4 +296,13 @@ public class Statement {
         result.append(");");
         return result.toString();
     }
+
+    static Method getMethod(Class<?> type, String name, Class<?>... args) {
+        try {
+            return MethodFinder.findMethod(type, name, args);
+        }
+        catch (NoSuchMethodException exception) {
+            return null;
+        }
+    }
 }

jdk/test/java/beans/EventHandler/Test6788531.java

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2009 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6788531
+ * @summary Tests public method lookup problem in EventHandler
+ * @author Sergey Malenkov
+ */
+
+import javax.swing.JButton;
+import java.awt.event.ActionListener;
+import java.beans.EventHandler;
+
+public class Test6788531 {
+    public static void main(String[] args) throws Exception {
+        JButton button = new JButton("hi");
+        button.addActionListener(EventHandler.create(ActionListener.class, new Private(), "run"));
+        button.addActionListener(EventHandler.create(ActionListener.class, new PrivateGeneric(), "run", "generic"));
+        button.doClick();
+    }
+
+    public static class Public {
+        public void run() {
+            throw new Error("method is overridden");
+        }
+    }
+
+    static class Private extends Public {
+        public void run() {
+            System.out.println("default");
+        }
+    }
+
+    public static class PublicGeneric<T> {
+        public void run(T object) {
+            throw new Error("method is overridden");
+        }
+    }
+
+    static class PrivateGeneric extends PublicGeneric<String> {
+        public void run(String string) {
+            System.out.println(string);
+        }
+    }
+}

jdk/test/java/beans/Statement/Test6788531.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2009 Sun Microsystems, Inc.  All Rights Reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Sun Microsystems, Inc., 4150 Network Circle, Santa Clara,
+ * CA 95054 USA or visit www.sun.com if you need additional information or
+ * have any questions.
+ */
+
+/*
+ * @test
+ * @bug 6788531
+ * @summary Tests public method lookup problem in Statement
+ * @author Sergey Malenkov
+ */
+
+import java.beans.Statement;
+
+public class Test6788531 {
+    public static void main(String[] args) throws Exception {
+        new Statement(new Private(), "run", null).execute();
+        new Statement(new PrivateGeneric(), "run", new Object[] {"generic"}).execute();
+    }
+
+    public static class Public {
+        public void run() {
+            throw new Error("method is overridden");
+        }
+    }
+
+    static class Private extends Public {
+        public void run() {
+            System.out.println("default");
+        }
+    }
+
+    public static class PublicGeneric<T> {
+        public void run(T object) {
+            throw new Error("method is overridden");
+        }
+    }
+
+    static class PrivateGeneric extends PublicGeneric<String> {
+        public void run(String string) {
+            System.out.println(string);
+        }
+    }
+}