You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2018/02/07 10:03:59 UTC

[commons-jexl] 01/03: JEXL-252, JEXL-250: hardened logic and more tests

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

henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git

commit 1e8d7570df17e02646ade96c5f90e1e7b3202913
Author: Henri Biestro <he...@apache.org>
AuthorDate: Tue Feb 6 12:11:46 2018 +0100

    JEXL-252, JEXL-250: hardened logic and more tests
---
 .../apache/commons/jexl3/internal/Interpreter.java | 24 +++++-
 .../commons/jexl3/parser/ASTIdentifierAccess.java  |  2 +-
 .../apache/commons/jexl3/PropertyAccessTest.java   | 94 +++++++++++++++++++++-
 3 files changed, 112 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
index 167de1a..cdf48e9 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -111,6 +111,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
+import org.apache.commons.jexl3.JxltEngine;
 
 
 /**
@@ -1009,17 +1010,32 @@ public class Interpreter extends InterpreterBase {
      */
     private Object evalIdentifier(ASTIdentifierAccess node) {
         if (node instanceof ASTIdentifierAccessJxlt) {
-            ASTIdentifierAccessJxlt accessJxlt = (ASTIdentifierAccessJxlt) node;
+            final ASTIdentifierAccessJxlt accessJxlt = (ASTIdentifierAccessJxlt) node;
+            final String src = node.getName();
             TemplateEngine.TemplateExpression expr = (TemplateEngine.TemplateExpression) accessJxlt.getExpression();
             if (expr == null) {
                 TemplateEngine jxlt = jexl.jxlt();
-                expr = jxlt.parseExpression(node.jexlInfo(), node.getName(), frame != null ? frame.getScope() : null);
+                try {
+                    expr = jxlt.parseExpression(node.jexlInfo(), src, frame != null ? frame.getScope() : null);
+                } catch(JxltEngine.Exception xjxlt) {
+                    return node.isSafe()? null : unsolvableProperty(node, src, xjxlt);
+                }
                 accessJxlt.setExpression(expr);
             }
             if (expr != null) {
-                return expr.evaluate(frame, context);
+                Object name;
+                try {
+                    name = expr.evaluate(frame, context);
+                } catch(JxltEngine.Exception xjxlt) {
+                    return node.isSafe()? null : unsolvableProperty(node, src, xjxlt);
+                }
+                if (name != null) {
+                    Integer id = ASTIdentifierAccess.parseIdentifier(name.toString());
+                    return id != null? id : name;
+                }
+                return null;
             }
-            throw new JexlException.Property(node, node.getName());
+            return node.isSafe()? null : unsolvableProperty(node, src, null);
         } else {
             return node.getIdentifier();
         }
diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java
index de61bef..941d5f3 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccess.java
@@ -59,7 +59,7 @@ public class ASTIdentifierAccess extends JexlNode {
      * @param id the identifier
      * @return an integer or null
      */
-    private static Integer parseIdentifier(String id) {
+    public static Integer parseIdentifier(String id) {
         // hand coded because the was no way to fail on leading '0's using NumberFormat
         if (id != null) {
             final int length = id.length();
diff --git a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
index 7d81cdb..ca76014 100644
--- a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
+++ b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
@@ -51,7 +51,6 @@ public class PropertyAccessTest extends JexlTestCase {
         Integer i42 = Integer.valueOf(42);
         Integer i43 = Integer.valueOf(43);
         String s42 = "fourty-two";
-        String s43 = "fourty-three";
         Object[] foo = new Object[3];
         foo[0] = foo;
         foo[1] = i42;
@@ -60,7 +59,7 @@ public class PropertyAccessTest extends JexlTestCase {
         asserter.setVariable("zero", Integer.valueOf(0));
         asserter.setVariable("one", Integer.valueOf(1));
         asserter.setVariable("two", Integer.valueOf(2));
-        for(int l = 0; l < 2; ++l) {
+        for (int l = 0; l < 2; ++l) {
             asserter.assertExpression("foo.0", foo);
             asserter.assertExpression("foo.0.'0'", foo);
             asserter.assertExpression("foo.'1'", foo[1]);
@@ -68,7 +67,26 @@ public class PropertyAccessTest extends JexlTestCase {
             asserter.assertExpression("foo.0.'1' = 43", i43);
             asserter.assertExpression("foo.0.'1'", i43);
             asserter.assertExpression("foo.0.'1' = 42", i42);
-            asserter.assertExpression("foo.0.'1'", i42);
+            //
+            asserter.assertExpression("foo?.0.'1'", i42);
+            asserter.assertExpression("foo?.0", foo);
+            asserter.assertExpression("foo?.0.'0'", foo);
+            asserter.assertExpression("foo?.'1'", foo[1]);
+            asserter.assertExpression("foo.0?.'1'", foo[1]);
+            asserter.assertExpression("foo?.0.'1' = 43", i43);
+            asserter.assertExpression("foo?.0?.'1'", i43);
+            asserter.assertExpression("foo?.0.'1' = 42", i42);
+            asserter.assertExpression("foo?.0.'1'", i42);
+            //
+            asserter.assertExpression("foo?.0.`1`", i42);
+            asserter.assertExpression("foo?.0", foo);
+            asserter.assertExpression("foo?.0.'0'", foo);
+            asserter.assertExpression("foo?.`1`", foo[1]);
+            asserter.assertExpression("foo?.0.`1`", foo[1]);
+            asserter.assertExpression("foo?.0.`${one}` = 43", i43);
+            asserter.assertExpression("foo.0?.`${one}`", i43);
+            asserter.assertExpression("foo.0.`${one}` = 42", i42);
+            asserter.assertExpression("foo?.0?.`${one}`", i42);
         }
     }
 
@@ -299,4 +317,74 @@ public class PropertyAccessTest extends JexlTestCase {
         String dbgdata = dbg.toString();
         Assert.assertEquals("foo.'q u u x'", dbgdata);
     }
+
+    @Test
+    public void testErroneousIdentifier() throws Exception {
+        MapContext ctx = new MapContext();
+        JexlEngine engine = new JexlBuilder().strict(true).silent(false).create();
+        
+        // base succeeds
+        String stmt = "(x)->{ x?.class1 ?? 'oops' }";
+        JexlScript script = engine.createScript(stmt);
+        Object result = script.execute(ctx, "querty");
+        Assert.assertEquals("oops", result);
+
+        // fail with unknown property
+        try {
+            stmt = "(x)->{ x.class1 ?? 'oops' }";
+            script = engine.createScript(stmt);
+            result = script.execute(ctx, "querty");
+            Assert.fail("should have failed!");
+            Assert.assertNull(result); // unreachable
+        } catch (JexlException.Property xjexl) {
+            Assert.assertTrue(xjexl.getMessage().contains("class1"));
+        }
+
+        // succeeds with jxlt & strict navigation
+        ctx.set("al", "la");
+        stmt = "(x)->{ x.`c${al}ss` ?? 'oops' }";
+        script = engine.createScript(stmt);
+        result = script.execute(ctx, "querty");
+        Assert.assertEquals("querty".getClass(), result);
+
+        // succeeds with jxlt & lenient navigation
+        stmt = "(x)->{ x?.`c${al}ss` ?? 'oops' }";
+        script = engine.createScript(stmt);
+        result = script.execute(ctx, "querty");
+        Assert.assertEquals("querty".getClass(), result);
+
+        // fails with jxlt & lenient navigation
+        stmt = "(x)->{ x?.`c${la}ss` ?? 'oops' }";
+        script = engine.createScript(stmt);
+        result = script.execute(ctx, "querty");
+        Assert.assertEquals("oops", result);
+
+        // fails with jxlt & strict navigation
+        try {
+            stmt = "(x)->{ x.`c${la}ss` ?? 'oops' }";
+            script = engine.createScript(stmt);
+            result = script.execute(ctx, "querty");
+            Assert.assertEquals("oops", result);
+            Assert.fail("should have failed!");
+        } catch (JexlException.Property xjexl) {
+            Assert.assertTrue(xjexl.getMessage().contains("c${la}ss"));
+        }
+
+        // fails with jxlt & lenient navigation
+        stmt = "(x)->{ x?.`c${la--ss` ?? 'oops' }";
+        script = engine.createScript(stmt);
+        result = script.execute(ctx, "querty");
+        Assert.assertEquals("oops", result);
+
+        // fails with jxlt & strict navigation
+        try {
+            stmt = "(x)->{ x.`c${la--ss` ?? 'oops' }";
+            script = engine.createScript(stmt);
+            result = script.execute(ctx, "querty");
+            Assert.assertEquals("oops", result);
+            Assert.fail("should have failed!");
+        } catch (JexlException.Property xjexl) {
+            Assert.assertTrue(xjexl.getMessage().contains("c${la--ss"));
+        }
+    }
 }
\ No newline at end of file

-- 
To stop receiving notification emails like this one, please contact
henrib@apache.org.