From 387db2bb771a21d05ddd07e61973b5aaf75e089c Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Mon, 10 Jun 2019 00:49:42 -0400 Subject: [PATCH 1/5] OGNL improve getter/setter detection algorithm: - Add detail to isDefaultMethod() method comment. - Add a new isNonDefaultPublicInterfaceMethod() method. - Modify collectAccessors() method to consider public interface methods in addition to default methods. - Modify _getGetMethod() to prefer the first valid public getter over other getters, then the first valid public non-Default interface getter and lastly the first getter of any kind. - Modify _getSetMethod() to prefer the first valid public setter over other setters, then the first valid public non-Default interface setter and lastly the first setter of any kind. --- src/java/ognl/OgnlRuntime.java | 108 +++++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 5 deletions(-) diff --git a/src/java/ognl/OgnlRuntime.java b/src/java/ognl/OgnlRuntime.java index 15bb3701..3f991804 100644 --- a/src/java/ognl/OgnlRuntime.java +++ b/src/java/ognl/OgnlRuntime.java @@ -1843,6 +1843,13 @@ private static void addMethodToResult(Map result, Method method) /** * Backport of java.lang.reflect.Method#isDefault() + * + * JDK8+ supports Default Methods for interfaces. Default Methods are defined as: + * public, non-abstract and declared within an interface (must also be non-static). + * + * @param method The Method to check against the requirements for a Default Method. + * + * @return true If the Method qualifies as a Default Method, false otherwise. */ private static boolean isDefaultMethod(Method method) { @@ -1851,6 +1858,24 @@ private static boolean isDefaultMethod(Method method) && method.getDeclaringClass().isInterface(); } + /** + * Determine if the provided Method is a non-Default public Interface method. + * + * Public non-Default Methods are defined as: + * public, abstract, non-static and declared within an interface. + * + * @param method The Method to check against the requirements for a non-Default Method. + * + * @return true If method qualifies as a non-Default public Interface method, false otherwise. + * + * @since 3.1.24 + */ + private static boolean isNonDefaultPublicInterfaceMethod(Method method) { + return ((method.getModifiers() + & (Modifier.ABSTRACT | Modifier.PUBLIC | Modifier.STATIC)) == (Modifier.ABSTRACT | Modifier.PUBLIC)) + && method.getDeclaringClass().isInterface(); + } + public static Map getAllMethods(Class targetClass, boolean staticMethods) { ClassCache cache = (staticMethods ? _staticMethodCache : _instanceMethodCache); @@ -2213,7 +2238,7 @@ private static void collectAccessors(Class c, String baseName, List result, bool final Method[] methods = c.getDeclaredMethods(); for (int i = 0; i < methods.length; i++) { if (c.isInterface()) { - if (isDefaultMethod(methods[i])) { + if (isDefaultMethod(methods[i]) || isNonDefaultPublicInterfaceMethod(methods[i])) { addIfAccessor(result, methods[i], baseName, findSets); } continue; @@ -2286,6 +2311,26 @@ public static Method getGetMethod(OgnlContext context, Class targetClass, String return method; } + /** + * Returns a qualifying get (getter) method, if one is available for the given targetClass and propertyName. + * + * Note: From OGNL 3.1.24 onward, this method will attempt to find the first get getter method(s) that match: + * 1) First get (getter) method, whether public or not. + * 2) First public get (getter) method, provided the method's declaring class is also public. + * This may be the same as 1), if 1) is also public and its declaring class is also public. + * 3) First public non-Default interface get (getter) method, provided the method's declaring class is also public. + * The order of preference (priority) for the above matches will be 2 (1st public getter), + * 3 (1st public non-Default interface getter), 1 (1st getter of any kind). + * This updated methodology should help limit the need to modify method accessibility levels in some circumstances. + * + * @param context The current execution context. + * @param targetClass Class to search for a get method (getter). + * @param propertyName Name of the property for the get method (getter). + * + * @return + * @throws IntrospectionException + * @throws OgnlException + */ private static Method _getGetMethod(OgnlContext context, Class targetClass, String propertyName) throws IntrospectionException, OgnlException { @@ -2295,6 +2340,9 @@ private static Method _getGetMethod(OgnlContext context, Class targetClass, Stri if (methods != null) { + Method firstGetter = null; + Method firstPublicGetter = null; + Method firstNonDefaultPublicInterfaceGetter = null; for (int i = 0, icount = methods.size(); i < icount; i++) { Method m = (Method) methods.get(i); @@ -2302,10 +2350,23 @@ private static Method _getGetMethod(OgnlContext context, Class targetClass, Stri if (mParameterTypes.length == 0) { - result = m; - break; + boolean declaringClassIsPublic = Modifier.isPublic(m.getDeclaringClass().getModifiers()); + if (firstGetter == null) { + firstGetter = m; + } + if (firstPublicGetter == null && Modifier.isPublic(m.getModifiers()) && declaringClassIsPublic) { + firstPublicGetter = m; + } + if (firstNonDefaultPublicInterfaceGetter == null && isNonDefaultPublicInterfaceMethod(m) && declaringClassIsPublic) { + firstNonDefaultPublicInterfaceGetter = m; + } + if (firstGetter != null && firstPublicGetter != null && firstNonDefaultPublicInterfaceGetter != null) { + break; + } } } + result = (firstPublicGetter != null) ? firstPublicGetter : + (firstNonDefaultPublicInterfaceGetter != null) ? firstNonDefaultPublicInterfaceGetter : firstGetter; } return result; @@ -2343,6 +2404,26 @@ public static Method getSetMethod(OgnlContext context, Class targetClass, String return method; } + /** + * Returns a qualifying set (setter) method, if one is available for the given targetClass and propertyName. + * + * Note: From OGNL 3.1.24 onward, this method will attempt to find the first set setter method(s) that match: + * 1) First set (setter) method, whether public or not. + * 2) First public set (setter) method, provided the method's declaring class is also public. + * This may be the same as 1), if 1) is also public and its declaring class is also public. + * 3) First public non-Default interface set (setter) method, provided the method's declaring class is also public. + * The order of preference (priority) for the above matches will be 2 (1st public setter), + * 3 (1st public non-Default interface setter), 1 (1st setter of any kind). + * This updated methodology should help limit the need to modify method accessibility levels in some circumstances. + * + * @param context The current execution context. + * @param targetClass Class to search for a set method (setter). + * @param propertyName Name of the property for the set method (setter). + * + * @return + * @throws IntrospectionException + * @throws OgnlException + */ private static Method _getSetMethod(OgnlContext context, Class targetClass, String propertyName) throws IntrospectionException, OgnlException { @@ -2352,16 +2433,33 @@ private static Method _getSetMethod(OgnlContext context, Class targetClass, Stri if (methods != null) { + Method firstSetter = null; + Method firstPublicSetter = null; + Method firstNonDefaultPublicInterfaceSetter = null; for (int i = 0, icount = methods.size(); i < icount; i++) { Method m = (Method) methods.get(i); Class[] mParameterTypes = findParameterTypes(targetClass, m); //getParameterTypes(m); if (mParameterTypes.length == 1) { - result = m; - break; + boolean declaringClassIsPublic = Modifier.isPublic(m.getDeclaringClass().getModifiers()); + if (firstSetter == null) { + firstSetter = m; + } + if (firstPublicSetter == null && Modifier.isPublic(m.getModifiers()) && declaringClassIsPublic) { + firstPublicSetter = m; + } + if (firstNonDefaultPublicInterfaceSetter == null && isNonDefaultPublicInterfaceMethod(m) && declaringClassIsPublic) { + firstNonDefaultPublicInterfaceSetter = m; + } + if (firstSetter != null && firstPublicSetter != null && firstNonDefaultPublicInterfaceSetter != null) { + break; + } } } + + result = (firstPublicSetter != null) ? firstPublicSetter : + (firstNonDefaultPublicInterfaceSetter != null) ? firstNonDefaultPublicInterfaceSetter : firstSetter; } return result; From d5bf01403fcc3f3f97ae6cf9bea79fce11862239 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Wed, 12 Jun 2019 22:28:10 -0400 Subject: [PATCH 2/5] Updated commit: - Updated @since and related comments to refer to 3.1.25 (post-merge of 3.1.24 changes). --- src/java/ognl/OgnlRuntime.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/java/ognl/OgnlRuntime.java b/src/java/ognl/OgnlRuntime.java index 42a39054..13c3ba1a 100644 --- a/src/java/ognl/OgnlRuntime.java +++ b/src/java/ognl/OgnlRuntime.java @@ -1922,7 +1922,7 @@ private static boolean isDefaultMethod(Method method) * * @return true If method qualifies as a non-Default public Interface method, false otherwise. * - * @since 3.1.24 + * @since 3.1.25 */ private static boolean isNonDefaultPublicInterfaceMethod(Method method) { return ((method.getModifiers() @@ -2368,7 +2368,7 @@ public static Method getGetMethod(OgnlContext context, Class targetClass, String /** * Returns a qualifying get (getter) method, if one is available for the given targetClass and propertyName. * - * Note: From OGNL 3.1.24 onward, this method will attempt to find the first get getter method(s) that match: + * Note: From OGNL 3.1.25 onward, this method will attempt to find the first get getter method(s) that match: * 1) First get (getter) method, whether public or not. * 2) First public get (getter) method, provided the method's declaring class is also public. * This may be the same as 1), if 1) is also public and its declaring class is also public. @@ -2461,7 +2461,7 @@ public static Method getSetMethod(OgnlContext context, Class targetClass, String /** * Returns a qualifying set (setter) method, if one is available for the given targetClass and propertyName. * - * Note: From OGNL 3.1.24 onward, this method will attempt to find the first set setter method(s) that match: + * Note: From OGNL 3.1.25 onward, this method will attempt to find the first set setter method(s) that match: * 1) First set (setter) method, whether public or not. * 2) First public set (setter) method, provided the method's declaring class is also public. * This may be the same as 1), if 1) is also public and its declaring class is also public. From a2c0c95d38e757b1e62db0a811b0968dbc5b37a0 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sat, 15 Jun 2019 09:09:41 -0400 Subject: [PATCH 3/5] Update: - Minor optimization in _getGetMethod() and _getSetMethod() to stop search once best possible match found. Previously was finding all 3 types of matches before stopping the search. --- src/java/ognl/OgnlRuntime.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/java/ognl/OgnlRuntime.java b/src/java/ognl/OgnlRuntime.java index 13c3ba1a..eaccda5a 100644 --- a/src/java/ognl/OgnlRuntime.java +++ b/src/java/ognl/OgnlRuntime.java @@ -2410,13 +2410,11 @@ private static Method _getGetMethod(OgnlContext context, Class targetClass, Stri } if (firstPublicGetter == null && Modifier.isPublic(m.getModifiers()) && declaringClassIsPublic) { firstPublicGetter = m; + break; // Stop looking (this is the best possible match) } if (firstNonDefaultPublicInterfaceGetter == null && isNonDefaultPublicInterfaceMethod(m) && declaringClassIsPublic) { firstNonDefaultPublicInterfaceGetter = m; } - if (firstGetter != null && firstPublicGetter != null && firstNonDefaultPublicInterfaceGetter != null) { - break; - } } } result = (firstPublicGetter != null) ? firstPublicGetter : @@ -2502,13 +2500,11 @@ private static Method _getSetMethod(OgnlContext context, Class targetClass, Stri } if (firstPublicSetter == null && Modifier.isPublic(m.getModifiers()) && declaringClassIsPublic) { firstPublicSetter = m; + break; // Stop looking (this is the best possible match) } if (firstNonDefaultPublicInterfaceSetter == null && isNonDefaultPublicInterfaceMethod(m) && declaringClassIsPublic) { firstNonDefaultPublicInterfaceSetter = m; } - if (firstSetter != null && firstPublicSetter != null && firstNonDefaultPublicInterfaceSetter != null) { - break; - } } } From 4f8dec7904fedf3893eae3f21dd4a7e73736e106 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Tue, 16 Jul 2019 22:58:44 -0400 Subject: [PATCH 4/5] Update: - Provide a Java option (environment) flag allowing users to revert to the original "return first match" getter/setter logic if needed. By default the new "best match" getter/setter logic will be used. --- src/java/ognl/OgnlRuntime.java | 49 +++++++++++++++++++++++++ src/test/java/ognl/OgnlRuntimeTest.java | 30 +++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/src/java/ognl/OgnlRuntime.java b/src/java/ognl/OgnlRuntime.java index eaccda5a..da63ba13 100644 --- a/src/java/ognl/OgnlRuntime.java +++ b/src/java/ognl/OgnlRuntime.java @@ -124,6 +124,36 @@ public class OgnlRuntime { private static boolean _jdk15 = false; private static boolean _jdkChecked = false; + /** + * Allow users to revert to the old "first match" lookup for getters/setters by OGNL using the JVM options: + * -Dognl.UseFirstMatchGetSetLookup=true + * -Dognl.UseFirstMatchGetSetLookup=false + * + * Note: Using the "false" value has the same effect as omitting the option completely. + * The default behaviour is to use the "best match" lookup for getters/setters. + * Using the "true" value reverts to the older "first match" lookup for getters/setters + * (in the event the "best match" processing causes issues for existing applications). + */ + static final String USE_FIRSTMATCH_GETSET_LOOKUP = "ognl.UseFirstMatchGetSetLookup"; + + /** + * Hold environment flag state associated with USE_FIRSTMATCH_GETSET_LOOKUP. + * Default: false (if not set) + */ + private static final boolean _useFirstMatchGetSetLookup; + static { + boolean initialFlagState = false; + try { + final String propertyString = System.getProperty(USE_FIRSTMATCH_GETSET_LOOKUP); + if (propertyString != null && propertyString.length() > 0) { + initialFlagState = Boolean.parseBoolean(propertyString); + } + } catch (Exception ex) { + // Unavailable (SecurityException, etc.) + } + _useFirstMatchGetSetLookup = initialFlagState; + } + static final ClassCache _methodAccessors = new ClassCacheImpl(); static final ClassCache _propertyAccessors = new ClassCacheImpl(); static final ClassCache _elementsAccessors = new ClassCacheImpl(); @@ -2407,6 +2437,9 @@ private static Method _getGetMethod(OgnlContext context, Class targetClass, Stri boolean declaringClassIsPublic = Modifier.isPublic(m.getDeclaringClass().getModifiers()); if (firstGetter == null) { firstGetter = m; + if (_useFirstMatchGetSetLookup) { + break; // Stop looking (emulate original logic, return 1st match) + } } if (firstPublicGetter == null && Modifier.isPublic(m.getModifiers()) && declaringClassIsPublic) { firstPublicGetter = m; @@ -2497,6 +2530,9 @@ private static Method _getSetMethod(OgnlContext context, Class targetClass, Stri boolean declaringClassIsPublic = Modifier.isPublic(m.getDeclaringClass().getModifiers()); if (firstSetter == null) { firstSetter = m; + if (_useFirstMatchGetSetLookup) { + break; // Stop looking (emulate original logic, return 1st match) + } } if (firstPublicSetter == null && Modifier.isPublic(m.getModifiers()) && declaringClassIsPublic) { firstPublicSetter = m; @@ -3545,5 +3581,18 @@ boolean containsKey(Class clazz, String propertyName) } + /** + * Returns the value of the flag indicating whether the old "first match" lookup for + * getters/setters is in effect or not. + * + * Note: Value is controlled by a Java option flag {@link OgnlRuntime#USE_FIRSTMATCH_GETSET_LOOKUP}. + * + * @return true if the old "first match" lookup is in effect, false otherwise. + * + * @since 3.1.25 + */ + public static boolean getUseFirstMatchGetSetLookupValue() { + return _useFirstMatchGetSetLookup; + } } diff --git a/src/test/java/ognl/OgnlRuntimeTest.java b/src/test/java/ognl/OgnlRuntimeTest.java index f0597b3f..e561c6e3 100644 --- a/src/test/java/ognl/OgnlRuntimeTest.java +++ b/src/test/java/ognl/OgnlRuntimeTest.java @@ -316,4 +316,34 @@ public void testPerformanceMultipleClassesMultipleMethodsMultipleThreads() throw (cumulativelRunTestElapsedNanoTime / (1000 * 1000 * 1000)) / totalNumberOfRunTestRuns + " s)"); } } + + /** + * Test OgnlRuntime value for _useFirstMatchGetSetLookup based on the System property + * represented by {@link OgnlRuntime#USE_FIRSTMATCH_GETSET_LOOKUP}. + */ + @Test + public void testUseFirstMatchGetSetStateFlag() { + // Ensure no exceptions, basic ouput for test report and sanity check on flag state. + final boolean defaultValue = false; // Expected non-configured default + boolean optionDefinedInEnvironment = false; // Track if option defined in environment + boolean flagValueFromEnvironment = false; // Value result from environment retrieval + try { + final String propertyString = System.getProperty(OgnlRuntime.USE_FIRSTMATCH_GETSET_LOOKUP); + if (propertyString != null && propertyString.length() > 0) { + optionDefinedInEnvironment = true; + flagValueFromEnvironment = Boolean.parseBoolean(propertyString); + } + } catch (Exception ex) { + // Unavailable (SecurityException, etc.) + } + if (optionDefinedInEnvironment) { + System.out.println("System property " + OgnlRuntime.USE_FIRSTMATCH_GETSET_LOOKUP + " value: " + flagValueFromEnvironment); + } else { + System.out.println("System property " + OgnlRuntime.USE_FIRSTMATCH_GETSET_LOOKUP + " not present. Default value should be: " + defaultValue); + } + System.out.println("Current OGNL value for Use First Match Get/Set State Flag: " + OgnlRuntime.getUseFirstMatchGetSetLookupValue()); + Assert.assertEquals("Mismatch between system property (or default) and OgnlRuntime _useFirstMatchGetSetLookup flag state ?", + optionDefinedInEnvironment ? flagValueFromEnvironment : defaultValue, OgnlRuntime.getUseFirstMatchGetSetLookupValue()); + } + } \ No newline at end of file From da3eb4b6a6f7b9bbbddc1e81e939dd77ca892909 Mon Sep 17 00:00:00 2001 From: JCgH4164838Gh792C124B5 <43964333+JCgH4164838Gh792C124B5@users.noreply.github.com> Date: Sun, 8 Sep 2019 23:34:13 -0400 Subject: [PATCH 5/5] Update source/target from 1.5 to 1.6 (Travis CI build failing with 1.5) --- pom.xml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index 52579714..7416feb3 100644 --- a/pom.xml +++ b/pom.xml @@ -78,11 +78,12 @@ + org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.6 + 1.6 @@ -92,8 +93,8 @@ testCompile - 1.5 - 1.5 + 1.6 + 1.6