Merge pull request #110 from JCgH4164838Gh792C124B5/localOGNL_getRead…

…WriteMethodUpdate

Update isMethodCallable() logic, re-introduce its usage for getReadMethod()

src/main/java/ognl/OgnlRuntime.java

@@ -2735,18 +2735,48 @@ private static void addIfAccessor(List result, Method method, String baseName, b
     }
 
     /**
-     * Convenience used to check if a method is volatile or synthetic so as to avoid
-     * calling un-callable methods.
+     * Convenience used to check if a method is a synthetic method so as to avoid
+     * calling un-callable methods.  These methods are not considered callable by
+     * OGNL in almost all circumstances.
+     * 
+     * This method considers any synthetic method (even bridge methods) as being un-callable.
+     * Even though synthetic and bridge methods can technically be called, by default
+     * OGNL excludes them from consideration.
+     * 
+     * Synthetic methods should be excluded in general, since calling such methods
+     * could introduce unanticipated risks.
      *
      * @param m The method to check.
-     * @return True if the method should be callable, false otherwise.
+     * @return True if the method should be callable (non-synthetic), false otherwise.
      */
     static boolean isMethodCallable(Method m)
     {
-        if ((isJdk15() && m.isSynthetic()) || Modifier.isVolatile(m.getModifiers()))
-            return false;
+        return !(m.isSynthetic() || m.isBridge());
+    }
 
-        return true;
+    /**
+     * Convenience used to check if a method is either a non-synthetic method or
+     * a bridge method.
+     * 
+     * Warning:  This method should <b>NOT<b> be used as a direct replacement for 
+     * {@link #isMethodCallable(Method)}.  Almost all OGNL processing assumes the
+     * exlcusion of synthetic methods in order to process correctly.  <b>Only</b>
+     * use this method to determine method callability for any OGNL processing
+     * after <b>careful</b> consideration.
+     * 
+     * This method considers synthetic methods that are not also bridge methods
+     * as being un-callable.
+     * 
+     * Synthetic methods should be excluded in general, since calling such methods
+     * could introduce unanticipated risks.
+     * 
+     * @since 3.2.16
+     *
+     * @param m The method to check.
+     * @return True if the method should be callable (non-synethetic or bridge), false otherwise.
+     */
+    static boolean isMethodCallable_BridgeOrNonSynthetic(Method m) {
+        return !m.isSynthetic() || m.isBridge();  // Reference: See PR#104.
     }
 
     /**
@@ -3596,6 +3626,11 @@ public static Method getReadMethod(Class target, String name, Class[] argClasses
 
             for (int i = 0; i < methods.length; i++)
             {
+                // Consider bridge methods as callable (also) for Read methods.
+                if (!isMethodCallable_BridgeOrNonSynthetic(methods[i])) {
+                    continue;
+                }
+
                 if ((methods[i].getName().equalsIgnoreCase(name)
                      || methods[i].getName().toLowerCase().equals("get" + name)
                      || methods[i].getName().toLowerCase().equals("has" + name)
@@ -3613,6 +3648,11 @@ public static Method getReadMethod(Class target, String name, Class[] argClasses
 
             for (int i = 0; i < methods.length; i++)
             {
+                // Consider bridge methods as callable (also) for Read methods.
+                if (!isMethodCallable_BridgeOrNonSynthetic(methods[i])) {
+                    continue;
+                }
+
                 if (methods[i].getName().equalsIgnoreCase(name)
                     && !methods[i].getName().startsWith("set")
                     && !methods[i].getName().startsWith("get")
@@ -3675,8 +3715,10 @@ public static Method getWriteMethod(Class target, String name, Class[] argClasse
             ArrayList<Method> candidates = new ArrayList<Method>();
 
             for (int i = 0; i < methods.length; i++) {
-                if (!isMethodCallable(methods[i].getMethod()))
+                // Consider bridge methods as callable (also) for Write methods.
+                if (!isMethodCallable_BridgeOrNonSynthetic(methods[i].getMethod())) {
                     continue;
+                }
 
                 if ((methods[i].getName().equalsIgnoreCase(name)
                      || methods[i].getName().toLowerCase().equals(name.toLowerCase())
@@ -3696,8 +3738,10 @@ public static Method getWriteMethod(Class target, String name, Class[] argClasse
 
             Method[] cmethods = target.getClass().getMethods();
             for (int i = 0; i < cmethods.length; i++) {
-                if (!isMethodCallable(cmethods[i]))
+                // Consider bridge methods as callable (also) for Write methods.
+                if (!isMethodCallable_BridgeOrNonSynthetic(cmethods[i])) {
                     continue;
+                }
 
                 if ((cmethods[i].getName().equalsIgnoreCase(name)
                      || cmethods[i].getName().toLowerCase().equals(name.toLowerCase())

src/test/java/ognl/TestOgnlRuntime.java

@@ -699,6 +699,9 @@ public String testMethod (String element, int i) {
         }
     }
 
+    /**
+     * Protected class for synthetic/bridge method tests.
+     */
     protected static class ProtectedParent {
         public void setName(String name) {
         }
@@ -707,15 +710,114 @@ public String getName() {
         }
     }
 
+    /**
+     * Public descendant class for synthetic/bridge method tests.
+     */
     public static class PublicChild extends ProtectedParent {
     }
 
