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 23c4577 JEXL-279: refined logic to discriminate undefined vs null
variables or properties
23c4577 is described below
commit 23c45771d63da0746d86c83d20b8b2f4019d4598
Author: henrib <[email protected]>
AuthorDate: Wed Dec 26 18:09:10 2018 +0100
JEXL-279: refined logic to discriminate undefined vs null variables or
properties
---
.../apache/commons/jexl3/internal/Debugger.java | 31 ++++++++
.../apache/commons/jexl3/internal/Interpreter.java | 89 +++++++++++++---------
.../commons/jexl3/internal/InterpreterBase.java | 42 +++++-----
.../org/apache/commons/jexl3/ExceptionTest.java | 8 +-
.../org/apache/commons/jexl3/Issues200Test.java | 85 ++++++++++++++++-----
.../java/org/apache/commons/jexl3/IssuesTest.java | 4 +-
6 files changed, 176 insertions(+), 83 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
index cfc4dbc..59a9743 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java
@@ -123,6 +123,8 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
protected int indentLevel = 0;
/** Perform indentation?. */
protected int indent = 2;
+ /** accept() relative depth. */
+ protected int depth = Integer.MAX_VALUE;
/**
* Creates a Debugger.
@@ -140,6 +142,7 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
end = 0;
indentLevel = 0;
indent = 2;
+ depth = 0;
}
/**
@@ -249,12 +252,32 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
* @param level the number of spaces for indentation, none if less or
equal to zero
*/
public void setIndentation(int level) {
+ indentation(level);
+ }
+
+ /**
+ * Sets the indentation level.
+ * @param level the number of spaces for indentation, none if less or
equal to zero
+ * @return this debugger instance
+ */
+ public Debugger indentation(int level) {
if (level <= 0) {
indent = 0;
} else {
indent = level;
}
indentLevel = 0;
+ return this;
+ }
+
+ /**
+ * Sets this debugger relative maximum depth.
+ * @param rdepth the maximum relative depth from the debugged node
+ * @return this debugger instance
+ */
+ public Debugger depth(int rdepth) {
+ this.depth = rdepth;
+ return this;
}
/**
@@ -264,10 +287,16 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
* @return visitor pattern value
*/
protected Object accept(JexlNode node, Object data) {
+ if (depth <= 0) {
+ builder.append("...");
+ return data;
+ }
if (node == cause) {
start = builder.length();
}
+ depth -= 1;
Object value = node.jjtAccept(this, data);
+ depth += 1;
if (node == cause) {
end = builder.length();
}
@@ -289,7 +318,9 @@ public class Debugger extends ParserVisitor implements
JexlInfo.Detail {
}
}
}
+ depth -= 1;
Object value = accept(child, data);
+ depth += 1;
// blocks, if, for & while dont need a ';' at end
if (!(child instanceof ASTJexlScript
|| child instanceof ASTBlock
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 76d3b68..ac48721 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -1019,7 +1019,7 @@ public class Interpreter extends InterpreterBase {
for (int i = 0; i < numChildren; i++) {
JexlNode nindex = node.jjtGetChild(i);
if (object == null) {
- return null;
+ return unsolvableProperty(nindex, stringifyProperty(nindex),
false, null);
}
Object index = nindex.jjtAccept(this, null);
cancelCheck(node);
@@ -1039,7 +1039,7 @@ public class Interpreter extends InterpreterBase {
&& node.jjtGetChild(which) instanceof ASTIdentifier
&& ((ASTIdentifier) node.jjtGetChild(which)).getSymbol() >= 0);
}
-
+
/**
* Evaluates an access identifier based on the 2 main implementations;
* static (name or numbered identifier) or dynamic (jxlt).
@@ -1082,7 +1082,7 @@ public class Interpreter extends InterpreterBase {
Object id = evalIdentifier(node);
return getAttribute(data, id, node);
}
-
+
@Override
protected Object visit(ASTReference node, Object data) {
cancelCheck(node);
@@ -1090,7 +1090,7 @@ public class Interpreter extends InterpreterBase {
final JexlNode parent = node.jjtGetParent();
// pass first piece of data in and loop through children
Object object = null;
- JexlNode objectNode = null;
+ JexlNode objectNode;
JexlNode ptyNode = null;
StringBuilder ant = null;
boolean antish = !(parent instanceof ASTReference);
@@ -1104,13 +1104,17 @@ public class Interpreter extends InterpreterBase {
if (ant != null) {
JexlNode child = objectNode.jjtGetChild(0);
if (child instanceof ASTIdentifierAccess) {
+ int alen = ant.length();
ant.append('.');
ant.append(((ASTIdentifierAccess)
child).getName());
object = context.get(ant.toString());
if (object != null) {
object = visit((ASTMethodNode) objectNode,
object, context);
+ continue;
+ } else {
+ // remove method name from antish
+ ant.delete(alen, ant.length());
}
- continue;
}
}
break;
@@ -1119,7 +1123,6 @@ public class Interpreter extends InterpreterBase {
}
} else if (objectNode instanceof ASTArrayAccess) {
if (object == null) {
- ptyNode = objectNode;
break;
} else {
antish = false;
@@ -1131,55 +1134,65 @@ public class Interpreter extends InterpreterBase {
if (object != null) {
// disallow mixing antish variable & bean with same root;
avoid ambiguity
antish = false;
- } else if (antish) { // if we still have a null object, check for
an antish variable
- if (ant == null) {
- JexlNode first = node.jjtGetChild(0);
- if (first instanceof ASTIdentifier) {
- ASTIdentifier afirst = (ASTIdentifier) first;
- ant = new StringBuilder(afirst.getName());
- } else {
- ptyNode = objectNode;
- break;
+ } 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;
+ }
}
- }
- for (; v <= c; ++v) {
- JexlNode child = node.jjtGetChild(v);
- if (child instanceof ASTIdentifierAccess) {
- ASTIdentifierAccess achild = (ASTIdentifierAccess)
child;
- if (achild.isSafe() || achild.isExpression()) {
- break main;
+ // catch up
+ 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(((ASTIdentifierAccess)
objectNode).getName());
+ } else {
+ // not an identifier, not antish
+ ptyNode = objectNode;
+ break main;
}
- ant.append('.');
- ant.append(((ASTIdentifierAccess)
objectNode).getName());
- } else {
- break main;
}
+ object = context.get(ant.toString());
}
- object = context.get(ant.toString());
- } else {
- // the last one may be null
- ptyNode = c != numChildren - 1? objectNode : null;
+ } else if (c != numChildren - 1) {
+ // only the last one may be null
+ ptyNode = objectNode;
break; //
}
}
if (object == null && !node.isTernaryProtected()) {
- if (antish && ant != null) {
- boolean undefined = !(context.has(ant.toString()) ||
isLocalVariable(node, 0));
- // variable unknown in context and not a local
- return node.isSafeLhs(jexl.safe)
- ? null
- : unsolvableVariable(node, ant.toString(), undefined);
- }
if (ptyNode != null) {
// am I the left-hand side of a safe op ?
return ptyNode.isSafeLhs(jexl.safe)
? null
: unsolvableProperty(node, stringifyProperty(ptyNode),
false, null);
}
+ if (antish) {
+ String aname = ant != null? ant.toString() :
stringifyProperty(node);
+ boolean undefined = !(context.has(aname) ||
isLocalVariable(node, 0));
+ // variable unknown in context and not a local
+ return node.isSafeLhs(jexl.safe)
+ ? null
+ : unsolvableVariable(node, undefined?
stringifyProperty(node) : aname, undefined);
+ }
}
return object;
}
-
+
@Override
protected Object visit(ASTAssignment node, Object data) {
return executeAssign(node, null, data);
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 e8a0287..b5d44fc 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java
@@ -26,7 +26,9 @@ import org.apache.commons.jexl3.introspection.JexlMethod;
import org.apache.commons.jexl3.introspection.JexlUberspect;
import org.apache.commons.jexl3.parser.ASTArrayAccess;
import org.apache.commons.jexl3.parser.ASTFunctionNode;
+import org.apache.commons.jexl3.parser.ASTIdentifier;
import org.apache.commons.jexl3.parser.ASTMethodNode;
+import org.apache.commons.jexl3.parser.ASTReference;
import org.apache.commons.jexl3.parser.JexlNode;
import org.apache.commons.jexl3.parser.ParserVisitor;
@@ -236,29 +238,33 @@ public abstract class InterpreterBase extends
ParserVisitor {
*/
protected String stringifyProperty(JexlNode node) {
if (node instanceof ASTArrayAccess) {
- if (node.jjtGetChild(0) != null) {
- return "["
- + node.jjtGetChild(0).toString()
- + "]";
- }
- return "[???]";
+ return "["
+ + stringifyPropertyValue(node.jjtGetChild(0))
+ + "]";
}
if (node instanceof ASTMethodNode) {
- if (node.jjtGetChild(0) != null) {
- return "."
- + node.jjtGetChild(0).toString()
- + "(...)";
- }
- return ".???(...)";
+ return stringifyPropertyValue(node.jjtGetChild(0));
}
if (node instanceof ASTFunctionNode) {
- if (node.jjtGetChild(0) != null) {
- return node.jjtGetChild(0).toString()
- + "(...)";
- }
- return "???(...)";
+ return stringifyPropertyValue(node.jjtGetChild(0));
+ }
+ if (node instanceof ASTIdentifier) {
+ return ((ASTIdentifier) node).getName();
+ }
+ if (node instanceof ASTReference) {
+ return stringifyPropertyValue(node.jjtGetChild(0));
}
- return node.toString();
+ return stringifyPropertyValue(node);
+ }
+
+ /**
+ * Pretty-prints a failing property value (de)reference.
+ * <p>Used by calls to unsolvableProperty(...).</p>
+ * @param node the property node
+ * @return the (pretty) string value
+ */
+ protected static String stringifyPropertyValue(JexlNode node) {
+ return node != null? new Debugger().depth(1).data(node) : "???";
}
/**
diff --git a/src/test/java/org/apache/commons/jexl3/ExceptionTest.java
b/src/test/java/org/apache/commons/jexl3/ExceptionTest.java
index b565eff..fc1b555 100644
--- a/src/test/java/org/apache/commons/jexl3/ExceptionTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ExceptionTest.java
@@ -132,10 +132,10 @@ public class ExceptionTest extends JexlTestCase {
// empty cotext
try {
/* Object o = */ e.evaluate(ctxt);
- Assert.fail("c.e not defined as variable should throw");
+ Assert.fail("c not defined as variable should throw");
} catch (JexlException.Variable xjexl) {
String msg = xjexl.getMessage();
- Assert.assertTrue(msg.indexOf("c.e") > 0);
+ Assert.assertTrue(msg.indexOf("c") > 0);
}
// disallow null operands
@@ -212,10 +212,10 @@ public class ExceptionTest extends JexlTestCase {
// empty cotext
try {
/* Object o = */ e.evaluate(ctxt);
- Assert.fail("c.e not declared as variable should throw");
+ Assert.fail("c not declared as variable should throw");
} catch (JexlException.Variable xjexl) {
String msg = xjexl.getMessage();
- Assert.assertTrue(msg.indexOf("c.e") > 0);
+ Assert.assertTrue(msg.indexOf("c") > 0);
}
// disallow null operands
diff --git a/src/test/java/org/apache/commons/jexl3/Issues200Test.java
b/src/test/java/org/apache/commons/jexl3/Issues200Test.java
index 8d520aa..27a9ff3 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues200Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues200Test.java
@@ -32,6 +32,8 @@ import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -631,39 +633,80 @@ public class Issues200Test extends JexlTestCase {
Assert.assertEquals(src, ctls[i], value);
}
}
+
+ public static class Context279 extends MapContext {
+ public String identity(String x) {
+ return x;
+ }
+ public Number identity(Number x) {
+ return x;
+ }
+ }
@Test
public void test279() throws Exception {
+ final Log logger = null;//LogFactory.getLog(Issues200Test.class);
Object result;
JexlScript script;
- JexlContext ctxt = new MapContext();
- ctxt.set("y.z", null);
- ctxt.set("z", null);
+ JexlContext ctxt = new Context279();
String[] srcs = new String[]{
- "var x = null; x[0];",
- "var x = null; x.0;",
+ "var z = null; identity(z[0]);",
+ "var z = null; z.0;",
+ "var z = null; z.foo();",
+ "z['y']['z']",
+ "z.y.any()",
+ "identity(z.any())",
"z[0]",
"z.0",
- "y.z[0]",
- "y.z.0",
+ "z.foo()",
+ "z.y[0]",
+ "z.y[0].foo()",
+ "z.y.0",
+ "z.y.foo()",
+ "var z = { 'y' : [42] }; z.y[1]",
+ "var z = { 'y' : [42] }; z.y.1",
+ "var z = { 'y' : [-42] }; z.y[1].foo()",
+ "var z = { 'y' : [42] }; z.y.1.foo()",
+ "var z = { 'y' : [null, null] }; z.y[1].foo()",
+ "var z = { 'y' : [null, null] }; z.y.1.foo()"
};
- for(boolean strict : new boolean[]{ true, false} ) {
- JexlEngine jexl = new
JexlBuilder().safe(false).strict(strict).create();
- for (String src : srcs) {
- script = jexl.createScript(src);
- try {
- result = script.execute(ctxt);
- if (strict) {
- Assert.fail("should have failed: " + src);
+ for (int i = 0; i < 2; ++i) {
+ for (boolean strict : new boolean[]{true, false}) {
+ JexlEngine jexl = new
JexlBuilder().safe(false).strict(strict).create();
+ for (String src : srcs) {
+ script = jexl.createScript(src);
+ try {
+ result = script.execute(ctxt);
+ if (strict) {
+ if (logger != null) {
+ logger.warn(ctxt.has("z") + ": " + src + ": no
fail, " + result);
+ }
+ Assert.fail("should have failed: " + src);
+ }
+ // not reachable
+ Assert.assertNull("non-null result ?!", result);
+ } catch (JexlException.Variable xvar) {
+ if (logger != null) {
+ logger.warn(ctxt.has("z") + ": " + src + ": fail,
" + xvar);
+ }
+ if (!strict) {
+ Assert.fail(src + ", should not have thrown " +
xvar);
+ } else {
+ Assert.assertTrue(src + ": " + xvar.toString(),
xvar.toString().contains("z"));
+ }
+ } catch (JexlException.Property xprop) {
+ if (logger != null) {
+ logger.warn(ctxt.has("z") + ": " + src + ": fail,
" + xprop);
+ }
+ if (!strict) {
+ Assert.fail(src + ", should not have thrown " +
xprop);
+ } else {
+ Assert.assertTrue(src + ": " + xprop.toString(),
xprop.toString().contains("1"));
+ }
}
- // not reachable
- Assert.assertNull("non-null result ?!", result);
- } catch (JexlException xany) {
- if (!strict) {
- Assert.fail(src + ", should not have thrown " + xany);
- }
}
}
+ ctxt.set("z.y", null);
}
}
}
diff --git a/src/test/java/org/apache/commons/jexl3/IssuesTest.java
b/src/test/java/org/apache/commons/jexl3/IssuesTest.java
index 477afa5..a941fa2 100644
--- a/src/test/java/org/apache/commons/jexl3/IssuesTest.java
+++ b/src/test/java/org/apache/commons/jexl3/IssuesTest.java
@@ -266,10 +266,10 @@ public class IssuesTest extends JexlTestCase {
e = jexl.createExpression("c.e");
try {
/* Object o = */ e.evaluate(ctxt);
- Assert.fail("c.e not declared as variable");
+ Assert.fail("c not declared as variable");
} catch (JexlException.Variable xjexl) {
String msg = xjexl.getMessage();
- Assert.assertTrue(msg.indexOf("c.e") > 0);
+ Assert.assertTrue(msg.indexOf("c") > 0);
}
ctxt.set("c", "{ 'a' : 3, 'b' : 5}");
