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 650575a JEXL-275: added option, tests and changes 650575a is described below commit 650575a978fec23872cc01472ac0757123d16905 Author: henrib <hen...@apache.org> AuthorDate: Mon Sep 17 22:09:15 2018 +0200 JEXL-275: added option, tests and changes --- RELEASE-NOTES.txt | 1 + .../java/org/apache/commons/jexl3/JexlBuilder.java | 22 +++ .../java/org/apache/commons/jexl3/JexlEngine.java | 2 +- .../org/apache/commons/jexl3/internal/Engine.java | 5 + .../apache/commons/jexl3/internal/Interpreter.java | 11 +- .../commons/jexl3/internal/InterpreterBase.java | 8 + .../org/apache/commons/jexl3/parser/JexlNode.java | 10 +- src/site/xdoc/changes.xml | 3 + .../org/apache/commons/jexl3/Issues200Test.java | 173 +------------------- .../apache/commons/jexl3/PropertyAccessTest.java | 181 +++++++++++++++++++++ 10 files changed, 236 insertions(+), 180 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 4d049d8..d791ca1 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -39,6 +39,7 @@ What's new in 3.2: New Features in 3.2: ==================== +* JEXL-275: Allow safe navigation as option * JEXL-273: Add do...while(...) loops * JEXL-264: Allow space, quote & double-quote in identifiers * JEXL-260: Automatically inject JexlContext in constructor call when possible diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java index 7332f57..928069e 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java +++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java @@ -82,6 +82,9 @@ public class JexlBuilder { /** Whether this engine is in lenient or strict mode; if unspecified, use the arithmetic lenient property. */ private Boolean strict = null; + /** Whether this engine is in tolerant mode. */ + private Boolean safe = false; + /** Whether error messages will carry debugging information. */ private Boolean debug = null; @@ -286,6 +289,25 @@ public class JexlBuilder { } /** + * Sets whether the engine considers dereferencing null in navigation expressions + * as errors or evaluates them as null. + * <p><code>x.y()</code> if x is null throws an exception when not safe, + * return null and warns if it is.<p> + * + * @param flag true means safe navigation, false throws exception when dereferencing null + * @return this builder + */ + public JexlBuilder safe(boolean flag) { + this.safe = flag; + return this; + } + + /** @return true if safe, false otherwise */ + public Boolean safe() { + return this.safe; + } + + /** * Sets whether the engine will report debugging information when error occurs. * * @param flag true implies debug is on, false implies debug is off. diff --git a/src/main/java/org/apache/commons/jexl3/JexlEngine.java b/src/main/java/org/apache/commons/jexl3/JexlEngine.java index deafdf8..06202f0 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlEngine.java +++ b/src/main/java/org/apache/commons/jexl3/JexlEngine.java @@ -130,7 +130,7 @@ public abstract class JexlEngine { * @return true if strict, false otherwise */ Boolean isStrict(); - + /** * Checks whether the arithmetic triggers errors during evaluation when null is used as an operand. * diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java b/src/main/java/org/apache/commons/jexl3/internal/Engine.java index e3de73d..bc9735a 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java @@ -94,6 +94,10 @@ public class Engine extends JexlEngine { */ protected final boolean strict; /** + * Whether this engine considers null in navigation expression as errors. + */ + protected final boolean safe; + /** * Whether expressions evaluated by this engine will throw exceptions (false) or return null (true) on errors. * Default is false. */ @@ -156,6 +160,7 @@ public class Engine extends JexlEngine { public Engine(JexlBuilder conf) { // options: this.strict = conf.strict() == null ? true : conf.strict(); + this.safe = conf.safe() == null ? false : conf.safe(); this.silent = conf.silent() == null ? false : conf.silent(); this.cancellable = conf.cancellable() == null ? !silent && strict : conf.cancellable(); this.debug = conf.debug() == null ? true : conf.debug(); 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 39094c0..4837bc7 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -114,6 +114,7 @@ import java.util.List; import java.util.Map; import java.util.regex.Pattern; import java.util.concurrent.Callable; +import org.apache.commons.jexl3.JxltEngine; /** @@ -1125,13 +1126,13 @@ public class Interpreter extends InterpreterBase { if (antish && ant != null) { boolean undefined = !(context.has(ant.toString()) || isLocalVariable(node, 0)); // variable unknown in context and not a local - return node.isSafeLhs() + 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() + return ptyNode.isSafeLhs(jexl.safe) ? null : unsolvableProperty(node, stringifyProperty(ptyNode), false, null); } @@ -1447,7 +1448,7 @@ public class Interpreter extends InterpreterBase { } else if (context.has(methodName)) { functor = context.get(methodName); isavar = functor != null; - } + } // name is a variable, cant be cached cacheable &= !isavar; } @@ -1465,7 +1466,7 @@ public class Interpreter extends InterpreterBase { } else { return unsolvableMethod(node, "?"); } - + // solving the call site CallDispatcher call = new CallDispatcher(node, cacheable); try { @@ -1473,7 +1474,7 @@ public class Interpreter extends InterpreterBase { Object eval = call.tryEval(target, methodName, argv); if (JexlEngine.TRY_FAILED != eval) { return eval; - } + } boolean functorp = false; boolean narrow = false; // pseudo loop to try acquiring methods without and with argument narrowing 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 6658594..e8a0287 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java +++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java @@ -25,6 +25,7 @@ import org.apache.commons.jexl3.JexlOperator; 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.ASTMethodNode; import org.apache.commons.jexl3.parser.JexlNode; import org.apache.commons.jexl3.parser.ParserVisitor; @@ -250,6 +251,13 @@ public abstract class InterpreterBase extends ParserVisitor { } return ".???(...)"; } + if (node instanceof ASTFunctionNode) { + if (node.jjtGetChild(0) != null) { + return node.jjtGetChild(0).toString() + + "(...)"; + } + return "???(...)"; + } return node.toString(); } 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 cd4706d..2a91f5e 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java +++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java @@ -195,11 +195,12 @@ public abstract class JexlNode extends SimpleNode { /** * Whether this node is the left-hand side of a safe access identifier as in. * For instance, in 'x?.y' , 'x' is safe. + * @param safe whether the engine is in safe-navigation mode * @return true if safe lhs, false otherwise */ - public boolean isSafeLhs() { + public boolean isSafeLhs(boolean safe) { if (this instanceof ASTReference) { - return jjtGetChild(0).isSafeLhs(); + return jjtGetChild(0).isSafeLhs(safe); } JexlNode parent = this.jjtGetParent(); if (parent == null) { @@ -219,10 +220,11 @@ public abstract class JexlNode extends SimpleNode { // seek next child in parent if (rhs >= 0 && rhs < nsiblings) { JexlNode rsibling = parent.jjtGetChild(rhs); - if (rsibling instanceof ASTMethodNode) { + if (rsibling instanceof ASTMethodNode || rsibling instanceof ASTFunctionNode) { rsibling = rsibling.jjtGetChild(0); } - if (rsibling instanceof ASTIdentifierAccess && ((ASTIdentifierAccess) rsibling).isSafe()) { + if (rsibling instanceof ASTIdentifierAccess + && (((ASTIdentifierAccess) rsibling).isSafe() || safe)) { return true; } } diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml index 61baaf0..96c1ab3 100644 --- a/src/site/xdoc/changes.xml +++ b/src/site/xdoc/changes.xml @@ -26,6 +26,9 @@ </properties> <body> <release version="3.2" date="unreleased"> + <action dev="henrib" type="add" issue="JEXL-275"> + Allow safe navigation as option + </action> <action dev="Dmitri Blinov" type="add" issue="JEXL-175" due-to="Dmitri Blinov"> Add do...while(...) loops </action> diff --git a/src/test/java/org/apache/commons/jexl3/Issues200Test.java b/src/test/java/org/apache/commons/jexl3/Issues200Test.java index ff44930..a4cfe4f 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues200Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues200Test.java @@ -320,7 +320,7 @@ public class Issues200Test extends JexlTestCase { Foo245 foo245 = new Foo245(); ctx.set("foo", foo245); - JexlEngine engine = new JexlBuilder().strict(true).silent(false).create(); + JexlEngine engine = new JexlBuilder().strict(true).safe(false).silent(false).create(); JexlExpression foobar = engine.createExpression("foo.bar"); JexlExpression foobaz = engine.createExpression("foo.baz"); JexlExpression foobarbaz = engine.createExpression("foo.bar.baz"); @@ -333,14 +333,14 @@ public class Issues200Test extends JexlTestCase { // fail level 1 try { foobaz.evaluate(ctx); - Assert.fail("foo.baz is not solvable"); + Assert.fail("foo.baz is not solvable, exception expected"); } catch(JexlException xp) { Assert.assertTrue(xp instanceof JexlException.Property); } // fail level 2 try { foobarbaz.evaluate(ctx); - Assert.fail("foo.bar.baz is not solvable"); + Assert.fail("foo.bar.baz is not solvable, exception expected"); } catch(JexlException xp) { Assert.assertTrue(xp instanceof JexlException.Property); } @@ -348,91 +348,6 @@ public class Issues200Test extends JexlTestCase { } @Test - public void test250() throws Exception { - MapContext ctx = new MapContext(); - HashMap<Object, Object> x = new HashMap<Object, Object>(); - x.put(2, "123456789"); - ctx.set("x", x); - JexlEngine engine = new JexlBuilder().strict(true).silent(false).create(); - String stmt = "x.2.class.name"; - JexlScript script = engine.createScript(stmt); - Object result = script.execute(ctx); - Assert.assertEquals("java.lang.String", result); - - try { - stmt = "x.3?.class.name"; - script = engine.createScript(stmt); - result = script.execute(ctx); - Assert.assertNull(result); - } catch (JexlException xany) { - Assert.fail("Should have evaluated to null"); - } - try { - stmt = "x?.3.class.name"; - script = engine.createScript(stmt); - result = script.execute(ctx); - Assert.fail("Should have thrown, fail on 3"); - Assert.assertNull(result); - } catch (JexlException xany) { - Assert.assertTrue(xany.detailedMessage().contains("3")); - } - try { - stmt = "x?.3?.class.name"; - script = engine.createScript(stmt); - result = script.execute(ctx); - Assert.assertNull(result); - } catch (JexlException xany) { - Assert.fail("Should have evaluated to null"); - } - try { - stmt = "y?.3.class.name"; - script = engine.createScript(stmt); - result = script.execute(ctx); - Assert.assertNull(result); - } catch (JexlException xany) { - Assert.fail("Should have evaluated to null"); - } - try { - stmt = "x?.y?.z"; - script = engine.createScript(stmt); - result = script.execute(ctx); - Assert.assertNull(result); - } catch (JexlException xany) { - Assert.fail("Should have evaluated to null"); - } - try { - stmt = "x? (x.y? (x.y.z ?: null) :null) : null"; - script = engine.createScript(stmt); - result = script.execute(ctx); - Assert.assertNull(result); - } catch (JexlException xany) { - Assert.fail("Should have evaluated to null"); - } - } - - @Test - public void test252() throws Exception { - MapContext ctx = new MapContext(); - JexlEngine engine = new JexlBuilder().strict(true).silent(false).create(); - String stmt = "(x, dflt)->{ x?.class1 ?? dflt }"; - JexlScript script = engine.createScript(stmt); - Object result = script.execute(ctx, "querty", "default"); - Assert.assertEquals("default", result); - try { - stmt = "(x, al, dflt)->{ x.`c${al}ss` ?? dflt }"; - script = engine.createScript(stmt); - result = script.execute(ctx, "querty", "la", "default"); - Assert.assertEquals(stmt.getClass(), result); - stmt = "(x, al, dflt)->{ x?.`c${al}ss` ?? dflt }"; - script = engine.createScript(stmt); - result = script.execute(ctx, "querty", "la", "default"); - Assert.assertEquals(stmt.getClass(), result); - } catch(JexlException xany) { - String xanystr = xany.toString(); - } - } - - @Test public void test256() throws Exception { MapContext ctx = new MapContext() { @Override public void set(String name, Object value) { @@ -653,86 +568,4 @@ public class Issues200Test extends JexlTestCase { Assert.assertTrue(result instanceof JexlScript); } - public static class Prompt { - private final Map<String, PromptValue> values = new HashMap<String, PromptValue>(); - - public Object get(String name) { - PromptValue v = values.get(name); - return v != null? v.getValue() : null; - } - - public void set(String name, Object value) { - values.put(name, new PromptValue(value)); - } - } - - /** - * A valued prompt. - */ - public static class PromptValue { - - /** Prompt value. */ - private Object value; - - public PromptValue(Object v) { - value = v; - } - - public Object getValue() { - return value; - } - - public void setValue(Object value) { - this.value = value; - } - } - - @Test - public void test272() throws Exception { - JexlEngine jexl = new JexlBuilder().strict(true).create(); - JexlContext ctxt = new MapContext(); - JexlScript script; - Object result = null; - Prompt p0 = new Prompt(); - p0.set("stuff", 42); - ctxt.set("$in", p0); - - // unprotected navigation - script = jexl.createScript("$in[p].intValue()", "p"); - try { - result = script.execute(ctxt, "fail"); - Assert.fail("should have thrown a " + JexlException.Property.class); - } catch (JexlException xany) { - Assert.assertEquals(JexlException.Property.class, xany.getClass()); - } - Assert.assertEquals(null, result); - result = script.execute(ctxt, "stuff"); - Assert.assertEquals(42, result); - - // protected navigation - script = jexl.createScript("$in[p]?.intValue()", "p"); - result = script.execute(ctxt, "fail"); - Assert.assertEquals(null, result); - result = script.execute(ctxt, "stuff"); - Assert.assertEquals(42, result); - - // unprotected navigation - script = jexl.createScript("$in.`${p}`.intValue()", "p"); - try { - result = script.execute(ctxt, "fail"); - Assert.fail("should have thrown a " + JexlException.Property.class); - } catch (JexlException xany) { - Assert.assertEquals(JexlException.Property.class, xany.getClass()); - } - result = script.execute(ctxt, "stuff"); - Assert.assertEquals(42, result); - - // protected navigation - script = jexl.createScript("$in.`${p}`?.intValue()", "p"); - result = script.execute(ctxt, "fail"); - Assert.assertEquals(null, result); - result = script.execute(ctxt, "stuff"); - Assert.assertEquals(42, result); - - } } diff --git a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java index ca76014..4af7a5a 100644 --- a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java +++ b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java @@ -387,4 +387,185 @@ public class PropertyAccessTest extends JexlTestCase { Assert.assertTrue(xjexl.getMessage().contains("c${la--ss")); } } + + @Test + public void test250() throws Exception { + MapContext ctx = new MapContext(); + HashMap<Object, Object> x = new HashMap<Object, Object>(); + x.put(2, "123456789"); + ctx.set("x", x); + JexlEngine engine = new JexlBuilder().strict(true).silent(false).create(); + String stmt = "x.2.class.name"; + JexlScript script = engine.createScript(stmt); + Object result = script.execute(ctx); + Assert.assertEquals("java.lang.String", result); + + try { + stmt = "x.3?.class.name"; + script = engine.createScript(stmt); + result = script.execute(ctx); + Assert.assertNull(result); + } catch (JexlException xany) { + Assert.fail("Should have evaluated to null"); + } + try { + stmt = "x?.3.class.name"; + script = engine.createScript(stmt); + result = script.execute(ctx); + Assert.fail("Should have thrown, fail on 3"); + Assert.assertNull(result); + } catch (JexlException xany) { + Assert.assertTrue(xany.detailedMessage().contains("3")); + } + try { + stmt = "x?.3?.class.name"; + script = engine.createScript(stmt); + result = script.execute(ctx); + Assert.assertNull(result); + } catch (JexlException xany) { + Assert.fail("Should have evaluated to null"); + } + try { + stmt = "y?.3.class.name"; + script = engine.createScript(stmt); + result = script.execute(ctx); + Assert.assertNull(result); + } catch (JexlException xany) { + Assert.fail("Should have evaluated to null"); + } + try { + stmt = "x?.y?.z"; + script = engine.createScript(stmt); + result = script.execute(ctx); + Assert.assertNull(result); + } catch (JexlException xany) { + Assert.fail("Should have evaluated to null"); + } + try { + stmt = "x? (x.y? (x.y.z ?: null) :null) : null"; + script = engine.createScript(stmt); + result = script.execute(ctx); + Assert.assertNull(result); + } catch (JexlException xany) { + Assert.fail("Should have evaluated to null"); + } + } + + public static class Prompt { + private final Map<String, PromptValue> values = new HashMap<String, PromptValue>(); + + public Object get(String name) { + PromptValue v = values.get(name); + return v != null? v.getValue() : null; + } + + public void set(String name, Object value) { + values.put(name, new PromptValue(value)); + } + } + + /** + * A valued prompt. + */ + public static class PromptValue { + + /** Prompt value. */ + private Object value; + + public PromptValue(Object v) { + value = v; + } + + public Object getValue() { + return value; + } + + public void setValue(Object value) { + this.value = value; + } + } + + @Test + public void test275a() throws Exception { + JexlEngine jexl = new JexlBuilder().strict(true).safe(false).create(); + JexlContext ctxt = new MapContext(); + JexlScript script; + Object result = null; + Prompt p0 = new Prompt(); + p0.set("stuff", 42); + ctxt.set("$in", p0); + + // unprotected navigation + script = jexl.createScript("$in[p].intValue()", "p"); + try { + result = script.execute(ctxt, "fail"); + Assert.fail("should have thrown a " + JexlException.Property.class); + } catch (JexlException xany) { + Assert.assertEquals(JexlException.Property.class, xany.getClass()); + } + Assert.assertEquals(null, result); + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + // protected navigation + script = jexl.createScript("$in[p]?.intValue()", "p"); + result = script.execute(ctxt, "fail"); + Assert.assertEquals(null, result); + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + // unprotected navigation + script = jexl.createScript("$in.`${p}`.intValue()", "p"); + try { + result = script.execute(ctxt, "fail"); + Assert.fail("should have thrown a " + JexlException.Property.class); + } catch (JexlException xany) { + Assert.assertEquals(JexlException.Property.class, xany.getClass()); + } + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + // protected navigation + script = jexl.createScript("$in.`${p}`?.intValue()", "p"); + result = script.execute(ctxt, "fail"); + Assert.assertEquals(null, result); + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + } + @Test + public void test275b() throws Exception { + JexlEngine jexl = new JexlBuilder().strict(true).safe(true).create(); + JexlContext ctxt = new MapContext(); + JexlScript script; + Object result = null; + Prompt p0 = new Prompt(); + p0.set("stuff", 42); + ctxt.set("$in", p0); + + // unprotected navigation + script = jexl.createScript("$in[p].intValue()", "p"); + result = script.execute(ctxt, "fail"); + Assert.assertEquals(null, result); + + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + + // unprotected navigation + script = jexl.createScript("$in.`${p}`.intValue()", "p"); + result = script.execute(ctxt, "fail"); + Assert.assertEquals(null, result); + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + // protected navigation + script = jexl.createScript("$in.`${p}`?.intValue()", "p"); + result = script.execute(ctxt, "fail"); + Assert.assertEquals(null, result); + result = script.execute(ctxt, "stuff"); + Assert.assertEquals(42, result); + + } + } \ No newline at end of file