-    public void testSyntheticReadMethod() throws Exception {
+    /**
+     * Test that synthetic bridge read methods can be found successfully.
+     * 
+     * Note: Only bridge methods should qualify, non-bridge synthetic methods should not.
+     * 
+     * @throws Exception 
+     */
+    public void testSyntheticBridgeReadMethod() throws Exception {
         assertNotNull(OgnlRuntime.getReadMethod(PublicChild.class, "name"));
     }
 
-    public void testSyntheticWriteMethod() throws Exception {
+    /**
+     * Test that synthetic bridge write methods can be found successfully.
+     * 
+     * Note: Only bridge methods should qualify, non-bridge synthetic methods should not.
+     * 
+     * @throws Exception 
+     */
+    public void testSyntheticBridgeWriteMethod() throws Exception {
         assertNotNull(OgnlRuntime.getWriteMethod(PublicChild.class, "name"));
     }
 
+    /**
+     * Public class for "is callable" method tests.
+     */
+    public static class SimplePublicClass {
+        String name = "name contents";
+
+        public String getName() {
+            return name;
+        }
+    }
+
+    /**
+     * Public class with non-public nested class for "is callable" method tests.
+     */
+    public static class SimpleNestingClass {
+        class NestedClass {
+            private String name = "nested name contents";
+        }
+
+        public String getNestedName() {
+            return new NestedClass().name;  // Should force creation of a synthetic method for NestedClass (to access its name field).
+        }
+    }
+
+    /**
+     * Test that normal non-synthetic methods are considered callable by both isMethodCallable() and isMethodCallable_BridgeOrNonSynthetic().
+     */
+    public void testConfirmStandardMethodCallability() {
+        Method method = null;
+        try {
+            method = SimplePublicClass.class.getDeclaredMethod("getName", (Class<?>[]) null);
+        } catch (NoSuchMethodException nsme) {
+            fail("SimplePublicClass.getName() method retrieval by reflection failed (NoSuchMethodException) ?");
+        }
+        assertNotNull("getName() method retrieval failed ?", method);
+        assertTrue("SimplePublicClass.getName() is a synthetic or bridge method ?", !(method.isBridge() || method.isSynthetic()));
+        assertTrue("SimplePublicClass.getName() is not considered callable by isMethodCallable() ?", OgnlRuntime.isMethodCallable(method));
+        assertTrue("SimplePublicClass.getName() is not considered callable by isMethodCallable_BridgeOrNonSynthetic() ?", OgnlRuntime.isMethodCallable_BridgeOrNonSynthetic(method));
+    }
+
+    /**
+     * Test that bridge methods ARE considered callable by isMethodCallable_BridgeOrNonSynthetic() ONLY, and NOT by isMethodCallable().
+     */
+    public void testConfirmBridgeMethodCallability() {
+        Method method = null;
+        try {
+            method = PublicChild.class.getDeclaredMethod("getName", (Class<?>[]) null);
+        } catch (NoSuchMethodException nsme) {
+            fail("PublicChild.getName() method retrieval by reflection failed (NoSuchMethodException) ?");
+        }
+        assertNotNull("getName() method retrieval failed ?", method);
+        assertTrue("PublicChild.getName() is not a bridge method ?", method.isBridge());
+        assertFalse("PublicChild.getName() is considered callable by isMethodCallable() ?", OgnlRuntime.isMethodCallable(method));
+        assertTrue("PublicChild.getName() is not considered callable by isMethodCallable_BridgeOrNonSynthetic() ?", OgnlRuntime.isMethodCallable_BridgeOrNonSynthetic(method));
+
+        try {
+            Class<?>[] argumentTypes = {String.class};
+            method = PublicChild.class.getDeclaredMethod("setName", argumentTypes);
+        } catch (NoSuchMethodException nsme) {
+            fail("PublicChild.setName() method retrieval by reflection failed (NoSuchMethodException) ?");
+        }
+        assertNotNull("setName() method retrieval failed ?", method);
+        assertTrue("PublicChild.setName() is not a bridge method ?", method.isBridge());
+        assertFalse("PublicChild.setName() is considered callable by isMethodCallable() ?", OgnlRuntime.isMethodCallable(method));
+        assertTrue("PublicChild.setName() is not considered callable by isMethodCallable_BridgeOrNonSynthetic() ?", OgnlRuntime.isMethodCallable_BridgeOrNonSynthetic(method));
+    }
+
+    /**
+     * Test that non-bridge synthetic methods are NOT considered callable by either isMethodCallable() or isMethodCallable_BridgeOrNonSynthetic().
+     */
+    public void testConfirmSyntheticMethodNonCallablility() {
+        Method method;
+        Method[] methods = SimpleNestingClass.NestedClass.class.getDeclaredMethods();
+        assertNotNull("Nested class has no methods ?", methods);
+        assertTrue("Nested class has no methods ?", methods.length > 0);
+        method = methods[0];
+        assertNotNull("Nested class method at index 0 is null ?", method);
+        assertTrue("SimpleAbstractClass.getName() is a synthetic method ?", method.isSynthetic());
+        assertFalse("SimpleAbstractClass.getName() is a bridge method ?", method.isBridge());
+        assertFalse("SimpleAbstractClass.getName() is considered callable by isMethodCallable() ?", OgnlRuntime.isMethodCallable(method));
+        assertFalse("SimpleAbstractClass.getName() is considered callable by isMethodCallable_BridgeOrNonSynthetic() ?", OgnlRuntime.isMethodCallable_BridgeOrNonSynthetic(method));
+    }
 }