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 2019/05/22 16:29:55 UTC

[commons-jexl] branch master updated: JEXL-30{1,3,4,5}: modified syntax (jjt) to better handle blocks and 'var' in identifiers; modified resolution and error handling of solving dot/array variables; tests, changes, release-notes

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


The following commit(s) were added to refs/heads/master by this push:
     new 68d1b31  JEXL-30{1,3,4,5}: modified syntax (jjt) to better handle blocks and 'var' in identifiers; modified resolution and error handling of solving dot/array variables; tests, changes, release-notes
68d1b31 is described below

commit 68d1b317f38e49a6cad3c283ad05c7d79f29b490
Author: henrib <he...@apache.org>
AuthorDate: Wed May 22 18:29:13 2019 +0200

    JEXL-30{1,3,4,5}: modified syntax (jjt) to better handle blocks and 'var' in identifiers; modified resolution and error handling of solving dot/array variables; tests, changes, release-notes
---
 RELEASE-NOTES.txt                                  |  4 +
 .../apache/commons/jexl3/internal/Interpreter.java | 85 ++++++++++++----------
 .../commons/jexl3/internal/InterpreterBase.java    |  2 +-
 .../commons/jexl3/internal/TemplateDebugger.java   |  2 +
 .../org/apache/commons/jexl3/parser/JexlNode.java  |  8 ++
 .../org/apache/commons/jexl3/parser/Parser.jjt     | 19 +++--
 src/site/xdoc/changes.xml                          | 12 +++
 .../org/apache/commons/jexl3/Issues300Test.java    | 65 +++++++++++++++--
 .../java/org/apache/commons/jexl3/ScriptTest.java  | 13 ++++
 .../java/org/apache/commons/jexl3/VarTest.java     | 10 +++
 .../apache/commons/jexl3/parser/ParserTest.java    | 14 ++--
 11 files changed, 177 insertions(+), 57 deletions(-)

diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index aa33c24..b5f402e 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -72,6 +72,10 @@ New Features in 3.2:
 Bugs Fixed in 3.2:
 ==================
 
