Merge branch 'pr/8'

YiranCdr · Dec 18, 2018 · 2d0d32e · 2d0d32e
2 parents fd9b9fd + e7d212e
commit 2d0d32e
Show file tree

Hide file tree

Showing 7 changed files with 64 additions and 19 deletions.
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
@@ -60,6 +60,8 @@ New Features in 3.2:
 Bugs Fixed in 3.2:
 ==================
 
+* JEXL-281:      MethodExecutor incorrectly tests for empty parameters list
+* JEXL-279:      Null variables property access do not throw exceptions
 * JEXL-278:      Ambiguous exceptions should point to actual statement ambiguity
 * JEXL-272:      Dereferencing null property not reported on method call
 * JEXL-271:      Hoisted variable is lost when currying lambda

diff --git a/src/main/java/org/apache/commons/jexl3/JexlInfo.java b/src/main/java/org/apache/commons/jexl3/JexlInfo.java
@@ -90,7 +90,8 @@ public JexlInfo() {
             if (!className.equals(cname)) {
                 // go deeper if called from jexl implementation classes
                 if (className.startsWith(pkgname + ".internal.")
-                    || className.startsWith(pkgname + ".Jexl")) {
+                    || className.startsWith(pkgname + ".Jexl")
+                    || className.startsWith(pkgname + ".parser")) {
                     cname = className;
                 } else {
                     break;

diff --git a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -2194,6 +2194,13 @@ protected Object visit(ASTReference node, Object data) {
                 } else {
                     antish = false;
                 }
+            } else if (objectNode instanceof ASTArrayAccess) {
+                if (object == null) {
+                    ptyNode = objectNode;
+                    break;
+                } else {
+                    antish = false;
+                }
             }
             // attempt to evaluate the property within the object (visit(ASTIdentifierAccess node))
             object = objectNode.jjtAccept(this, object);
@@ -2206,14 +2213,10 @@ protected Object visit(ASTReference node, Object data) {
                     JexlNode first = node.jjtGetChild(0);
                     if (first instanceof ASTIdentifier) {
                         ASTIdentifier afirst = (ASTIdentifier) first;
-                        if (afirst.getSymbol() < 0) {
-                            ant = new StringBuilder(afirst.getName());
-                        } else {
-                            break main;
-                        }
+                        ant = new StringBuilder(afirst.getName());
                     } else {
                         ptyNode = objectNode;
-                        break main;
+                        break;
                     }
                 }
                 for (; v <= c; ++v) {

diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodExecutor.java
@@ -70,13 +70,11 @@ private MethodExecutor(Class<?> c, java.lang.reflect.Method m, MethodKey k) {
         super(c, m, k);
         int vastart = -1;
         Class<?> vaclass = null;
-        if (method != null) {
-            Class<?>[] formal = method.getParameterTypes();
+        if (MethodKey.isVarArgs(method)) {
             // if the last parameter is an array, the method is considered as vararg
-            if (formal.length > 0 && MethodKey.isVarArgs(method)) {
-                vastart = formal.length - 1;
-                vaclass = formal[vastart].getComponentType();
-            }
+            Class<?>[] formal = method.getParameterTypes();
+            vastart = formal.length - 1;
+            vaclass = formal[vastart].getComponentType();
         }
         vaStart = vastart;
         vaClass = vaclass;
@@ -153,5 +151,3 @@ private Object[] handleVarArg(Object[] actual) {
         return actual;
     }
 }
-
-
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java
@@ -201,7 +201,7 @@ public static boolean isVarArgs(final Method method) {
         }
         // before climbing up the hierarchy, verify that the last parameter is an array
         final Class<?>[] ptypes = method.getParameterTypes();
-        if (ptypes.length > 0 && ptypes[ptypes.length - 1].getComponentType() == null) {
+        if (ptypes.length == 0 || ptypes[ptypes.length - 1].getComponentType() == null) {
             return false;
         }
         final String mname = method.getName();
@@ -383,9 +383,11 @@ private static boolean isInvocationConvertible(
         /* Primitive conversion check. */
         if (formal.isPrimitive()) {
             Class<?>[] clist = strict ? STRICT_CONVERTIBLES.get(formal) : CONVERTIBLES.get(formal);
-            for(int c = 0; c < clist.length; ++c) {
-                if (actual == clist[c]) {
-                    return true;
+            if (clist != null) {
+                for (int c = 0; c < clist.length; ++c) {
+                    if (actual == clist[c]) {
+                        return true;
+                    }
                 }
             }
             return false;

diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml
@@ -26,6 +26,12 @@
     </properties>
     <body>
         <release version="3.2" date="unreleased">
+            <action dev="henrib" type="fix" issue="JEXL-281" due-to="Mirek Hankus">
+                MethodExecutor incorrectly tests for empty parameters list
+            </action>
+            <action dev="henrib" type="fix" issue="JEXL-279">
+                Null variables property access do not throw exceptions
+            </action>
             <action dev="henrib" type="fix" issue="JEXL-278">
                 Ambiguous exceptions should point to actual statement ambiguity
             </action>

diff --git a/src/test/java/org/apache/commons/jexl3/Issues200Test.java b/src/test/java/org/apache/commons/jexl3/Issues200Test.java
@@ -631,4 +631,39 @@ public void test278() throws Exception {
             Assert.assertEquals(src, ctls[i], value);
         }
     }
+
+    @Test
+    public void test279() throws Exception {
+        Object result;
+        JexlScript script;
+        JexlContext ctxt = new MapContext();
+        ctxt.set("y.z", null);
+        ctxt.set("z", null);
+        String[] srcs = new String[]{
+             "var x = null; x[0];",
+             "var x = null; x.0;",
+             "z[0]",
+             "z.0",
+             "y.z[0]",
+             "y.z.0",
+        };
+        for(boolean strict : new boolean[]{ true, false} ) {
+            JexlEngine jexl = new JexlBuilder().safe(false).strict(strict).create();
+            for (String src : srcs) {
+                script = jexl.createScript(src);
+                try {
+                    result = script.execute(ctxt);
+                    if (strict) {
+                        Assert.fail("should have failed: " + src);
+                    }
+                    // not reachable
+                    Assert.assertNull("non-null result ?!", result);
+                } catch (JexlException xany) {
+                   if (!strict) {
+                       Assert.fail(src + ", should not have thrown " + xany);
+                   }
+                }
+            }
+        }
+    }
 }