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 a10776a JEXL-306: simplify ternary protection handling, factorize
behavior
a10776a is described below
commit a10776a6afd3a9f2c07069cf56183a78e0a4ed76
Author: henrib <[email protected]>
AuthorDate: Tue Jun 11 14:49:28 2019 +0200
JEXL-306: simplify ternary protection handling, factorize behavior
---
.../org/apache/commons/jexl3/JexlException.java | 2 +-
.../apache/commons/jexl3/internal/Interpreter.java | 31 +++-------------------
.../commons/jexl3/internal/InterpreterBase.java | 4 +--
.../org/apache/commons/jexl3/parser/JexlNode.java | 24 +++++++++++++++++
.../org/apache/commons/jexl3/Issues300Test.java | 10 +++++++
5 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlException.java
b/src/main/java/org/apache/commons/jexl3/JexlException.java
index 2662d22..cccccbe 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlException.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlException.java
@@ -123,7 +123,7 @@ public class JexlException extends RuntimeException {
* @param info the information
* @return the information or null
*/
- public static JexlInfo getInfo(JexlNode node, JexlInfo info) {
+ private static JexlInfo getInfo(JexlNode node, JexlInfo info) {
if (info != null && node != null) {
final Debugger dbg = new Debugger();
if (dbg.debug(node)) {
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 6f6fe3c..9ef7b4a 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java
@@ -864,18 +864,7 @@ public class Interpreter extends InterpreterBase {
@Override
protected Object visit(ASTTernaryNode node, Object data) {
- Object condition = null;
- try {
- condition = node.jjtGetChild(0).jjtAccept(this, data);
- } catch(JexlException.Property xprop) {
- if (logger.isDebugEnabled()) {
- logger.debug(node, xprop);
- }
- } catch(JexlException.Variable xvar) {
- if (logger.isDebugEnabled()) {
- logger.debug(node, xvar);
- }
- }
+ Object condition = node.jjtGetChild(0).jjtAccept(this, data);
// ternary as in "x ? y : z"
if (node.jjtGetNumChildren() == 3) {
if (condition != null && arithmetic.toBoolean(condition)) {
@@ -894,18 +883,7 @@ public class Interpreter extends InterpreterBase {
@Override
protected Object visit(ASTNullpNode node, Object data) {
- Object lhs = null;
- try {
- lhs = node.jjtGetChild(0).jjtAccept(this, data);
- } catch(JexlException.Property xprop) {
- if (logger.isDebugEnabled()) {
- logger.debug(node, xprop);
- }
- } catch(JexlException.Variable xvar) {
- if (logger.isDebugEnabled()) {
- logger.debug(node, xvar);
- }
- }
+ Object lhs = node.jjtGetChild(0).jjtAccept(this, data);
// null elision as in "x ?? z"
return lhs != null? lhs : node.jjtGetChild(1).jjtAccept(this, data);
}
@@ -976,8 +954,7 @@ public class Interpreter extends InterpreterBase {
Object value = context.get(name);
if (value == null
&& !(node.jjtGetParent() instanceof ASTReference)
- && !(context.has(name))
- /*&& !node.isTernaryProtected()*/) {
+ && !(context.has(name))) {
return jexl.safe
? null
: unsolvableVariable(node, name, !(node.getSymbol() >=
0 || context.has(name)));
@@ -1144,7 +1121,7 @@ public class Interpreter extends InterpreterBase {
}
}
// am I the left-hand side of a safe op ?
- if (object == null/* && !node.isTernaryProtected()*/) {
+ if (object == null) {
if (ptyNode != null) {
if (ptyNode.isSafeLhs(jexl.safe)) {
return null;
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 e24a95e..399ee3a 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()) {
+ if (isStrictEngine() && !node.isTernaryProtected()) {
throw new JexlException.Variable(node, var, undef);
} else if (logger.isDebugEnabled()) {
logger.debug(JexlException.variableError(node, var, undef));
@@ -351,7 +351,7 @@ public abstract class InterpreterBase extends ParserVisitor
{
* @return throws JexlException if strict and not silent, null otherwise
*/
protected Object unsolvableProperty(JexlNode node, String property,
boolean undef, Throwable cause) {
- if (isStrictEngine()) {
+ if (isStrictEngine() && !node.isTernaryProtected()) {
throw new JexlException.Property(node, property, undef, cause);
} else if (logger.isDebugEnabled()) {
logger.debug(JexlException.propertyError(node, property, undef));
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 0862965..64efd98 100644
--- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
+++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java
@@ -248,4 +248,28 @@ public abstract class JexlNode extends SimpleNode {
}
return false;
}
+
+ /**
+ * Check if a null evaluated expression is protected by a ternary
expression.
+ * <p>
+ * The rationale is that the ternary / elvis expressions are meant for the
user to explictly take control
+ * over the error generation; ie, ternaries can return null even if the
engine in strict mode
+ * would normally throw an exception.
+ * </p>
+ * @return true if nullable variable, false otherwise
+ */
+ public boolean isTernaryProtected() {
+ 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) {
+ return node == walk.jjtGetChild(0);
+ }
+ if (!(walk instanceof ASTReference || walk instanceof
ASTArrayAccess)) {
+ break;
+ }
+ node = walk;
+ }
+ return false;
+ }
}
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index 44ac793..d91ea0c 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -165,4 +165,14 @@ public class Issues300Test {
o = e.execute(null);
Assert.assertEquals(2, o);
}
+
+ @Test
+ public void testIssue306d() throws Exception {
+ JexlEngine jexl = new JexlBuilder().safe(true).create();
+ JexlScript e = jexl.createScript("x.y[z.t] ?: 2", "x");
+ Object o = e.execute(null, new Object());
+ Assert.assertEquals(2, o);
+ o = e.execute(null);
+ Assert.assertEquals(2, o);
+ }
}
