Skip to content
Permalink
Browse files
JEXL-359: subtle refactor to allow implementing a 3.2-like interpreter;
  • Loading branch information
henrib committed Feb 18, 2022
1 parent 73bb932 commit 3efa7e58cf90a0a8583fd6d1bcdabda7326d5d35
Showing 5 changed files with 78 additions and 40 deletions.
@@ -1,6 +1,6 @@

Apache Commons JEXL
Version 3.2.1
Version 3.3
Release Notes


@@ -1229,7 +1229,7 @@ protected Object visit(final ASTReference node, final Object data) {
final String aname = ant != null ? ant.toString() : "?";
final boolean defined = isVariableDefined(frame, block, aname);
// defined but null; arg of a strict operator?
if (defined && !node.jjtGetParent().isStrictOperator(arithmetic)) {
if (defined && !isStrictOperand(node)) {
return null;
}
return unsolvableVariable(node, aname, !defined);
@@ -38,7 +38,9 @@
import org.apache.commons.jexl3.parser.ASTIdentifier;
import org.apache.commons.jexl3.parser.ASTIdentifierAccess;
import org.apache.commons.jexl3.parser.ASTMethodNode;
import org.apache.commons.jexl3.parser.ASTNullpNode;
import org.apache.commons.jexl3.parser.ASTReference;
import org.apache.commons.jexl3.parser.ASTTernaryNode;
import org.apache.commons.jexl3.parser.ASTVar;
import org.apache.commons.jexl3.parser.JexlNode;
import org.apache.commons.jexl3.parser.ParserVisitor;
@@ -152,6 +154,14 @@ protected void closeIfSupported(final Object closeable) {
}
}

/**
* @param node the operand node
* @return true if this node is an operand of a strict operator, false otherwise
*/
protected boolean isStrictOperand(JexlNode node) {
return node.jjtGetParent().isStrictOperator(arithmetic);
}

/**
* Resolves a namespace, eventually allocating an instance using context as constructor argument.
* <p>
@@ -308,8 +318,8 @@ protected Object getVariable(final Frame frame, final LexicalScope block, final
final Object value = frame.get(symbol);
// not out of scope with no lexical shade ?
if (value != Scope.UNDEFINED) {
// null argument of an arithmetic operator ?
if (value == null && identifier.jjtGetParent().isStrictOperator(arithmetic)) {
// null operand of an arithmetic operator ?
if (value == null && isStrictOperand(identifier)) {
return unsolvableVariable(identifier, name, false); // defined but null
}
return value;
@@ -328,7 +338,7 @@ protected Object getVariable(final Frame frame, final LexicalScope block, final
if (!ignore) {
return undefinedVariable(identifier, name); // undefined
}
} else if (identifier.jjtGetParent().isStrictOperator(arithmetic)) {
} else if (isStrictOperand(identifier)) {
return unsolvableVariable(identifier, name, false); // defined but null
}
}
@@ -438,6 +448,30 @@ protected Object redefinedVariable(final JexlNode node, final String var) {
return variableError(node, var, VariableIssue.REDEFINED);
}

/**
* 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 explicitly 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
*/
protected boolean isTernaryProtected(JexlNode node) {
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;
}

/**
* Triggered when a variable generates an issue.
* @param node the node where the error originated from
@@ -446,7 +480,7 @@ protected Object redefinedVariable(final JexlNode node, final String var) {
* @return throws JexlException if strict and not silent, null otherwise
*/
protected Object variableError(final JexlNode node, final String var, final VariableIssue issue) {
if (isStrictEngine() && !node.isTernaryProtected()) {
if (isStrictEngine() && !isTernaryProtected(node)) {
throw new JexlException.Variable(node, var, issue);
}
if (logger.isDebugEnabled()) {
@@ -490,7 +524,7 @@ protected Object unsolvableMethod(final JexlNode node, final String method, fina
* @return throws JexlException if strict and not silent, null otherwise
*/
protected Object unsolvableProperty(final JexlNode node, final String property, final boolean undef, final Throwable cause) {
if (isStrictEngine() && !node.isTernaryProtected()) {
if (isStrictEngine() && !isTernaryProtected(node)) {
throw new JexlException.Property(node, property, undef, cause);
}
if (logger.isDebugEnabled()) {
@@ -202,14 +202,6 @@ public boolean isGlobalVar() {
return false;
}

/**
* Whether this node is a local variable.
* @return true if local, false otherwise
*/
public boolean isLocalVar() {
return this instanceof ASTIdentifier && ((ASTIdentifier) this).getSymbol() >= 0;
}

/**
* Whether this node is the left-hand side of a safe access identifier as in.
* For instance, in 'x?.y' , 'x' is safe.
@@ -259,31 +251,6 @@ public boolean isSafeLhs(final boolean safe) {
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 explicitly 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;
}

/**
* An info bound to its node.
* <p>Used to parse expressions for templates.
@@ -317,4 +317,41 @@ public void testParsePermissionsFailures() {
}
}
}

protected static class Foo2 {
protected String protectedMethod() {
return "foo2";
}
public String publicMethod() {
return "foo2";
}
}

public static class Foo3 extends Foo2 {
@Override public String protectedMethod() {
return "foo3";
}
@Override public String publicMethod() {
return "foo3";
}
}

@Test public void testProtectedOverrideFail() throws Exception {
JexlScript script;
Object r;
Foo2 foo3 = new Foo3();
JexlEngine jexl = new JexlBuilder().safe(false).create();
// call public override, ok
script = jexl.createScript("x.publicMethod()", "x");
r = script.execute(null, foo3);
Assert.assertEquals("foo3",r);
// call public override of protected, nok
script = jexl.createScript("x.protectedMethod()", "x");
try {
r = script.execute(null, foo3);
Assert.fail("protectedMethod() is not public through superclass Foo2");
} catch(JexlException xjexl) {
Assert.assertNotNull(xjexl);
}
}
}

0 comments on commit 3efa7e5

Please sign in to comment.