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 ef8a3e3 JEXL-323, JEXL-327: fixed error reporting an null checks Task
#JEXL-327 - map[null] does not work in assignment context
ef8a3e3 is described below
commit ef8a3e3435e2948b2afd7972901eac5a45721b19
Author: henrib <[email protected]>
AuthorDate: Wed Jan 29 17:57:01 2020 +0100
JEXL-323, JEXL-327: fixed error reporting an null checks
Task #JEXL-327 - map[null] does not work in assignment context
---
RELEASE-NOTES.txt | 3 +
.../apache/commons/jexl3/internal/Interpreter.java | 29 +++---
.../org/apache/commons/jexl3/parser/JexlNode.java | 5 +-
src/site/xdoc/changes.xml | 9 ++
.../org/apache/commons/jexl3/Issues300Test.java | 112 +++++++++++++++++----
5 files changed, 122 insertions(+), 36 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index c329e77..27f7655 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -74,8 +74,11 @@ New Features in 3.2:
Bugs Fixed in 3.2:
==================
+* JEXL-327: map[null] does not work in assignment context
+* JEXL-326: Link to "JavaCC" on syntax reference page is broken
* JEXL-325: Potential race-condition in NumberParser.toString()
* JEXL-324: JexlEngine.createExpression("new()").getParsedText() throws
NPE
+* JEXL-323: Ant-style variables can throw exception when evaluated for
their value
* JEXL-322: JXLT String literals cannot contain curly brace
* JEXL-321: Empty do-while loop is broken
* JEXL-320: "mvn test" fails with COMPILATION ERROR in
SynchronizedArithmetic.java on Java 11
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 d0dd721..99a81c9 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -1132,6 +1132,7 @@ public class Interpreter extends InterpreterBase {
for (int c = 0; c < numChildren; c++) {
objectNode = node.jjtGetChild(c);
if (objectNode instanceof ASTMethodNode) {
+ antish = false;
if (object == null) {
// we may be performing a method call on an antish var
if (ant != null) {
@@ -1147,19 +1148,17 @@ public class Interpreter extends InterpreterBase {
} else {
// remove method name from antish
ant.delete(alen, ant.length());
+ ptyNode = objectNode;
}
}
}
break;
- } else {
- antish = false;
}
} else if (objectNode instanceof ASTArrayAccess) {
+ antish = false;
if (object == null) {
ptyNode = objectNode;
break;
- } else {
- antish = false;
}
}
// attempt to evaluate the property within the object
(visit(ASTIdentifierAccess node))
@@ -1216,7 +1215,7 @@ public class Interpreter extends InterpreterBase {
break; //
}
}
- // am I the left-hand side of a safe op ?
+ // dealing with null
if (object == null) {
if (ptyNode != null) {
if (ptyNode.isSafeLhs(isSafe())) {
@@ -1224,19 +1223,23 @@ public class Interpreter extends InterpreterBase {
}
if (ant != null) {
String aname = ant.toString();
- boolean undefined = !(context.has(aname) ||
isLocalVariable(node, 0) || isFunctionCall(node));
+ boolean undefined = !(context.has(aname));
return unsolvableVariable(node, aname, undefined);
}
- return unsolvableProperty(node, stringifyProperty(ptyNode),
ptyNode == objectNode, null);
+ return unsolvableProperty(node,
+ stringifyProperty(ptyNode), ptyNode == objectNode,
null);
}
if (antish) {
if (node.isSafeLhs(isSafe())) {
return null;
}
String aname = ant != null ? ant.toString() : "?";
- boolean undefined = !(context.has(aname) ||
isLocalVariable(node, 0) || isFunctionCall(node));
- if (undefined || arithmetic.isStrict()) {
- return unsolvableVariable(node, aname, undefined);
+ boolean defined = context.has(aname);
+ if (defined && !arithmetic.isStrict()) {
+ return null;
+ }
+ if (!defined || !(node.jjtGetParent() instanceof
ASTJexlScript)) {
+ return unsolvableVariable(node, aname, !defined);
}
}
}
@@ -1450,10 +1453,8 @@ public class Interpreter extends InterpreterBase {
} else {
throw new JexlException(objectNode, "illegal assignment form");
}
- if (property == null) {
- // no property, we fail
- return unsolvableProperty(propertyNode, "<?>.<null>", true, null);
- }
+ // we may have a null property as in map[null], no check needed.
+ // we can not *have* a null object though.
if (object == null) {
// no object, we fail
return unsolvableProperty(objectNode, "<null>.<?>", true, null);
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 940f17a..f085182 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
@@ -260,7 +260,10 @@ public abstract class JexlNode extends SimpleNode {
JexlNode node = this;
for (JexlNode walk = node.jjtGetParent(); walk != null; walk =
walk.jjtGetParent()) {
// protect only the condition part of the ternary
- if (walk instanceof ASTTernaryNode || walk instanceof ASTNullpNode
|| walk instanceof ASTEQNode || walk instanceof ASTNENode) {
+ if (walk instanceof ASTTernaryNode
+ || walk instanceof ASTNullpNode
+ || walk instanceof ASTEQNode
+ || walk instanceof ASTNENode) {
return node == walk.jjtGetChild(0);
}
if (!(walk instanceof ASTReference || walk instanceof
ASTArrayAccess)) {
diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml
index 01e461a..8d80239 100644
--- a/src/site/xdoc/changes.xml
+++ b/src/site/xdoc/changes.xml
@@ -26,12 +26,21 @@
</properties>
<body>
<release version="3.2" date="unreleased">
+ <action dev="henrib" type="fix" issue="JEXL-327" due-to="David
Costanzo">
+ map[null] does not work in assignment context
+ </action>
+ <action dev="henrib" type="fix" issue="JEXL-326" due-to="David
Costanzo">
+ Link to "JavaCC" on syntax reference page is broken
+ </action>
<action dev="Dmitri Blinov" type="fix" issue="JEXL-325"
due-to="Dmitri Blinov">
Potential race-condition in NumberParser.toString()
</action>
<action dev="henrib" type="fix" issue="JEXL-324" due-to="David
Costanzo">
JexlEngine.createExpression("new()").getParsedText() throws NPE
</action>
+ <action dev="henrib" type="fix" issue="JEXL-323" due-to="David
Costanzo">
+ Ant-style variables can throw exception when evaluated for
their value
+ </action>
<action dev="henrib" type="fix" issue="JEXL-322"
due-to="Constantin Hirsch">
JXLT String literals cannot contain curly braces
</action>
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index f4c260f..65a4d6a 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -18,6 +18,7 @@ package org.apache.commons.jexl3;
import java.io.StringReader;
import java.io.StringWriter;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.LinkedHashMap;
@@ -25,7 +26,6 @@ import java.util.List;
import java.util.Map;
import org.junit.Assert;
import static org.junit.Assert.assertEquals;
-import org.junit.Ignore;
import org.junit.Test;
/**
@@ -47,7 +47,7 @@ public class Issues300Test {
Assert.fail(src + ": Should have failed");
}
} catch (Exception ex) {
- //
+ Assert.assertTrue(ex.getMessage().contains("x"));
}
}
}
@@ -421,26 +421,68 @@ public class Issues300Test {
Assert.assertEquals("L'utilisateur user322 s'est connecte", output);
}
- @Ignore
+ @Test
public void test323() throws Exception {
- // Create or retrieve an engine
JexlEngine jexl = new JexlBuilder().safe(false).create();
- // on recent code: JexlEngine jexl = new
JexlBuilder().safe(false).create();
-
- // Populate to identical global variables
- JexlContext jc = new MapContext();
- jc.set("NormalVariable", null);
- jc.set("ant.variable", null);
-
- // Evaluate the value of the normal variable
- JexlExpression expression1 = jexl.createExpression("NormalVariable");
- Object o1 = expression1.evaluate(jc);
- Assert.assertEquals(null, o1);
-
- // Evaluate the value of the ant-style variable
- JexlExpression expression2 = jexl.createExpression("ant.variable");
- Object o2 = expression2.evaluate(jc); // <-- BUG: throws exception
instead of returning null
- Assert.assertEquals(null, o2);
+ Map<String,Object> vars = new HashMap<String, Object>();
+ JexlContext jc = new MapContext(vars);
+ JexlScript script;
+ Object result;
+
+ // nothing in context, ex
+ try {
+ script = jexl.createScript("a.n.t.variable");
+ result = script.execute(jc);
+ Assert.fail("a.n.t.variable is undefined!");
+ } catch(JexlException.Variable xvar) {
+ Assert.assertTrue(xvar.toString().contains("a.n.t"));
+ }
+
+ // defined and null
+ jc.set("a.n.t.variable", null);
+ script = jexl.createScript("a.n.t.variable");
+ result = script.execute(jc);
+ Assert.assertEquals(null, result);
+
+ // defined and null, dereference
+ jc.set("a.n.t", null);
+ try {
+ script = jexl.createScript("a.n.t[0].variable");
+ result = script.execute(jc);
+ Assert.fail("a.n.t is null!");
+ } catch(JexlException.Variable xvar) {
+ Assert.assertTrue(xvar.toString().contains("a.n.t"));
+ }
+
+ // undefined, dereference
+ vars.remove("a.n.t");
+ try {
+ script = jexl.createScript("a.n.t[0].variable");
+ result = script.execute(jc);
+ Assert.fail("a.n.t is undefined!");
+ } catch(JexlException.Variable xvar) {
+ Assert.assertTrue(xvar.toString().contains("a.n.t"));
+ }
+ // defined, derefence undefined property
+ List<Object> inner = new ArrayList<Object>();
+ vars.put("a.n.t", inner);
+ try {
+ script = jexl.createScript("a.n.t[0].variable");
+ result = script.execute(jc);
+ Assert.fail("a.n.t is null!");
+ } catch(JexlException.Property xprop) {
+ Assert.assertTrue(xprop.toString().contains("0"));
+ }
+ // defined, derefence undefined property
+ inner.add(42);
+ try {
+ script = jexl.createScript("a.n.t[0].variable");
+ result = script.execute(jc);
+ Assert.fail("a.n.t is null!");
+ } catch(JexlException.Property xprop) {
+ Assert.assertTrue(xprop.toString().contains("variable"));
+ }
+
}
@Test
@@ -458,5 +500,33 @@ public class Issues300Test {
Assert.assertTrue(xparse.toString().contains("new"));
}
}
-
+
+ @Test
+ public void test325() throws Exception {
+ JexlEngine jexl = new JexlBuilder().safe(false).create();
+ Map<String, Object> map = new HashMap<String, Object>() {
+ @Override
+ public Object get(Object key) {
+ return super.get(key == null ? "" : key);
+ }
+
+ @Override
+ public Object put(String key, Object value) {
+ return super.put(key == null ? "" : key, value);
+ }
+ };
+ map.put("42", 42);
+ JexlContext jc = new MapContext();
+ JexlScript script;
+ Object result;
+
+ script = jexl.createScript("map[null] = 42", "map");
+ result = script.execute(jc, map);
+ Assert.assertEquals(42, result);
+ script = jexl.createScript("map[key]", "map", "key");
+ result = script.execute(jc, map, null);
+ Assert.assertEquals(42, result);
+ result = script.execute(jc, map, "42");
+ Assert.assertEquals(42, result);
+ }
}