+* JEXL-305:      Script debugger produces incorrect syntax
+* JEXL-304:      Error parsing overview.limit.var
+* JEXL-303:      Block syntax is broken
+* JEXL-301:      Array access operator does not fail on null object in non-strict arithmetic mode
 * JEXL-300:      Ant-ish variables should not use safe-access operator syntax
 * JEXL-299:      Improve message error when method could not be found
 * JEXL-296:      Real literal in scientific format is not parsed without suffix
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 fbf6c5b..8dd9a0a 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -1078,6 +1078,7 @@ public class Interpreter extends InterpreterBase {
                 }
             } else if (objectNode instanceof ASTArrayAccess) {
                 if (object == null) {
+                    ptyNode = objectNode;
                     break;
                 } else {
                     antish = false;
@@ -1090,61 +1091,69 @@ public class Interpreter extends InterpreterBase {
                 // disallow mixing antish variable & bean with same root; avoid ambiguity
                 antish = false;
             } else if (antish) {
-                // skip the first node case since it was trialed in jjtAccept above and returned null
-                if (c > 0) {
-                    // create first from first node
-                    if (ant == null) {
-                        // if we still have a null object, check for an antish variable
-                        JexlNode first = node.jjtGetChild(0);
-                        if (first instanceof ASTIdentifier) {
-                            ASTIdentifier afirst = (ASTIdentifier) first;
-                            ant = new StringBuilder(afirst.getName());
-                        } else {
-                            // not an identifier, not antish
-                            ptyNode = objectNode;
-                            break main;
+                // create first from first node
+                if (ant == null) {
+                    // if we still have a null object, check for an antish variable
+                    JexlNode first = node.jjtGetChild(0);
+                    if (first instanceof ASTIdentifier) {
+                        ASTIdentifier afirst = (ASTIdentifier) first;
+                        ant = new StringBuilder(afirst.getName());
+                        // skip the first node case since it was trialed in jjtAccept above and returned null
+                        if (c == 0) {
+                            continue;
                         }
+                    } else {
+                        // not an identifier, not antish
+                        ptyNode = objectNode;
+                        break main;
                     }
-                    // catch up to current node
-                    for (; v <= c; ++v) {
-                        JexlNode child = node.jjtGetChild(v);
-                        if (child instanceof ASTIdentifierAccess) {
-                            ASTIdentifierAccess achild = (ASTIdentifierAccess) child;
-                            if (achild.isSafe() || achild.isExpression()) {
-                                break main;
-                            }
-                            ant.append('.');
-                            ant.append(achild.getName());
-                        } else {
-                            // not an identifier, not antish
-                            ptyNode = objectNode;
+                }
+                // catch up to current node
+                for (; v <= c; ++v) {
+                    JexlNode child = node.jjtGetChild(v);
+                    if (child instanceof ASTIdentifierAccess) {
+                        ASTIdentifierAccess achild = (ASTIdentifierAccess) child;
+                        if (achild.isSafe() || achild.isExpression()) {
                             break main;
                         }
+                        ant.append('.');
+                        ant.append(achild.getName());
+                    } else {
+                        // not an identifier, not antish
+                        ptyNode = objectNode;
+                        break main;
                     }
-                    // solve antish
-                    object = context.get(ant.toString());
                 }
+                // solve antish
+                object = context.get(ant.toString());
             } else if (c != numChildren - 1) {
                 // only the last one may be null
                 ptyNode = objectNode;
                 break; //
             }
         }
+        // am I the left-hand side of a safe op ?
         if (object == null && !node.isTernaryProtected()) {
             if (ptyNode != null) {
-                // am I the left-hand side of a safe op ?
-                return ptyNode.isSafeLhs(jexl.safe)
-                       ? null
-                       : unsolvableProperty(node, stringifyProperty(ptyNode), ptyNode == objectNode, null);
+                if (ptyNode.isSafeLhs(jexl.safe)) {
+                    return null;
+                }
+                if (ant != null) {
+                    String aname = ant.toString();
+                    boolean undefined = !(context.has(aname) || isLocalVariable(node, 0) || isFunctionCall(node));
+                    return unsolvableVariable(node, aname, undefined);
+                }
+                return unsolvableProperty(node, stringifyProperty(ptyNode), ptyNode == objectNode, null);
             }
             if (antish) {
-                String pstr = stringifyProperty(node);
-                String aname = ant != null? ant.toString() : pstr;
+                if (node.isSafeLhs(jexl.safe)) {
+                    return null;
+                }
+                String aname = ant != null ? ant.toString() : "?";
                 boolean undefined = !(context.has(aname) || isLocalVariable(node, 0) || isFunctionCall(node));
-                // variable unknown in context and not a local
-                return node.isSafeLhs(jexl.safe)
-                        ? null
-                        : unsolvableVariable(node, undefined? pstr : aname, undefined);
+                if (undefined || arithmetic.isStrict()) {
+                    return unsolvableVariable(node, aname, undefined);
+                }
             }
         }
         return object;
diff --git a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
index c2669e0..e24a95e 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
@@ -308,7 +308,7 @@ public abstract class InterpreterBase extends ParserVisitor {
      * @return throws JexlException if strict and not silent, null otherwise
      */
     protected Object unsolvableVariable(JexlNode node, String var, boolean undef) {
-        if (isStrictEngine() && (undef || arithmetic.isStrict())) {
+        if (isStrictEngine()) {
             throw new JexlException.Variable(node, var, undef);
         } else if (logger.isDebugEnabled()) {
             logger.debug(JexlException.variableError(node, var, undef));
diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateDebugger.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateDebugger.java
index 418087e..6ff3d68 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/TemplateDebugger.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateDebugger.java
@@ -193,6 +193,7 @@ public class TemplateDebugger extends Debugger {
                     case ' ':
                     case ';':
                         return;
+                    default: // continue
                 }
             }
         }
@@ -212,6 +213,7 @@ public class TemplateDebugger extends Debugger {
                 case '}':
                     builder.append('\n');
                     return;
+                default: // continue
             }
         }
     }
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
index 66b74d7..c8ae5e5 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
@@ -55,6 +55,14 @@ public abstract class JexlNode extends SimpleNode {
     public void jjtSetLastToken(Token t) {
         // nothing
     }
+    
+    public int getLine() {
+        return this.lc >>> 0xc;
+    }
+    
+    public int getColumn() {
+        return this.lc & 0xfff;
+    }
 
     /**
      * Gets the associated JexlInfo instance.
diff --git a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
index c463e02..e5c6f2b 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
+++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt
@@ -134,8 +134,8 @@ TOKEN_MGR_DECLS : {
 }
 
 <*> TOKEN : { /* SEPARATORS */
-      < LPAREN : "(" >
-    | < RPAREN : ")" >
+      < LPAREN : "(">
+    | < RPAREN : ")">
     | < LCURLY : "{" >
     | < RCURLY : "}" >
     | < LBRACKET : "[" >
@@ -211,7 +211,7 @@ TOKEN_MGR_DECLS : {
 
 <DOT_ID> TOKEN : /* IDENTIFIERS */
 {
-  < DOT_IDENTIFIER: ( [ "0"-"9", "a"-"z", "A"-"Z", "_", "$", "@" ])+ > { popDot(); } /* Revert state to default. */
+  < DOT_IDENTIFIER: ( [ "0"-"9", "a"-"z", "A"-"Z", "_", "$", "@" ])+ | "var" > { popDot(); } /* Revert state to default. */
 }
 
 <DEFAULT, REGISTERS> TOKEN : /* IDENTIFIERS */
@@ -313,12 +313,17 @@ void AnnotatedStatement() #AnnotatedStatement() : {}
     (LOOKAHEAD(<ANNOTATION>) Annotation())+ (LOOKAHEAD(1) Block() | Statement())
  }
 
+void StatementLookahead() #void : {}
+{
+    <SEMICOL> | <IF> | <FOR> | <DO> | <WHILE> | <RETURN> | <BREAK> | <CONTINUE> | <VAR> | <PRAGMA>
+}
+
 void Statement() #void : {}
 {
     <SEMICOL>
     | LOOKAHEAD(<ANNOTATION>) AnnotatedStatement()
-    | LOOKAHEAD(<LCURLY> Expression() <SEMICOL>) Block() // to disambiguate the set literals
-    | LOOKAHEAD(<LCURLY> Statement() <SEMICOL>) Block() //  to disambiguate the set literals
+    | LOOKAHEAD(<LCURLY> StatementLookahead()) Block() // to disambiguate the set literals
+    | LOOKAHEAD(<LCURLY> Expression() <SEMICOL>) Block() //  to disambiguate the set literals
     | IfStatement()
     | ForeachStatement()
     | WhileStatement()
@@ -847,6 +852,8 @@ void IdentifierAccess() #void :
     <DOT> (
         t=<DOT_IDENTIFIER> { jjtThis.setIdentifier(t.image); } #IdentifierAccess
     |
+        <VAR> { jjtThis.setIdentifier("var"); } #IdentifierAccess
+    |
         t=<STRING_LITERAL> { jjtThis.setIdentifier(Parser.buildString(t.image, true)); } #IdentifierAccess
     |
         t=<JXLT_LITERAL> { jjtThis.setIdentifier(Parser.buildString(t.image, true)); } #IdentifierAccessJxlt
@@ -855,6 +862,8 @@ void IdentifierAccess() #void :
     <QDOT> (
         t=<DOT_IDENTIFIER> { jjtThis.setIdentifier(t.image); } #IdentifierAccessSafe
     |
+        <VAR> { jjtThis.setIdentifier("var"); } #IdentifierAccessSafe
+    |
         t=<STRING_LITERAL> { jjtThis.setIdentifier(Parser.buildString(t.image, true)); } #IdentifierAccessSafe
     |
         t=<JXLT_LITERAL> { jjtThis.setIdentifier(Parser.buildString(t.image, true)); } #IdentifierAccessSafeJxlt
diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml
index 29416bd..1bf8259 100644
--- a/src/site/xdoc/changes.xml
+++ b/src/site/xdoc/changes.xml
@@ -26,6 +26,18 @@
     </properties>
     <body>
         <release version="3.2" date="unreleased">
+            <action dev="henrib" type="fix" issue="JEXL-305 due-to="Dmitri Blinov">
+                Script debugger produces incorrect syntax
+            </action>
+            <action dev="henrib" type="fix" issue="JEXL-304" due-to="Marcus Warm">
+                Error parsing overview.limit.var
+            </action>
+            <action dev="henrib" type="fix" issue="JEXL-303" due-to="Dmitri Blinov">
+                Block syntax is broken
+            </action>
+            <action dev="henrib" type="fix" issue="JEXL-301" due-to="Dmitri Blinov">
+                Array access operator does not fail on null object in non-strict arithmetic mode
+            </action>
             <action dev="henrib" type="fix" issue="JEXL-296" due-to="Dmitri Blinov">
                 Ant-ish variables should not use safe-access operator syntax
             </action>
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index 554c3c3..4d19c4c 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -16,15 +16,17 @@
  */
 package org.apache.commons.jexl3;
 
+import java.util.HashMap;
 import org.junit.Assert;
-import org.junit.Ignore;
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
 
 /**
  * Test cases for reported issue between JEXL-300 and JEXL-399.
  */
 public class Issues300Test {
-    @Ignore
-    public void testNullArrayAccess301a() throws Exception {
+    @Test
+    public void testIssue301a() throws Exception {
         JexlEngine jexl = new JexlBuilder().safe(false).arithmetic(new JexlArithmetic(false)).create();
         String[] srcs = new String[]{
             "var x = null; x.0", "var x = null; x[0]", "var x = [null,1]; x[0][0]"
@@ -43,8 +45,8 @@ public class Issues300Test {
         }
     }
 
-    @Ignore
-    public void testNullArrayAccess301b() throws Exception {
+    @Test
+    public void testIssues301b() throws Exception {
         JexlEngine jexl = new JexlBuilder().safe(false).arithmetic(new JexlArithmetic(false)).create();
         Object[] xs = new Object[]{null, null, new Object[]{null, 1}};
         String[] srcs = new String[]{
@@ -63,4 +65,57 @@ public class Issues300Test {
             }
         }
     }
+ 
+     @Test
+    public void testIssue302() throws Exception {
+        JexlContext jc = new MapContext();
+        String[] strs = new String[]{
+            "{if (0) 1 else 2; var x = 4;}",
+            "if (0) 1; else 2; ",
+            "{ if (0) 1; else 2; }"
+        };
+        JexlEngine jexl = new JexlBuilder().create();
+        for(String str : strs) {
+        JexlScript e = jexl.createScript(str);
+        Object o = e.execute(jc);
+        int oo = ((Number) o).intValue() % 2;
+        Assert.assertEquals("Block result is wrong " + str, 0, oo);
+        }
+    }  
+    
+    @Test
+    public void testIssue304() {
+        JexlEngine jexlEngine = new JexlBuilder().strict(false).create();
+        JexlExpression jexlExpresssion = jexlEngine.createExpression("overview.limit.var");
+
+        HashMap<String,Object> map3 = new HashMap<String,Object>();
+        map3.put("var", "4711");
+        HashMap<String,Object> map2 = new HashMap<String,Object>();
+        map2.put("limit", map3);
+        HashMap<String,Object> map = new HashMap<String,Object>();
+        map.put("overview", map2);
+
+        JexlContext context = new MapContext(map);
+        Object value = jexlExpresssion.evaluate(context);
+        assertEquals("4711", value); // fails
+        
+        map.clear();
+        map.put("overview.limit.var", 42);
+        value = jexlExpresssion.evaluate(context);
+        assertEquals(42, value); // fails
+        
+    }
+    
+    @Test
+    public void testIssue305() throws Exception {
+        JexlEngine jexl = new JexlBuilder().create();
+        JexlScript e;
+        e = jexl.createScript("{while(false) {}; var x = 1;}");
+        String str0 = e.getParsedText();
+        e =  jexl.createScript(str0);
+        Assert.assertNotNull(e);
+        String str1 = e.getParsedText();
+        Assert.assertEquals(str0, str1);
+    }
+
 }
diff --git a/src/test/java/org/apache/commons/jexl3/ScriptTest.java b/src/test/java/org/apache/commons/jexl3/ScriptTest.java
index 0921c16..b643ca1 100644
--- a/src/test/java/org/apache/commons/jexl3/ScriptTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ScriptTest.java
@@ -19,6 +19,7 @@ package org.apache.commons.jexl3;
 import java.io.File;
 import java.net.URL;
 import org.junit.Assert;
+import org.junit.Ignore;
 import org.junit.Test;
 
 /**
@@ -77,6 +78,18 @@ public class ScriptTest extends JexlTestCase {
         Assert.assertEquals("Wrong result", new Integer(7), result);
     }
 
+    @Ignore
+    public void testScriptFromFile2() throws Exception {
+        File testScript = new File("/Users/henri.biestro/Downloads/sts8689.jexl");
+//        String testScript = "cube(()->{ while (true) { if (true) { if (true) {"
+//                + "montantTTL_7_tempo +=  montantTVANonDeductibleParMois) * exchangeRate_entite / exchangeRate_contrat;\n"
+//                + "}}}})";
+
+        JexlScript s = JEXL.createScript(testScript);
+        Assert.assertNotNull("No result", s);
+    }
+
+    
     @Test
     public void testArgScriptFromFile() throws Exception {
         File testScript = new File(TEST_ADD);
diff --git a/src/test/java/org/apache/commons/jexl3/VarTest.java b/src/test/java/org/apache/commons/jexl3/VarTest.java
index affe33f..37b9955 100644
--- a/src/test/java/org/apache/commons/jexl3/VarTest.java
+++ b/src/test/java/org/apache/commons/jexl3/VarTest.java
@@ -278,6 +278,16 @@ public class VarTest extends JexlTestCase {
         vars = e.getVariables();
         expect = mkref(new String[][]{{"A"}});
         Assert.assertTrue(eq(expect, vars));
+        
+        e = JEXL.createScript("a[b]['c']");
+        vars = e.getVariables();
+        expect = mkref(new String[][]{{"a"}, {"b"}});
+        Assert.assertTrue(eq(expect, vars));
+        
+        e = JEXL.createScript("a['b'][c]");
+        vars = e.getVariables();
+        expect = mkref(new String[][]{{"a", "b"}, {"c"}});
+        Assert.assertTrue(eq(expect, vars));
     }
 
     @Test
diff --git a/src/test/java/org/apache/commons/jexl3/parser/ParserTest.java b/src/test/java/org/apache/commons/jexl3/parser/ParserTest.java
index 0c66c16..a15b01f 100644
--- a/src/test/java/org/apache/commons/jexl3/parser/ParserTest.java
+++ b/src/test/java/org/apache/commons/jexl3/parser/ParserTest.java
@@ -17,11 +17,9 @@
 package org.apache.commons.jexl3.parser;
 
 import java.io.StringReader;
-import org.apache.commons.jexl3.JexlEngine;
 
 import org.apache.commons.jexl3.JexlException;
 import org.apache.commons.jexl3.JexlFeatures;
-import org.apache.commons.logging.LogFactory;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -30,7 +28,7 @@ import org.junit.Test;
  *
  */
 public class ParserTest {
-    static final JexlFeatures features = new JexlFeatures();
+    static final JexlFeatures FEATURES = new JexlFeatures();
     public ParserTest() {}
 
     /**
@@ -40,13 +38,13 @@ public class ParserTest {
     public void testParse() throws Exception {
         Parser parser = new Parser(new StringReader(";"));
         JexlNode sn;
-        sn = parser.parse(null, features, "foo = 1;", null);
+        sn = parser.parse(null, FEATURES, "foo = 1;", null);
         Assert.assertNotNull("parsed node is null", sn);
 
-        sn = parser.parse(null, features, "foo = \"bar\";", null);
+        sn = parser.parse(null, FEATURES, "foo = \"bar\";", null);
         Assert.assertNotNull("parsed node is null", sn);
 
-        sn = parser.parse(null, features, "foo = 'bar';", null);
+        sn = parser.parse(null, FEATURES, "foo = 'bar';", null);
         Assert.assertNotNull("parsed node is null", sn);
     }
 
@@ -56,7 +54,7 @@ public class ParserTest {
         for(String op : ops) {
             Parser parser = new Parser(new StringReader(";"));
             try {
-                JexlNode sn = parser.parse(null, features, "foo() "+op+" 1;", null);
+                JexlNode sn = parser.parse(null, FEATURES, "foo() "+op+" 1;", null);
                 Assert.fail("should have failed on invalid assignment " + op);
             } catch (JexlException.Parsing xparse) {
                 // ok
@@ -70,7 +68,7 @@ public class ParserTest {
     public void testErrorAmbiguous() throws Exception {
         Parser parser = new Parser(new StringReader(";"));
         try {
-            JexlNode sn = parser.parse(null, features, "x = 1 y = 5", null);
+            JexlNode sn = parser.parse(null, FEATURES, "x = 1 y = 5", null);
             Assert.fail("should have failed on ambiguous statement");
         } catch (JexlException.Ambiguous xambiguous) {
             // ok