You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2022/02/25 10:21:17 UTC

[tomcat] branch main updated: Fix edge case in EL method matching

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
     new afa662d  Fix edge case in EL method matching
afa662d is described below

commit afa662d7d71747769d1a97c999d696883859a5d0
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Feb 25 10:20:49 2022 +0000

    Fix edge case in EL method matching
    
    When resolving methods in EL expressions that use beans and/or static
    fields, ensure that any custom type conversion is considered when
    identifying the method to call.
---
 java/jakarta/el/BeanELResolver.java        |  6 +--
 java/jakarta/el/StaticFieldELResolver.java |  9 ++--
 java/jakarta/el/Util.java                  | 31 ++++++-------
 test/jakarta/el/TestBeanELResolver.java    | 74 ++++++++++++++++++++++++++++++
 webapps/docs/changelog.xml                 |  9 ++++
 5 files changed, 102 insertions(+), 27 deletions(-)

diff --git a/java/jakarta/el/BeanELResolver.java b/java/jakarta/el/BeanELResolver.java
index b39fa6d..8d41119 100644
--- a/java/jakarta/el/BeanELResolver.java
+++ b/java/jakarta/el/BeanELResolver.java
@@ -143,12 +143,10 @@ public class BeanELResolver extends ELResolver {
         String methodName = factory.coerceToType(method, String.class);
 
         // Find the matching method
-        Method matchingMethod =
-                Util.findMethod(base.getClass(), base, methodName, paramTypes, params);
+        Method matchingMethod = Util.findMethod(context, base.getClass(), base, methodName, paramTypes, params);
 
         Object[] parameters = Util.buildParameters(
-                matchingMethod.getParameterTypes(), matchingMethod.isVarArgs(),
-                params);
+                context, matchingMethod.getParameterTypes(), matchingMethod.isVarArgs(), params);
 
         Object result = null;
         try {
diff --git a/java/jakarta/el/StaticFieldELResolver.java b/java/jakarta/el/StaticFieldELResolver.java
index 365b979..831309a 100644
--- a/java/jakarta/el/StaticFieldELResolver.java
+++ b/java/jakarta/el/StaticFieldELResolver.java
@@ -92,11 +92,10 @@ public class StaticFieldELResolver extends ELResolver {
             String methodName = (String) method;
 
             if ("<init>".equals(methodName)) {
-                Constructor<?> match =
-                        Util.findConstructor(clazz, paramTypes, params);
+                Constructor<?> match = Util.findConstructor(context, clazz, paramTypes, params);
 
                 Object[] parameters = Util.buildParameters(
-                        match.getParameterTypes(), match.isVarArgs(), params);
+                        context, match.getParameterTypes(), match.isVarArgs(), params);
 
                 Object result = null;
 
@@ -113,7 +112,7 @@ public class StaticFieldELResolver extends ELResolver {
 
             } else {
                 // Static method so base should be null
-                Method match = Util.findMethod(clazz, null, methodName, paramTypes, params);
+                Method match = Util.findMethod(context, clazz, null, methodName, paramTypes, params);
 
                 if (match == null) {
                     throw new MethodNotFoundException(Util.message(context,
@@ -122,7 +121,7 @@ public class StaticFieldELResolver extends ELResolver {
                 }
 
                 Object[] parameters = Util.buildParameters(
-                        match.getParameterTypes(), match.isVarArgs(), params);
+                        context, match.getParameterTypes(), match.isVarArgs(), params);
 
                 Object result = null;
                 try {
diff --git a/java/jakarta/el/Util.java b/java/jakarta/el/Util.java
index 880c1db..badccfb 100644
--- a/java/jakarta/el/Util.java
+++ b/java/jakarta/el/Util.java
@@ -198,7 +198,7 @@ class Util {
      * This method duplicates code in org.apache.el.util.ReflectionUtil. When
      * making changes keep the code in sync.
      */
-    static Method findMethod(Class<?> clazz, Object base, String methodName,
+    static Method findMethod(ELContext context, Class<?> clazz, Object base, String methodName,
             Class<?>[] paramTypes, Object[] paramValues) {
 
         if (clazz == null || methodName == null) {
@@ -215,7 +215,7 @@ class Util {
 
         List<Wrapper<Method>> wrappers = Wrapper.wrap(methods, methodName);
 
-        Wrapper<Method> result = findWrapper(clazz, wrappers, methodName, paramTypes, paramValues);
+        Wrapper<Method> result = findWrapper(context, clazz, wrappers, methodName, paramTypes, paramValues);
 
         return getMethod(clazz, base, result.unWrap());
     }
@@ -225,7 +225,7 @@ class Util {
      * making changes keep the code in sync.
      */
     @SuppressWarnings("null")
-    private static <T> Wrapper<T> findWrapper(Class<?> clazz, List<Wrapper<T>> wrappers,
+    private static <T> Wrapper<T> findWrapper(ELContext context, Class<?> clazz, List<Wrapper<T>> wrappers,
             String name, Class<?>[] paramTypes, Object[] paramValues) {
 
         Map<Wrapper<T>,MatchResult> candidates = new HashMap<>();
@@ -291,7 +291,7 @@ class Util {
                                 noMatch = true;
                                 break;
                             } else {
-                                if (isCoercibleFrom(paramValues[j], varType)) {
+                                if (isCoercibleFrom(context, paramValues[j], varType)) {
                                     coercibleMatch++;
                                     varArgsMatch++;
                                 } else {
@@ -314,7 +314,7 @@ class Util {
                             noMatch = true;
                             break;
                         } else {
-                            if (isCoercibleFrom(paramValues[i], mParamTypes[i])) {
+                            if (isCoercibleFrom(context, paramValues[i], mParamTypes[i])) {
                                 coercibleMatch++;
                             } else {
                                 noMatch = true;
@@ -510,11 +510,11 @@ class Util {
      * This method duplicates code in org.apache.el.util.ReflectionUtil. When
      * making changes keep the code in sync.
      */
-    private static boolean isCoercibleFrom(Object src, Class<?> target) {
+    private static boolean isCoercibleFrom(ELContext context, Object src, Class<?> target) {
         // TODO: This isn't pretty but it works. Significant refactoring would
         //       be required to avoid the exception.
         try {
-            getExpressionFactory().coerceToType(src, target);
+            context.convertToType(src, target);
         } catch (ELException e) {
             return false;
         }
@@ -580,7 +580,7 @@ class Util {
     }
 
 
-    static Constructor<?> findConstructor(Class<?> clazz, Class<?>[] paramTypes,
+    static Constructor<?> findConstructor(ELContext context, Class<?> clazz, Class<?>[] paramTypes,
             Object[] paramValues) {
 
         String methodName = "<init>";
@@ -599,7 +599,7 @@ class Util {
 
         List<Wrapper<Constructor<?>>> wrappers = Wrapper.wrap(constructors);
 
-        Wrapper<Constructor<?>> wrapper = findWrapper(clazz, wrappers, methodName, paramTypes, paramValues);
+        Wrapper<Constructor<?>> wrapper = findWrapper(context, clazz, wrappers, methodName, paramTypes, paramValues);
 
         Constructor<?> constructor = wrapper.unWrap();
 
@@ -622,9 +622,7 @@ class Util {
     }
 
 
-    static Object[] buildParameters(Class<?>[] parameterTypes,
-            boolean isVarArgs,Object[] params) {
-        ExpressionFactory factory = getExpressionFactory();
+    static Object[] buildParameters(ELContext context, Class<?>[] parameterTypes, boolean isVarArgs,Object[] params) {
         Object[] parameters = null;
         if (parameterTypes.length > 0) {
             parameters = new Object[parameterTypes.length];
@@ -637,22 +635,19 @@ class Util {
                 int varArgIndex = parameterTypes.length - 1;
                 // First argCount-1 parameters are standard
                 for (int i = 0; (i < varArgIndex); i++) {
-                    parameters[i] = factory.coerceToType(params[i],
-                            parameterTypes[i]);
+                    parameters[i] = context.convertToType(params[i], parameterTypes[i]);
                 }
                 // Last parameter is the varargs
                 Class<?> varArgClass = parameterTypes[varArgIndex].getComponentType();
                 final Object varargs = Array.newInstance(varArgClass, (paramCount - varArgIndex));
                 for (int i = (varArgIndex); i < paramCount; i++) {
-                    Array.set(varargs, i - varArgIndex,
-                            factory.coerceToType(params[i], varArgClass));
+                    Array.set(varargs, i - varArgIndex, context.convertToType(params[i], varArgClass));
                 }
                 parameters[varArgIndex] = varargs;
             } else {
                 parameters = new Object[parameterTypes.length];
                 for (int i = 0; i < parameterTypes.length; i++) {
-                    parameters[i] = factory.coerceToType(params[i],
-                            parameterTypes[i]);
+                    parameters[i] = context.convertToType(params[i], parameterTypes[i]);
                 }
             }
         }
diff --git a/test/jakarta/el/TestBeanELResolver.java b/test/jakarta/el/TestBeanELResolver.java
index 864e1a7..f32fe3e 100644
--- a/test/jakarta/el/TestBeanELResolver.java
+++ b/test/jakarta/el/TestBeanELResolver.java
@@ -703,6 +703,31 @@ public class TestBeanELResolver {
     }
 
     @Test
+    public void testInvokeVarargsCoerce22() {
+        BeanELResolver resolver = new BeanELResolver();
+        StandardELContext context = new StandardELContext(ELManager.getExpressionFactory());
+        context.addELResolver(new StringToLongNeverFailResolver());
+
+        Object result = resolver.invoke(context, new TesterBean(BEAN_NAME), "getNameVarargs",
+                new Class<?>[] { String.class, String.class, String.class },
+                new Object[] { "AA", "BB", "CC" });
+
+        Assert.assertEquals(BEAN_NAME, result);
+    }
+
+    @Test(expected=MethodNotFoundException.class)
+    public void testInvokeVarargsCoerce23() {
+        BeanELResolver resolver = new BeanELResolver();
+        StandardELContext context = new StandardELContext(ELManager.getExpressionFactory());
+
+        Object result = resolver.invoke(context, new TesterBean(BEAN_NAME), "getNameVarargs",
+                new Class<?>[] { String.class, String.class, String.class },
+                new Object[] { "AA", "BB", "CC" });
+
+        Assert.assertEquals(BEAN_NAME, result);
+    }
+
+    @Test
     public void testInvokeVarargs01() {
         BeanELResolver resolver = new BeanELResolver();
         ELContext context = new StandardELContext(ELManager.getExpressionFactory());
@@ -1011,4 +1036,53 @@ public class TestBeanELResolver {
         GET_VALUE, SET_VALUE, GET_TYPE, INVOKE
     }
 
+
+    /*
+     * Custom resolver that will always convert a string to an integer. If the
+     * provided string is not a valid integer, zero will be returned.
+     */
+    private static class StringToLongNeverFailResolver extends ELResolver {
+
+        @Override
+        public Object getValue(ELContext context, Object base, Object property) {
+            return null;
+        }
+
+        @Override
+        public Class<?> getType(ELContext context, Object base, Object property) {
+            return null;
+        }
+
+        @Override
+        public void setValue(ELContext context, Object base, Object property, Object value) {
+            throw new PropertyNotWritableException();
+        }
+
+        @Override
+        public boolean isReadOnly(ELContext context, Object base, Object property) {
+            return true;
+        }
+
+        @Override
+        public Class<?> getCommonPropertyType(ELContext context, Object base) {
+            return null;
+        }
+
+        @Override
+        public <T> T convertToType(ELContext context, Object obj, Class<T> type) {
+            if (Integer.class.equals(type) && obj instanceof String) {
+                context.setPropertyResolved(true);
+                Integer result;
+                try {
+                    result = Integer.valueOf((String) obj);
+                } catch (NumberFormatException e) {
+                    result = Integer.valueOf(0);
+                }
+                @SuppressWarnings("unchecked")
+                T t = (T) result;
+                return t;
+            }
+            return super.convertToType(context, obj, type);
+        }
+    }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index d399207..4b6f9dc 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -105,6 +105,15 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 10.1.0-M12 (markt)" rtext="in development">
+  <subsection name="Jasper">
+    <changelog>
+      <fix>
+        When resolving methods in EL expressions that use beans and/or static
+        fields, ensure that any custom type conversion is considered when
+        identifying the method to call. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Web applications">
     <changelog>
       <fix>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org