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 2022/08/26 17:12:43 UTC

[commons-jexl] branch master updated: JEXL-373: fixing regression in error reporting and lexical frame cleansing;

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 9f0e5c15 JEXL-373: fixing regression in error reporting and lexical frame cleansing;
9f0e5c15 is described below

commit 9f0e5c151183d34c3a1d1d9ffbd41ca4d0726bd4
Author: henrib <he...@apache.org>
AuthorDate: Fri Aug 26 19:12:37 2022 +0200

    JEXL-373: fixing regression in error reporting and lexical frame cleansing;
---
 .../java/org/apache/commons/jexl3/JexlException.java     |  2 +-
 .../org/apache/commons/jexl3/internal/LexicalScope.java  | 16 ++++++++++------
 .../java/org/apache/commons/jexl3/parser/JexlParser.java | 11 ++++++++---
 .../java/org/apache/commons/jexl3/Issues300Test.java     | 14 ++++++++++++++
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/main/java/org/apache/commons/jexl3/JexlException.java b/src/main/java/org/apache/commons/jexl3/JexlException.java
index a683596f..19f73d45 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlException.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlException.java
@@ -446,7 +446,7 @@ public class JexlException extends RuntimeException {
     }
 
     /**
-     * Thrown when parsing fails due to an invalid assigment.
+     * Thrown when parsing fails due to an invalid assignment.
      *
      * @since 3.0
      */
diff --git a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
index 4be5d755..2f983e01 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
@@ -21,9 +21,8 @@ import java.util.BitSet;
 /**
  * The set of symbols declared in a lexical scope.
  * <p>The symbol identifiers are determined by the functional scope.</p>
- * <p>We use 3 bits per symbol; bit 0 sets the actual symbol as lexical (let),
- * bit 1 as a const, bit 2 as a defined (valued) const.
- * There are actually only 4 used states: 0, 1, 3, 7</p>
+ * <p>We use 2 bits per symbol; bit 0 sets the actual symbol as lexical (let), bit 1 as a const.
+ * There are actually only 4 used states: 0, 1, 3</p>
  */
 public class LexicalScope {
     /**
@@ -35,6 +34,11 @@ public class LexicalScope {
      * Declared, const, defined.
      */
     protected static final int BITS_PER_SYMBOL = 2;
+    /**
+     * Bitmask for symbols.
+     * Declared, const, defined.
+     */
+    protected static final long SYMBOL_MASK = (1L << BITS_PER_SYMBOL) - 1; // 3, as 1+2, 2 bits
     /**
      * Number of symbols.
      */
@@ -170,15 +174,15 @@ public class LexicalScope {
             long clean = symbols;
             while (clean != 0L) {
                 final int s = Long.numberOfTrailingZeros(clean);
-                // call clean for symbol definition (7 as a mask for 3 bits,1+2+4)
-                clean &= ~(7L << s);
+                // call clean for symbol definition (3 as a mask for 2 bits,1+2)
+                clean &= ~(SYMBOL_MASK << s);
                 cleanSymbol.accept(s >> BITS_PER_SYMBOL);
             }
         }
         symbols = 0L;
         if (moreSymbols != null) {
             if (cleanSymbol != null) {
-                // step over const and definition (3 bits per symbol)
+                // step by bits per symbol
                 for (int s = moreSymbols.nextSetBit(0); s != -1; s = moreSymbols.nextSetBit(s + BITS_PER_SYMBOL)) {
                     cleanSymbol.accept(s + LONGBITS);
                 }
diff --git a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
index bae8e971..157ae9c2 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlParser.java
@@ -629,14 +629,19 @@ public abstract class JexlParser extends StringParser {
         } else if (ASSIGN_NODES.contains(node.getClass())) {
             final JexlNode lv = node.jjtGetChild(0);
             if (!lv.isLeftValue()) {
-                throw new JexlException.Assignment(lv.jexlInfo(), null).clean();
+                JexlInfo xinfo = lv.jexlInfo();
+                xinfo = info.at(xinfo.getLine(), xinfo.getColumn());
+                final String msg = readSourceLine(source, xinfo.getLine());
+                throw new JexlException.Assignment(xinfo, msg).clean();
             }
             if (lv instanceof ASTIdentifier && !(lv instanceof ASTVar)) {
                 ASTIdentifier var = (ASTIdentifier) lv;
                 int symbol = var.getSymbol();
                 boolean isconst = symbol >= 0 && block != null && block.isConstant(symbol);
-                if (isconst) { // if not a declaration...
-                    throw new JexlException.Parsing(var.jexlInfo(), var.getName() +": const assignment.").clean();
+                if (isconst) { // if constant, fail...
+                    JexlInfo xinfo = lv.jexlInfo();
+                    xinfo = info.at(xinfo.getLine(), xinfo.getColumn());
+                    throw new JexlException.Assignment(xinfo, var.getName()).clean();
                 }
             }
         }
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index 94005566..6477243a 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -897,4 +897,18 @@ public class Issues300Test {
         Assert.assertTrue(result instanceof LinkedHashMap);
         Assert.assertEquals(1, ((Map) result).size());
     }
+
+    @Test
+    public void test373b() {
+        final String src = "var i = ++1";
+        JexlEngine jexl = new JexlBuilder().safe(true).create();
+        JexlInfo info = new JexlInfo("badscript", 0, 0);
+        try {
+            JexlScript script = jexl.createScript(info, src);
+            Assert.fail("should not parse");
+        } catch(JexlException.Parsing xparse) {
+            String msg = xparse.getMessage();
+            Assert.assertTrue(msg.contains("badscript"));
+        }
+    }
 }