This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch JEXL-384 in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit d664f6579c2bb8406c892fe1f4f2d76078e29976 Author: henrib <hen...@apache.org> AuthorDate: Sat Nov 5 18:36:29 2022 +0100 JEXL-384: refine strictness by introducing strict-cast flag in arithmetic only used by cast operators and set as false if strict is false or isStrict method is overridden; - added 'condition' operator to use in if/while/for; - simplified test accordingly; --- .../org/apache/commons/jexl3/JexlArithmetic.java | 92 +++++++++++++++------- .../org/apache/commons/jexl3/JexlOperator.java | 9 ++- .../apache/commons/jexl3/internal/Interpreter.java | 16 ++-- .../org/apache/commons/jexl3/Issues300Test.java | 14 +--- 4 files changed, 86 insertions(+), 45 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java index e813151a..9f6eca41 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java +++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java @@ -22,6 +22,7 @@ import org.apache.commons.jexl3.introspection.JexlMethod; import java.lang.reflect.Array; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.math.BigDecimal; import java.math.BigInteger; import java.math.MathContext; @@ -80,6 +81,9 @@ public class JexlArithmetic { /** Whether this JexlArithmetic instance behaves in strict or lenient mode. */ private final boolean strict; + /** Whether this JexlArithmetic instance allows null as argument to cast methods - toXXX(). */ + private final boolean strictCast; + /** The big decimal math context. */ private final MathContext mathContext; @@ -119,6 +123,17 @@ public class JexlArithmetic { // ignore } this.ctor = actor; + boolean cast = strict; + // if isStrict is not overridden, we are in strict-cast mode + if (cast) { + try { + Method istrict = getClass().getMethod("isStrict", JexlOperator.class); + cast = JexlArithmetic.class == istrict.getDeclaringClass(); + } catch (Exception e) { + // ignore + } + } + this.strictCast = cast; } /** @@ -371,7 +386,17 @@ public class JexlArithmetic { * @return true if strict, false if lenient */ public boolean isStrict() { - return this.strict; + return strict; + } + + /** + * Checks whether this JexlArithmetic instance + * strictly considers null as an error when used as operand of a cast method (toXXX()).. + * + * @return true if strict-cast, false if lenient + */ + public boolean isStrictCast() { + return strictCast; } /** @@ -380,21 +405,32 @@ public class JexlArithmetic { * If null-safe (ie not-strict), the operator does accept null arguments even if the arithmetic itself is strict.</p> * <p>The default implementation considers equal/not-equal operators as null-safe so one can check for null as in * <code>if (myvar == null) {...}</code>. Note that this operator is used for equal and not-equal syntax. The complete - * list of operators that are not strict are (==, [], []=, ., .=). </p> - * <p>An arithmetic refining its strict behavior handling for more operators must declare which by overriding - * this method.</p> + * list of operators that are not strict are (==, [], []=, ., .=, empty, size, contains). </p> + * <p> + * An arithmetic refining its strict behavior handling for more operators must declare which by overriding + * this method. + * </p> + * <p> + * If this method is overridden, the arithmetic instance is <em>NOT</em> in strict-cast mode. Tp restore the + * strict-cast behavior, override the {@link #isStrictCast()} method/ + * </p> * @param operator the operator to check for null-argument(s) handling * @return true if operator considers null arguments as errors, false if operator has appropriate semantics * for null argument(s) */ public boolean isStrict(JexlOperator operator) { - switch(operator) { - case EQ: - case ARRAY_GET: - case ARRAY_SET: - case PROPERTY_GET: - case PROPERTY_SET: - return false; + if (operator != null) { + switch (operator) { + case EQ: + case ARRAY_GET: + case ARRAY_SET: + case PROPERTY_GET: + case PROPERTY_SET: + case EMPTY: + case SIZE: + case CONTAINS: + return false; + } } return isStrict(); } @@ -435,22 +471,25 @@ public class JexlArithmetic { * The result of +,/,-,*,% when both operands are null. * * @return Integer(0) if lenient - * @throws ArithmeticException if strict + * @throws ArithmeticException if strict-cast */ protected Object controlNullNullOperands() { - if (isStrict()) { + if (isStrictCast()) { throw new NullOperand(); } return 0; } /** - * Throw a NPE if arithmetic is strict. + * Throw a NPE if arithmetic is strict-cast. + * <p>This method is called by the cast methods ({@link #toBoolean(Object)}, {@link #toInteger(Object)}, + * {@link #toDouble(Object)}, {@link #toString(Object)}, {@link #toBigInteger(Object)}, {@link #toBigDecimal(Object)}) + * when they encounter a null argument.</p> * * @throws ArithmeticException if strict */ protected void controlNullOperand() { - if (isStrict()) { + if (isStrictCast()) { throw new NullOperand(); } } @@ -556,7 +595,7 @@ public class JexlArithmetic { Number result = original; if (original instanceof BigDecimal) { final BigDecimal bigd = (BigDecimal) original; - // if it's bigger than a double it can't be narrowed + // if it is bigger than a double, it can not be narrowed if (bigd.compareTo(BIGD_DOUBLE_MAX_VALUE) > 0 || bigd.compareTo(BIGD_DOUBLE_MIN_VALUE) < 0) { return original; @@ -587,7 +626,7 @@ public class JexlArithmetic { } else { if (original instanceof BigInteger) { final BigInteger bigi = (BigInteger) original; - // if it's bigger than a Long it can't be narrowed + // if it is bigger than a Long, it can not be narrowed if (bigi.compareTo(BIGI_LONG_MAX_VALUE) > 0 || bigi.compareTo(BIGI_LONG_MIN_VALUE) < 0) { return original; @@ -751,7 +790,6 @@ public class JexlArithmetic { */ protected Object increment(Object val, int incr) { if (val == null) { - controlNullOperand(); return incr; } if (val instanceof Integer) { @@ -803,7 +841,7 @@ public class JexlArithmetic { : left instanceof String && right instanceof String; if (!strconcat) { try { - // if both (non null) args fit as long + // if both (non-null) args fit as long final Number ln = asLongNumber(left); final Number rn = asLongNumber(right); if (ln != null && rn != null) { @@ -835,9 +873,7 @@ public class JexlArithmetic { final BigInteger result = l.add(r); return narrowBigInteger(left, right, result); } catch (final ArithmeticException nfe) { - if (left == null || right == null) { - controlNullOperand(); - } + // ignore and continue in sequence } } return (left == null? "" : toString(left)).concat(right == null ? "" : toString(right)); @@ -855,7 +891,7 @@ public class JexlArithmetic { if (left == null && right == null) { return controlNullNullOperands(); } - // if both (non null) args fit as long + // if both (non-null) args fit as long final Number ln = asLongNumber(left); final Number rn = asLongNumber(right); if (ln != null && rn != null) { @@ -908,7 +944,7 @@ public class JexlArithmetic { if (left == null && right == null) { return controlNullNullOperands(); } - // if both (non null) args fit as long + // if both (non-null) args fit as long final Number ln = asLongNumber(left); final Number rn = asLongNumber(right); if (ln != null && rn != null) { @@ -977,7 +1013,7 @@ public class JexlArithmetic { if (left == null && right == null) { return controlNullNullOperands(); } - // if both (non null) args fit as int + // if both (non-null) args fit as int final Number ln = asLongNumber(left); final Number rn = asLongNumber(right); if (ln != null && rn != null) { @@ -1021,7 +1057,7 @@ public class JexlArithmetic { if (left == null && right == null) { return controlNullNullOperands(); } - // if both (non null) args fit as long + // if both (non-null) args fit as long final Number ln = asLongNumber(left); final Number rn = asLongNumber(right); if (ln != null && rn != null) { @@ -1063,7 +1099,6 @@ public class JexlArithmetic { */ public Object negate(final Object val) { if (val == null) { - controlNullOperand(); return null; } if (val instanceof Integer) { @@ -1120,7 +1155,6 @@ public class JexlArithmetic { */ public Object positivize(final Object val) { if (val == null) { - controlNullOperand(); return null; } if (val instanceof Short) { @@ -1598,7 +1632,7 @@ public class JexlArithmetic { final String strval = val.toString(); return !strval.isEmpty() && !"false".equals(strval); } - // non null value is true + // non-null value is true return true; } diff --git a/src/main/java/org/apache/commons/jexl3/JexlOperator.java b/src/main/java/org/apache/commons/jexl3/JexlOperator.java index 2b93eb24..905f92a9 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlOperator.java +++ b/src/main/java/org/apache/commons/jexl3/JexlOperator.java @@ -409,7 +409,14 @@ public enum JexlOperator { * <br><strong>Method:</strong> <code>Iterator<Object> forEach(R y);</code>. * @since 3.1 */ - FOR_EACH("for(...)", "forEach", 1); + FOR_EACH("for(...)", "forEach", 1), + + /** + * Test condition in if, for, while. + * <br><strong>Method:</strong> <code>boolean testCondition(R y);</code>. + * @since 3.3 + */ + CONDITION("?", "testCondition", 1); /** * The operator symbol. 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 433440c0..d3433e1b 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -536,6 +536,11 @@ public class Interpreter extends InterpreterBase { } } + private boolean testPredicate(JexlNode node, Object condition) { + final Object predicate = operators.tryOverload(node, JexlOperator.CONDITION, condition); + return arithmetic.toBoolean(predicate != JexlEngine.TRY_FAILED? predicate : condition); + } + @Override protected Object visit(final ASTIfStatement node, final Object data) { final int n = 0; @@ -544,8 +549,9 @@ public class Interpreter extends InterpreterBase { Object result = null; // pairs of { conditions , 'then' statement } for(int ifElse = 0; ifElse < (numChildren - 1); ifElse += 2) { - final Object condition = node.jjtGetChild(ifElse).jjtAccept(this, null); - if (arithmetic.toBoolean(condition)) { + final JexlNode testNode = node.jjtGetChild(ifElse); + final Object condition = testNode.jjtAccept(this, null); + if (testPredicate(testNode, condition)) { // first objectNode is true statement return node.jjtGetChild(ifElse + 1).jjtAccept(this, null); } @@ -753,7 +759,7 @@ public class Interpreter extends InterpreterBase { // last child is body final JexlNode statement = (form & 8) != 0 ? node.jjtGetChild(nc) : null; // while(predicate())... - while (predicate == null || arithmetic.toBoolean(predicate.jjtAccept(this, data))) { + while (predicate == null || testPredicate(predicate, predicate.jjtAccept(this, data))) { cancelCheck(node); // the body if (statement != null) { @@ -785,7 +791,7 @@ public class Interpreter extends InterpreterBase { Object result = null; /* first objectNode is the condition */ final JexlNode condition = node.jjtGetChild(0); - while (arithmetic.toBoolean(condition.jjtAccept(this, data))) { + while (testPredicate(condition, condition.jjtAccept(this, data))) { cancelCheck(node); if (node.jjtGetNumChildren() > 1) { try { @@ -819,7 +825,7 @@ public class Interpreter extends InterpreterBase { //continue; } } - } while (arithmetic.toBoolean(condition.jjtAccept(this, data))); + } while (testPredicate(condition, condition.jjtAccept(this, data))); return result; } diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java index f80ac72d..cab32c2e 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java @@ -1016,17 +1016,11 @@ public class Issues300Test { super(astrict); } @Override - public boolean toBoolean(final Object val) { - return val == null? false : super.toBoolean(val); - } - @Override - public Object not(final Object val) { - return val == null? true : super.not(val); - } - @Override public boolean isStrict(JexlOperator op) { - if (JexlOperator.NOT == op) { - return false; + switch(op) { + case NOT: + case CONDITION: + return false; } return super.isStrict(op); }