This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch JEXL-476 in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit fd0062b8c768ba2351553ee6ce0ffb3265d23869 Author: henrib <[email protected]> AuthorDate: Wed Nov 11 16:37:34 2026 +0100 JEXL-476: Add the safeUndefined option to restrict safe navigation to null - JexlOptions.setSafeUndefined(boolean), JexlBuilder.safeUndefined(boolean), true by default; usable as a flag name and through the jexl.options pragma; - when false, safe navigation only applies to null; an undefined variable, property, method or function is an error, with or without ?., ?[ or the safe option; - when true, the safe access operators treat properties, methods and array accesses alike: x.missing?.y and x?['missing'] evaluate as null as x.missing()?.y and x?.missing do; - ??, ?: and the ternary condition tolerate a missing method or function; - a safe access no longer hides an exception thrown by a property getter. Co-Authored-By: Claude <[email protected]> --- src/changes/changes.xml | 1 + .../java/org/apache/commons/jexl3/JexlBuilder.java | 27 +++++++ .../java/org/apache/commons/jexl3/JexlOptions.java | 47 +++++++++++- .../apache/commons/jexl3/internal/Interpreter.java | 41 +++++------ .../commons/jexl3/internal/InterpreterBase.java | 85 +++++++++++++++++++++- .../org/apache/commons/jexl3/parser/JexlNode.java | 2 +- src/site/xdoc/reference/syntax.xml | 4 + .../org/apache/commons/jexl3/Issues400Test.java | 73 ++++++++++++++++++- 8 files changed, 250 insertions(+), 30 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 33beeb12..b2550e67 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -31,6 +31,7 @@ <!-- FIX --> <action dev="henrib" due-to="Claude" type="fix" issue="JEXL-475">A jexl.options pragma modifies the engine options when evaluated with a plain context.</action> <!-- ADD --> + <action dev="henrib" due-to="Claude" type="add" issue="JEXL-476">Add the safeUndefined option (JexlOptions.setSafeUndefined(boolean), JexlBuilder.safeUndefined(boolean)) to restrict safe navigation to null, undefined variables, properties and methods remaining errors; safe access operators treat properties, methods and array accesses alike.</action> <action dev="henrib" due-to="Claude" type="add" issue="JEXL-474">Declare antish variable names at parse time through JexlFeatures.antishTest(Predicate), disabling runtime antish resolution; add JexlFeatures.namePatterns(Collection).</action> <!-- UPDATE --> <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-parent from 105 to 107.</action> diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java index 338894bd..a475edfd 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java +++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java @@ -922,4 +922,31 @@ public class JexlBuilder { this.uberspect = u; return this; } + + /** + * Does safe navigation apply to what is undefined? + * + * @return true if safe navigation applies to undefined and null, false if it only applies to null + * @since 3.7.2 + */ + public boolean safeUndefined() { + return options.isSafeUndefined(); + } + + /** + * Sets whether safe navigation applies to an undefined variable, property, method or function + * as it does to null. + * <p>{@code x?.y} if x is undefined or has no property y evaluates as null when the flag is true, + * which is the default, throws an exception when it is false; {@code x?.y} if x is null evaluates + * as null in both cases.</p> + * + * @see JexlOptions#setSafeUndefined(boolean) + * @param flag true means safe navigation applies to undefined and null, false means it only applies to null + * @return this builder + * @since 3.7.2 + */ + public JexlBuilder safeUndefined(final boolean flag) { + options.setSafeUndefined(flag); + return this; + } } diff --git a/src/main/java/org/apache/commons/jexl3/JexlOptions.java b/src/main/java/org/apache/commons/jexl3/JexlOptions.java index 908adc32..23e00fe2 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlOptions.java +++ b/src/main/java/org/apache/commons/jexl3/JexlOptions.java @@ -39,6 +39,7 @@ import org.apache.commons.jexl3.internal.Engine; * <li>constCapture: whether captured variables will throw an error if an attempt is made to change their value</li> * <li>strictInterpolation: whether interpolation strings always return a string or attempt to parse and return integer</li> * <li>booleanLogical: whether logical expressions ("" , ||) coerce their result to boolean</li> + * <li>safeUndefined: whether safe navigation applies to what is undefined as it does to null</li> * </ul> * The sensible default is cancellable, strict and strictArithmetic. * <p>This interface replaces the now deprecated JexlEngine.Options. @@ -47,6 +48,9 @@ import org.apache.commons.jexl3.internal.Engine; */ public final class JexlOptions { + /** The safe navigation of undefined flag. */ + private static final int SAFE_UNDEFINED = 12; + /** The namespace auto-instantiation flag. */ private static final int NAMESPACE_INSTANTIATE = 11; @@ -87,12 +91,12 @@ public final class JexlOptions { private static final String[] NAMES = { "cancellable", "strict", "silent", "safe", "lexical", "antish", "lexicalShade", "sharedInstance", "constCapture", "strictInterpolation", - "booleanShortCircuit", "namespaceInstantiation" + "booleanShortCircuit", "namespaceInstantiation", "safeUndefined" }; /** Default mask .*/ private static int DEFAULT = 1 /*<< CANCELLABLE*/ | 1 << STRICT | 1 << ANTISH | 1 << SAFE - | 1 << NAMESPACE_INSTANTIATE; + | 1 << NAMESPACE_INSTANTIATE | 1 << SAFE_UNDEFINED; /** * Checks the value of a flag in the mask. @@ -377,6 +381,18 @@ public final class JexlOptions { return isSet(STRICT_INTERPOLATION, flags); } + /** + * Checks whether safe navigation applies to what is undefined. + * + * @return true if safe navigation evaluates what is undefined as null, + * false if it only applies to null + * @see #setSafeUndefined(boolean) + * @since 3.7.2 + */ + public boolean isSafeUndefined() { + return isSet(SAFE_UNDEFINED, flags); + } + /** * Sets options from engine. * @@ -535,6 +551,8 @@ public final class JexlOptions { * <p>If safe, encountering null during a navigation expression - dereferencing a method or a field through a null * object or property - will <em>not</em> be considered an error but evaluated as <em>null</em>. It is recommended * to use <em>setSafe(false)</em> as an explicit default.</p> + * <p>If safe, every access behaves as a safe access, {@code x.y} as {@code x?.y}; whether this also + * applies to what is undefined is determined by {@link #setSafeUndefined(boolean)}.</p> * * @param flag true if safe, false otherwise */ @@ -596,6 +614,31 @@ public final class JexlOptions { flags = set(STRICT_INTERPOLATION, flags, strict); } + /** + * Sets whether safe navigation applies to what is undefined. + * <p>A variable that is neither a local nor known to the context, a property or a method an object does + * not have, a function that cannot be solved are <em>undefined</em>; a strict engine reports them as errors. + * A safe access, {@code x?.y}, {@code x?.y()}, {@code x?[y]} or any access when {@link #setSafe(boolean)} + * is set, evaluates as null when {@code x} is null.</p> + * <p>When this flag is set, which is the default, {@code x?.y}, {@code x?.y()} and {@code x?[y]} also + * evaluate as null when {@code x} is undefined or does not have the member {@code y}, and what is undefined + * evaluates as null when the access that follows it is safe, as in {@code x.y?.z}. + * The {@link #setSafe(boolean) safe} option excuses an undefined variable or method in the same way, + * with two exceptions kept for compatibility: an undefined property remains an error, as in {@code x.y} + * when {@code x} does not have the property {@code y}, and an undefined variable that is the value + * of an assignment evaluates as null, as in {@code x = y}. + * When this flag is not set, safe navigation only applies to null and what is undefined is an error, + * without exception.</p> + * <p>In both cases, the condition of a ternary and the left-hand side of the elvis ({@code ?:}) and + * null-coalescing ({@code ??}) operators tolerate what is undefined.</p> + * + * @param flag true if safe navigation applies to undefined and null, false if it only applies to null + * @since 3.7.2 + */ + public void setSafeUndefined(final boolean flag) { + flags = set(SAFE_UNDEFINED, flags, flag); + } + @Override public String toString() { final StringBuilder strb = new StringBuilder(); for(int i = 0; i < NAMES.length; ++i) { 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 9acb3908..a079f572 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -275,7 +275,7 @@ public class Interpreter extends InterpreterBase { // ...(x)(y) methodName = null; cacheable = false; - } else if (!node.isSafeLhs(isSafe())) { + } else if (!isSafeUndefined() || !node.isSafeLhs(isSafe())) { return unsolvableMethod(node, "?(...)"); } else { // safe lhs @@ -391,7 +391,7 @@ public class Interpreter extends InterpreterBase { throw invocationException(node, methodName, xany); } // we have either evaluated and returned or no method was found - return node.isSafeLhs(isSafe()) + return isSafeUndefined() && node.isSafeLhs(isSafe()) ? null : unsolvableMethod(node, methodName, argv); } @@ -1721,7 +1721,7 @@ public class Interpreter extends InterpreterBase { object = data; if (object == null) { // no object, we fail - return node.isSafeLhs(isSafe()) + return isSafe() || ((ASTIdentifierAccess) methodNode).isSafe() ? null : unsolvableMethod(methodNode, "<null>.<?>(...)"); } @@ -1736,9 +1736,9 @@ public class Interpreter extends InterpreterBase { for (int a = 1; a < node.jjtGetNumChildren(); ++a) { if (result == null) { // no method, we fail// variable unknown in context and not a local - return node.isSafeLhs(isSafe()) + return isSafeUndefined() && node.isSafeLhs(isSafe()) ? null - : unsolvableMethod(methodNode, "<?>.<null>(...)"); + : unsolvableMethod(node, "<?>.<null>(...)"); } final ASTArguments argNode = (ASTArguments) node.jjtGetChild(a); result = call(node, object, result, argNode); @@ -1902,6 +1902,8 @@ public class Interpreter extends InterpreterBase { boolean antish = !(parent instanceof ASTReference) && options.isAntish(); // the index of the access to apply to the object int deref = 0; + // whether the variable the reference starts with is undefined + boolean undef = false; main: for (int c = 0; c < numChildren; c++) { objectNode = node.jjtGetChild(c); @@ -1939,6 +1941,10 @@ public class Interpreter extends InterpreterBase { object = objectNode.jjtAccept(this, object); deref = c + 1; cancelCheck(node); + if (object == null && c == 0 && objectNode instanceof ASTIdentifier) { + // a null variable or an undefined one ? + undef = !isVariableDefined(frame, ((ASTIdentifier) objectNode).getName()); + } if (object != null) { // disallow mixing antish variable & bean with same root; avoid ambiguity antish = false; @@ -1972,7 +1978,10 @@ public class Interpreter extends InterpreterBase { break main; } // solve antish - object = context.get(ant.toString()); + final String aname = ant.toString(); + object = context.get(aname); + // an antish variable may be defined and null + undef = undef && object == null && !context.has(aname); } else if (c != numChildren - 1) { // only the last one may be null ptyNode = c == 0 && numChildren > 1 ? node.jjtGetChild(1) : objectNode; @@ -1981,9 +1990,11 @@ public class Interpreter extends InterpreterBase { } // dealing with null if (object == null) { + // safe navigation applies to null, to an undefined variable as an option + final boolean nullable = !undef || isSafeUndefined(); if (ptyNode != null) { // the access applied to null decides, as in x?.y, x?.y() or x?[y] - if (isSafe() || deref < numChildren && isSafeAccess(node.jjtGetChild(deref))) { + if (nullable && (isSafe() || deref < numChildren && isSafeAccess(node.jjtGetChild(deref)))) { return null; } if (ant != null) { @@ -1995,7 +2006,7 @@ public class Interpreter extends InterpreterBase { stringifyProperty(ptyNode), ptyNode == objectNode, null); } if (antish) { - if (node.isSafeLhs(isSafe())) { + if (nullable && node.isSafeLhs(isSafe())) { return null; } final String aname = Objects.toString(ant, "?"); @@ -2010,20 +2021,6 @@ public class Interpreter extends InterpreterBase { return object; } - /** - * Checks whether an access in a reference is explicitly safe, as in ?.y, ?.y() or ?[y]. - * - * @param access the access, a method node being checked through the member it calls - * @return true if the access is safe, false otherwise - */ - private static boolean isSafeAccess(final JexlNode access) { - final JexlNode member = access instanceof ASTMethodNode ? access.jjtGetChild(0) : access; - if (member instanceof ASTIdentifierAccess) { - return ((ASTIdentifierAccess) member).isSafe(); - } - return member instanceof ASTArrayAccess && ((ASTArrayAccess) member).isSafeChild(0); - } - @Override protected Object visit(final ASTReferenceExpression node, final Object data) { return node.jjtGetChild(0).jjtAccept(this, 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 61396ae5..bd740349 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java +++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java @@ -663,8 +663,8 @@ public abstract class InterpreterBase extends ParserVisitor { + ", property: " + attribute; throw new UnsupportedOperationException(error, xcause); } - final boolean safe = node instanceof ASTIdentifierAccess && ((ASTIdentifierAccess) node).isSafe(); - if (safe) { + // safe navigation may apply to an undefined property, an access that failed remains an error + if (xcause == null && isSafeUndefined(node)) { return null; } final String attrStr = Objects.toString(attribute, null); @@ -706,7 +706,8 @@ public abstract class InterpreterBase extends ParserVisitor { if (!context.has(name)) { // not defined, ignore in some cases... final boolean ignore = identifier.jjtGetParent() instanceof ASTReference - || isSafe() && (symbol >= 0 || identifier.jjtGetParent() instanceof ASTAssignment); + || isSafeUndefined() && isSafe() + && (symbol >= 0 || identifier.jjtGetParent() instanceof ASTAssignment); if (!ignore) { return undefinedVariable(identifier, name); // undefined } @@ -770,6 +771,82 @@ public abstract class InterpreterBase extends ParserVisitor { return options.isSilent(); } + /** + * Whether safe navigation applies to what is undefined in this interpreter. + * + * @return true if safe navigation applies to undefined and null, false if it only applies to null + */ + protected boolean isSafeUndefined() { + return options.isSafeUndefined(); + } + + /** + * Whether safe navigation excuses an undefined property. + * <p>The access to the property is explicitly safe, as in x?.y or x?[y], or the access that follows it is, + * as in x.y?.z; the safe option does not excuse an undefined property.</p> + * + * @param node the node accessing the property + * @return true if the property evaluates as null, false if it is an error + */ + protected boolean isSafeUndefined(final JexlNode node) { + if (!isSafeUndefined()) { + return false; + } + JexlNode access = node; + JexlNode parent = node.jjtGetParent(); + if (parent instanceof ASTArrayAccess) { + // an index, as in x?[y] or x[y]?[z] + final ASTArrayAccess indexed = (ASTArrayAccess) parent; + final int next = indexOf(indexed, node) + 1; + if (indexed.isSafeChild(next - 1)) { + return true; + } + if (next < indexed.jjtGetNumChildren()) { + return indexed.isSafeChild(next); + } + access = indexed; + parent = indexed.jjtGetParent(); + } else if (isSafeAccess(node)) { + return true; + } + // the access that follows, as in x.y?.z + if (parent == null) { + return false; + } + final int next = indexOf(parent, access) + 1; + return next > 0 && next < parent.jjtGetNumChildren() && isSafeAccess(parent.jjtGetChild(next)); + } + + /** + * Seeks a node in its parent. + * + * @param parent the parent + * @param child the child + * @return the index of the child in its parent, -1 if not found + */ + private static int indexOf(final JexlNode parent, final JexlNode child) { + for (int c = 0; c < parent.jjtGetNumChildren(); ++c) { + if (parent.jjtGetChild(c) == child) { + return c; + } + } + return -1; + } + + /** + * Checks whether an access in a reference is explicitly safe, as in ?.y, ?.y() or ?[y]. + * + * @param access the access, a method node being checked through the member it calls + * @return true if the access is safe, false otherwise + */ + protected static boolean isSafeAccess(final JexlNode access) { + final JexlNode member = access instanceof ASTMethodNode ? access.jjtGetChild(0) : access; + if (member instanceof ASTIdentifierAccess) { + return ((ASTIdentifierAccess) member).isSafe(); + } + return member instanceof ASTArrayAccess && ((ASTArrayAccess) member).isSafeChild(0); + } + /** * Whether this interpreter is currently evaluating with a strict engine flag. * @@ -1134,7 +1211,7 @@ public abstract class InterpreterBase extends ParserVisitor { * @return throws JexlException if strict and not silent, null otherwise */ protected Object unsolvableMethod(final JexlNode node, final String method, final Object[] args) { - if (isStrictEngine()) { + if (isStrictEngine() && !isTernaryProtected(node)) { throw new JexlException.Method(node, method, args); } if (logger.isDebugEnabled()) { 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 705f33d5..28f24ae7 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java +++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java @@ -301,7 +301,7 @@ public abstract class JexlNode extends SimpleNode implements JexlCache.Reference return true; } if (rsibling instanceof ASTArrayAccess) { - return safe; + return safe || ((ASTArrayAccess) rsibling).isSafeChild(0); } } return false; diff --git a/src/site/xdoc/reference/syntax.xml b/src/site/xdoc/reference/syntax.xml index f90979b6..5c62b84e 100644 --- a/src/site/xdoc/reference/syntax.xml +++ b/src/site/xdoc/reference/syntax.xml @@ -291,6 +291,10 @@ In the previous expression, if 'foo' is null, the whole expression will evaluate as null. Note that this can also be used in a chain as in <code>x?[y]?[z]</code>. </p> + <p>Whether safe-access operators shortcut non-existent references - an undefined variable, + property or method - as they do null is determined by the <code>safeUndefined</code> option, + true by default; when false, only null is shortcut and a non-existent reference is an error. + </p> <p>Access operators can be overloaded in <code>JexlArithmetic</code>, so that the operator behavior will differ depending on the type of the operator arguments</p> </td> diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java b/src/test/java/org/apache/commons/jexl3/Issues400Test.java index 884af4a9..0e4436f1 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java @@ -1393,5 +1393,76 @@ public class Issues400Test { assertNull(jexl.createScript("#pragma jexl.options '-strict'\nfoo").execute(null)); assertThrows(JexlException.Variable.class, () -> jexl.createScript("foo").execute(null)); } -} + @Test + void test476() { + final String err = "error"; + // an expression and its result when (safeUndefined, safe) is (true, false), (true, true), (false, false), (false, true) + final Object[][] cases = { + // an undefined variable + {"u", err, err, err, err}, + {"u.y", err, null, err, err}, + {"u?.y", null, null, err, err}, + {"u[0]", err, null, err, err}, + {"u?[0]", null, null, err, err}, + {"u.m()", err, null, err, err}, + {"u?.m()", null, null, err, err}, + {"u == null", err, err, err, err}, + {"u ?? 1", 1, 1, 1, 1}, + // an undefined property + {"'abc'.nothing", err, err, err, err}, + {"'abc'?.nothing", null, null, err, err}, + {"'abc'['nothing']", err, err, err, err}, + {"'abc'?['nothing']", null, null, err, err}, + {"'abc'.nothing.y", err, err, err, err}, + {"'abc'.nothing?.y", null, null, err, err}, + {"'abc'.nothing ?? 1", 1, 1, 1, 1}, + // an undefined method + {"'abc'.nothing()", err, null, err, err}, + {"'abc'?.nothing()", null, null, err, err}, + {"'abc'.nothing().y", err, null, err, err}, + {"'abc'.nothing()?.y", null, null, err, err}, + {"'abc'.nothing() ?? 1", 1, 1, 1, 1}, + // an undefined function + {"nothing(1)", err, err, err, err}, + {"nothing(1).y", err, null, err, err}, + {"nothing(1)?.y", null, null, err, err}, + {"nothing(1) ?? 1", 1, 1, 1, 1}, + // null is not undefined + {"nul", null, null, null, null}, + {"nul.y", err, null, err, null}, + {"nul?.y", null, null, null, null}, + {"nul?.y.z", null, null, null, null}, + {"nul[0]", err, null, err, null}, + {"nul?[0]", null, null, null, null}, + {"nul.m()", err, null, err, null}, + {"nul?.m()", null, null, null, null}, + {"nul(1)", err, err, err, err}, + {"nul + 1", err, err, err, err}, + {"nul.y ?? 1", 1, 1, 1, 1}, + }; + final JexlContext ctxt = new MapContext(); + ctxt.set("nul", null); + for (int m = 0; m < 4; ++m) { + final boolean undef = (m & 2) == 0; + final boolean safe = (m & 1) != 0; + final JexlEngine jexl = new JexlBuilder().strict(true).safe(safe).safeUndefined(undef).create(); + for (final Object[] c : cases) { + final String src = (String) c[0]; + final Object expected = c[m + 1]; + final String msg = src + ", safeUndefined: " + undef + ", safe: " + safe; + final JexlScript script = jexl.createScript(src); + if (expected == err) { + assertThrows(JexlException.class, () -> script.execute(ctxt), msg); + } else { + assertEquals(expected, script.execute(ctxt), msg); + } + } + } + // as a pragma + final JexlEngine jexl = new JexlBuilder().features(JexlFeatures.createDefault().pragma(true)).strict(true).create(); + assertNull(jexl.createScript("u?.y").execute(ctxt)); + assertThrows(JexlException.Variable.class, + () -> jexl.createScript("#pragma jexl.options '-safeUndefined'\nu?.y").execute(ctxt)); + } +}
