diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 4d11ff595..d1d73bb2f 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -35,6 +35,7 @@ New Features in 3.3.1: Bugs Fixed in 3.3.1: =================== +* JEXL-403: Exception while evaluating template literal used in array assignment in loop. * JEXL-402: parse failed with empty return value. ======================================================================================================================== diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 8235f55b1..9caafabda 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -36,7 +36,10 @@ Allow 'trailing commas' or ellipsis while defining array, map and set literals - + + Exception while evaluating template literal used in array assignment in loop. + + Parse failed with empty return value. 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 5d3c0ce0c..057218312 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -625,7 +625,7 @@ private Object forIterator(final ASTForeachStatement node, final Object data) { final ASTReference loopReference = (ASTReference) node.jjtGetChild(0); final ASTIdentifier loopVariable = (ASTIdentifier) loopReference.jjtGetChild(0); final int symbol = loopVariable.getSymbol(); - final boolean lexical = loopVariable.isLexical() || options.isLexical() ;// && node.getSymbolCount() > 0; + final boolean lexical = loopVariable.isLexical() || options.isLexical(); final LexicalFrame locals = lexical? new LexicalFrame(frame, block) : null; final boolean loopSymbol = symbol >= 0 && loopVariable instanceof ASTVar; if (lexical) { @@ -1065,9 +1065,9 @@ protected Object visit(final ASTJexlScript script, final Object data) { // if the function is named, assign in the local frame final JexlNode child0 = script.jjtGetChild(0); if (child0 instanceof ASTVar) { - final ASTVar var = (ASTVar) child0; - this.visit(var, data); - final int symbol = var.getSymbol(); + final ASTVar variable = (ASTVar) child0; + this.visit(variable, data); + final int symbol = variable.getSymbol(); frame.set(symbol, closure); // make the closure accessible to itself, ie capture the currently set variable after frame creation closure.setCaptured(symbol, closure); @@ -1390,24 +1390,24 @@ protected Object executeAssign(final JexlNode node, final JexlOperator assignop, cancelCheck(node); // left contains the reference to assign to final JexlNode left = node.jjtGetChild(0); - final ASTIdentifier var; + final ASTIdentifier variable; Object object = null; final int symbol; // check var decl with assign is ok if (left instanceof ASTIdentifier) { - var = (ASTIdentifier) left; - symbol = var.getSymbol(); - if (symbol >= 0 && (var.isLexical() || options.isLexical())) { - if (var instanceof ASTVar) { - if (!defineVariable((ASTVar) var, block)) { - return redefinedVariable(var, var.getName()); + variable = (ASTIdentifier) left; + symbol = variable.getSymbol(); + if (symbol >= 0 && (variable.isLexical() || options.isLexical())) { + if (variable instanceof ASTVar) { + if (!defineVariable((ASTVar) variable, block)) { + return redefinedVariable(variable, variable.getName()); } - } else if (var.isShaded() && (var.isLexical() || options.isLexicalShade())) { - return undefinedVariable(var, var.getName()); + } else if (variable.isShaded() && (variable.isLexical() || options.isLexicalShade())) { + return undefinedVariable(variable, variable.getName()); } } } else { - var = null; + variable = null; symbol = -1; } boolean antish = options.isAntish(); @@ -1418,7 +1418,7 @@ protected Object executeAssign(final JexlNode node, final JexlOperator assignop, // actual value to return, right in most cases Object actual = right; // a (var?) v = ... expression - if (var != null) { + if (variable != null) { if (symbol >= 0) { // check we are not assigning a symbol itself if (last < 0) { @@ -1430,18 +1430,18 @@ protected Object executeAssign(final JexlNode node, final JexlOperator assignop, frame.set(symbol, right); } else { // go through potential overload - final Object self = getVariable(frame, block, var); + final Object self = getVariable(frame, block, variable); final Consumer f = r -> frame.set(symbol, r); actual = operators.tryAssignOverload(node, assignop, f, self, right); } return actual; // 1 } - object = getVariable(frame, block, var); + object = getVariable(frame, block, variable); // top level is a symbol, can not be an antish var antish = false; } else { // check we are not assigning direct global - final String name = var.getName(); + final String name = variable.getName(); if (last < 0) { if (assignop == null) { setContextVariable(node, name, right); @@ -1894,15 +1894,18 @@ protected Object visit(final ASTConstructorNode node, final Object data) { @Override protected Object visit(final ASTJxltLiteral node, final Object data) { - TemplateEngine.TemplateExpression tp = (TemplateEngine.TemplateExpression) node.jjtGetValue(); - if (tp == null) { + Object cache = node.getExpression(); + TemplateEngine.TemplateExpression tp; + if (cache instanceof TemplateEngine.TemplateExpression) { + tp = (TemplateEngine.TemplateExpression) cache; + } else { final TemplateEngine jxlt = jexl.jxlt(); JexlInfo info = node.jexlInfo(); if (this.block != null) { info = new JexlNode.Info(node, info); } tp = jxlt.parseExpression(info, node.getLiteral(), frame != null ? frame.getScope() : null); - node.jjtSetValue(tp); + node.setExpression(tp); } if (tp != null) { return tp.evaluate(context, frame, options); diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccessJxlt.java b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccessJxlt.java index 73af1772f..c995aadeb 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccessJxlt.java +++ b/src/main/java/org/apache/commons/jexl3/parser/ASTIdentifierAccessJxlt.java @@ -23,7 +23,7 @@ * x.`expr`. */ public class ASTIdentifierAccessJxlt extends ASTIdentifierAccess { - protected JxltEngine.Expression jxltExpr; + protected transient JxltEngine.Expression jxltExpression; ASTIdentifierAccessJxlt(final int id) { super(id); @@ -39,11 +39,11 @@ public boolean isExpression() { } public void setExpression(final JxltEngine.Expression tp) { - jxltExpr = tp; + jxltExpression = tp; } public JxltEngine.Expression getExpression() { - return jxltExpr; + return jxltExpression; } } diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTJxltLiteral.java b/src/main/java/org/apache/commons/jexl3/parser/ASTJxltLiteral.java index 66c6292d1..3ba760179 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/ASTJxltLiteral.java +++ b/src/main/java/org/apache/commons/jexl3/parser/ASTJxltLiteral.java @@ -16,13 +16,15 @@ */ package org.apache.commons.jexl3.parser; +import org.apache.commons.jexl3.JxltEngine; + public final class ASTJxltLiteral extends JexlNode { - /** - * - */ + /** serial uid.*/ private static final long serialVersionUID = 1L; - /** The actual literal value; the inherited 'value' member may host a cached template expression. */ + /** The actual literal value. */ private String literal = null; + /** The expression (parsed). */ + private transient JxltEngine.Expression jxltExpression = null; ASTJxltLiteral(final int id) { super(id); @@ -36,6 +38,14 @@ void setLiteral(final String literal) { this.literal = literal; } + public void setExpression(JxltEngine.Expression e) { + this.jxltExpression = e; + } + + public JxltEngine.Expression getExpression() { + return jxltExpression; + } + /** * Gets the literal value. * @return the string literal diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java b/src/test/java/org/apache/commons/jexl3/Issues400Test.java index 4ddc2eac8..ea9d30cff 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java @@ -64,22 +64,28 @@ public void test402() { @Test public void test403() { - String src = "var a = {'a': 1};\n" + - "var list = [a, a];\n" + - "let map1 = {:};\n" + - "for (var item : list) {\n" + - " map1[`${item.a}`] = 1;\n" + - "}\n " + - "map1"; - final JexlEngine jexl = new JexlBuilder().create(); - JexlScript script = jexl.createScript(src); - for(int i = 0; i < 2; ++ i) { - Object result = script.execute(null); - Assert.assertTrue(result instanceof Map); - Map map = (Map) result; - Assert.assertEquals(1, map.size()); - Assert.assertTrue(map.containsKey(1)); - Assert.assertTrue(map.containsValue(1)); + for(String setmap : new String[]{ + " map1.`${item.a}` = 1;\n", + " map1[`${item.a}`] = 1;\n", + " map1[item.a] = 1;\n" + }) { + String src = "var a = {'a': 1};\n" + + "var list = [a, a];\n" + + "let map1 = {:};\n" + + "for (var item : list) {\n" + + setmap + + "}\n " + + "map1"; + final JexlEngine jexl = new JexlBuilder().cache(64).create(); + JexlScript script = jexl.createScript(src); + for (int i = 0; i < 2; ++i) { + Object result = script.execute(null); + Assert.assertTrue(result instanceof Map); + Map map = (Map) result; + Assert.assertEquals(1, map.size()); + Assert.assertTrue(map.containsKey(1)); + Assert.assertTrue(map.containsValue(1)); + } } } }