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 2f7d6e9 JEXL-272: properly detect and report null property dereference JEXL-271: moved lambda/curry tests to test class 2f7d6e9 is described below commit 2f7d6e91261a7211e34efaa53e7e6eb860e1dc62 Author: henrib <hen...@apache.org> AuthorDate: Wed Sep 12 14:51:57 2018 +0200 JEXL-272: properly detect and report null property dereference JEXL-271: moved lambda/curry tests to test class --- RELEASE-NOTES.txt | 4 +- .../org/apache/commons/jexl3/JexlException.java | 52 ++++++++++---- .../java/org/apache/commons/jexl3/JexlScript.java | 2 +- .../org/apache/commons/jexl3/ObjectContext.java | 4 +- .../apache/commons/jexl3/internal/Interpreter.java | 20 +++--- .../commons/jexl3/internal/InterpreterBase.java | 37 ++++++++-- .../org/apache/commons/jexl3/internal/Scope.java | 6 ++ .../org/apache/commons/jexl3/parser/JexlNode.java | 3 + src/site/xdoc/changes.xml | 3 + .../org/apache/commons/jexl3/Issues200Test.java | 83 ++++++++++++++++++++++ .../org/apache/commons/jexl3/JexlTestCase.java | 6 +- .../java/org/apache/commons/jexl3/LambdaTest.java | 41 ++++++----- 12 files changed, 209 insertions(+), 52 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 85db7c9..243a61b 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -40,7 +40,8 @@ New Features in 3.2: ==================== * JEXL-264: Allow space, quote & double-quote in identifiers -* JEXL-260: Automatically inject JexlContext in constructor call when possible +* JEXL-260: Automatically inject JexlContext in constructor call when possib +* JEXL-272: Dereferencing null property not reported on method callle * JEXL-252: Allow for interpolated strings to be used in property access operators * JEXL-250: Safe navigation operator (?.) * JEXL-248: Allow range subexpression as an array property assignment identifier @@ -56,6 +57,7 @@ New Features in 3.2: Bugs Fixed in 3.2: ================== +* JEXL-272: Dereferencing null property not reported on method call * JEXL-271: Hoisted variable is lost when currying lambda * JEXL-270: Wrong Script$Curried creation when script.curry() method is called inside script * JEXL-261: JexlEngine.setClassLoader(...) should reload namespaces that are classes diff --git a/src/main/java/org/apache/commons/jexl3/JexlException.java b/src/main/java/org/apache/commons/jexl3/JexlException.java index d23ecc0..1d009d0 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlException.java +++ b/src/main/java/org/apache/commons/jexl3/JexlException.java @@ -439,24 +439,43 @@ public class JexlException extends RuntimeException { */ public static class Property extends JexlException { /** + * Undefined variable flag. + */ + private final boolean undefined; + + /** * Creates a new Property exception instance. * * @param node the offending ASTnode - * @param var the unknown variable + * @param pty the unknown property + * @param cause the exception causing the error + * @deprecated 3.2 */ - public Property(JexlNode node, String var) { - this(node, var, null); + @Deprecated + public Property(JexlNode node, String pty, Throwable cause) { + this(node, pty, true, cause); } - + /** * Creates a new Property exception instance. * - * @param node the offending ASTnode - * @param var the unknown variable + * @param node the offending ASTnode + * @param pty the unknown property + * @param undef whether the variable is null or undefined * @param cause the exception causing the error */ - public Property(JexlNode node, String var, Throwable cause) { - super(node, var, cause); + public Property(JexlNode node, String pty, boolean undef, Throwable cause) { + super(node, pty, cause); + undefined = undef; + } + + /** + * Whether the variable causing an error is undefined or evaluated as null. + * + * @return true if undefined, false otherwise + */ + public boolean isUndefined() { + return undefined; } /** @@ -468,7 +487,7 @@ public class JexlException extends RuntimeException { @Override protected String detailedMessage() { - return "unsolvable property '" + getProperty() + "'"; + return (undefined? "undefined" : "null value") + " property " + getProperty(); } } @@ -476,14 +495,19 @@ public class JexlException extends RuntimeException { * Generates a message for an unsolvable property error. * * @param node the node where the error occurred - * @param var the variable + * @param pty the property + * @param undef whether the property is null or undefined * @return the error message */ - public static String propertyError(JexlNode node, String var) { + public static String propertyError(JexlNode node, String pty, boolean undef) { StringBuilder msg = errorAt(node); - msg.append("unsolvable property '"); - msg.append(var); - msg.append('\''); + if (undef) { + msg.append("unsolvable"); + } else { + msg.append("null value"); + } + msg.append(" property "); + msg.append(pty); return msg.toString(); } diff --git a/src/main/java/org/apache/commons/jexl3/JexlScript.java b/src/main/java/org/apache/commons/jexl3/JexlScript.java index 67b3e5d..ada303c 100644 --- a/src/main/java/org/apache/commons/jexl3/JexlScript.java +++ b/src/main/java/org/apache/commons/jexl3/JexlScript.java @@ -94,7 +94,7 @@ public interface JexlScript { /** * Gets this script unbound parameters. - * <p>Parameters that haven't been bound by a previous call to curry(). + * <p>Parameters that haven't been bound by a previous call to curry().</p> * @return the parameters or null * @since 3.2 */ diff --git a/src/main/java/org/apache/commons/jexl3/ObjectContext.java b/src/main/java/org/apache/commons/jexl3/ObjectContext.java index 05f89c9..9217935 100644 --- a/src/main/java/org/apache/commons/jexl3/ObjectContext.java +++ b/src/main/java/org/apache/commons/jexl3/ObjectContext.java @@ -66,7 +66,7 @@ public class ObjectContext<T> implements JexlContext, JexlContext.NamespaceResol return jget.invoke(object); } catch (Exception xany) { if (jexl.isStrict()) { - throw new JexlException.Property(null, name, xany); + throw new JexlException.Property(null, name, true, xany); } } } @@ -82,7 +82,7 @@ public class ObjectContext<T> implements JexlContext, JexlContext.NamespaceResol } catch (Exception xany) { // ignore if (jexl.isStrict()) { - throw new JexlException.Property(null, name, xany); + throw new JexlException.Property(null, name, true, xany); } } } 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 51b25ee..259b8d1 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Interpreter.java @@ -24,6 +24,7 @@ import org.apache.commons.jexl3.JexlEngine; import org.apache.commons.jexl3.JexlException; import org.apache.commons.jexl3.JexlOperator; import org.apache.commons.jexl3.JexlScript; +import org.apache.commons.jexl3.JxltEngine; import org.apache.commons.jexl3.introspection.JexlMethod; import org.apache.commons.jexl3.introspection.JexlPropertyGet; import org.apache.commons.jexl3.introspection.JexlPropertySet; @@ -110,7 +111,6 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; -import org.apache.commons.jexl3.JxltEngine; /** @@ -1000,7 +1000,7 @@ public class Interpreter extends InterpreterBase { } catch (JxltEngine.Exception xjxlt) { cause = xjxlt; } - return node.isSafe() ? null : unsolvableProperty(node, src, cause); + return node.isSafe() ? null : unsolvableProperty(node, src, true, cause); } else { return node.getIdentifier(); } @@ -1095,11 +1095,15 @@ 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()? null : unsolvableVariable(node, ant.toString(), undefined); + return node.isSafeLhs() + ? null + : unsolvableVariable(node, ant.toString(), undefined); } if (ptyNode != null) { // am I the left-hand side of a safe op ? - return ptyNode.isSafeLhs()? null : unsolvableProperty(node, ptyNode.toString(), null); + return ptyNode.isSafeLhs() + ? null + : unsolvableProperty(node, stringifyProperty(ptyNode), false, null); } } return object; @@ -1291,11 +1295,11 @@ public class Interpreter extends InterpreterBase { } if (property == null) { // no property, we fail - return unsolvableProperty(propertyNode, "<?>.<null>", null); + return unsolvableProperty(propertyNode, "<?>.<null>", true, null); } if (object == null) { // no object, we fail - return unsolvableProperty(objectNode, "<null>.<?>", null); + return unsolvableProperty(objectNode, "<null>.<?>", true, null); } // 3: one before last, assign if (assignop != null) { @@ -1678,7 +1682,7 @@ public class Interpreter extends InterpreterBase { return null; } else { String attrStr = attribute != null ? attribute.toString() : null; - return unsolvableProperty(node, attrStr, xcause); + return unsolvableProperty(node, attrStr, true, xcause); } } else { // direct call @@ -1753,7 +1757,7 @@ public class Interpreter extends InterpreterBase { // lets fail if (node != null) { String attrStr = attribute != null ? attribute.toString() : null; - unsolvableProperty(node, attrStr, xcause); + unsolvableProperty(node, attrStr, true, xcause); } else { // direct call String error = "unable to set object property" 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 865ba7e..6658594 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java +++ b/src/main/java/org/apache/commons/jexl3/internal/InterpreterBase.java @@ -24,6 +24,8 @@ import org.apache.commons.jexl3.JexlException; 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.ASTMethodNode; import org.apache.commons.jexl3.parser.JexlNode; import org.apache.commons.jexl3.parser.ParserVisitor; @@ -211,18 +213,45 @@ public abstract class InterpreterBase extends ParserVisitor { /** * Triggered when a property can not be resolved. * @param node the node where the error originated from - * @param var the property name + * @param property the property node * @param cause the cause if any + * @param undef whether the property is undefined or null * @return throws JexlException if strict and not silent, null otherwise */ - protected Object unsolvableProperty(JexlNode node, String var, Throwable cause) { + protected Object unsolvableProperty(JexlNode node, String property, boolean undef, Throwable cause) { if (isStrictEngine()) { - throw new JexlException.Property(node, var, cause); + throw new JexlException.Property(node, property, undef, cause); } else if (logger.isDebugEnabled()) { - logger.debug(JexlException.propertyError(node, var), cause); + logger.debug(JexlException.propertyError(node, property, undef)); } return null; } + + /** + * Pretty-prints a failing property (de)reference. + * <p>Used by calls to unsolvableProperty(...).</p> + * @param node the property node + * @return the (pretty) string + */ + protected String stringifyProperty(JexlNode node) { + if (node instanceof ASTArrayAccess) { + if (node.jjtGetChild(0) != null) { + return "[" + + node.jjtGetChild(0).toString() + + "]"; + } + return "[???]"; + } + if (node instanceof ASTMethodNode) { + if (node.jjtGetChild(0) != null) { + return "." + + node.jjtGetChild(0).toString() + + "(...)"; + } + return ".???(...)"; + } + return node.toString(); + } /** * Triggered when an operator fails. diff --git a/src/main/java/org/apache/commons/jexl3/internal/Scope.java b/src/main/java/org/apache/commons/jexl3/internal/Scope.java index 245f38c..a0d52ab 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Scope.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Scope.java @@ -243,6 +243,12 @@ public final class Scope { public String[] getParameters() { return getParameters(0); } + + /** + * Gets this script parameters. + * @param bound number of known bound parameters (curry) + * @return the parameter names + */ protected String[] getParameters(int bound) { int unbound = parms - bound; if (namedVariables != null && unbound > 0) { 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 9009954..cd4706d 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java +++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java @@ -219,6 +219,9 @@ 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) { + rsibling = rsibling.jjtGetChild(0); + } if (rsibling instanceof ASTIdentifierAccess && ((ASTIdentifierAccess) rsibling).isSafe()) { return true; } diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml index c320979..dea7c8e 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="fix" issue="JEXL-272"> + Dereferencing null property not reported on method call + </action> <action dev="henrib" type="fix" issue="JEXL-271" due-to="Dmitri Blinov"> Hoisted variable is lost when currying lambda </action> diff --git a/src/test/java/org/apache/commons/jexl3/Issues200Test.java b/src/test/java/org/apache/commons/jexl3/Issues200Test.java index 184b371..ff44930 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues200Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues200Test.java @@ -652,4 +652,87 @@ public class Issues200Test extends JexlTestCase { result = script.execute(ctxt); 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/JexlTestCase.java b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java index f50a179..666b706 100644 --- a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java +++ b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java @@ -56,7 +56,11 @@ public class JexlTestCase { public void tearDown() throws Exception { debuggerCheck(JEXL); } - + + static JexlEngine createEngine() { + return new JexlBuilder().create(); + } + public static JexlEngine createEngine(boolean lenient) { return new JexlBuilder().arithmetic(new JexlArithmetic(!lenient)).cache(512).create(); } diff --git a/src/test/java/org/apache/commons/jexl3/LambdaTest.java b/src/test/java/org/apache/commons/jexl3/LambdaTest.java index 353cbc2..78bb49d 100644 --- a/src/test/java/org/apache/commons/jexl3/LambdaTest.java +++ b/src/test/java/org/apache/commons/jexl3/LambdaTest.java @@ -16,7 +16,6 @@ */ package org.apache.commons.jexl3; -import org.apache.commons.jexl3.internal.Engine; import java.util.List; import java.util.Set; import java.util.concurrent.Callable; @@ -35,7 +34,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testScriptArguments() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript s = jexl.createScript(" x + x ", "x"); JexlScript s42 = jexl.createScript("s(21)", "s"); Object result = s42.execute(null, s); @@ -44,7 +43,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testScriptContext() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript s = jexl.createScript("function(x) { x + x }"); String fsstr = s.getParsedText(0); Assert.assertEquals("(x)->{ x + x; }", fsstr); @@ -63,7 +62,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testLambda() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); String strs = "var s = function(x) { x + x }; s(21)"; JexlScript s42 = jexl.createScript(strs); Object result = s42.execute(null); @@ -76,7 +75,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testLambdaClosure() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); String strs = "var t = 20; var s = function(x, y) { x + y + t}; s(15, 7)"; JexlScript s42 = jexl.createScript(strs); Object result = s42.execute(null); @@ -97,7 +96,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testLambdaLambda() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); String strs = "var t = 19; ( (x, y)->{ var t = 20; x + y + t} )(15, 7);"; JexlScript s42 = jexl.createScript(strs); Object result = s42.execute(null); @@ -116,7 +115,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testNestLambda() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); String strs = "( (x)->{ (y)->{ x + y } })(15)(27)"; JexlScript s42 = jexl.createScript(strs); Object result = s42.execute(null); @@ -125,7 +124,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testNestLambada() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlContext ctx = null; String strs = "(x)->{ (y)->{ x + y } }"; JexlScript s42 = jexl.createScript(strs); @@ -148,7 +147,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testHoistLambada() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlContext ctx = null; JexlScript s42; Object result; @@ -183,7 +182,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testRecurse() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlContext jc = new MapContext(); try { JexlScript script = jexl.createScript("var fact = (x)->{ if (x <= 1) 1; else x * fact(x - 1) }; fact(5)"); @@ -197,7 +196,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testRecurse2() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlContext jc = new MapContext(); // adding some hoisted vars to get it confused try { @@ -214,7 +213,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testRecurse3() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlContext jc = new MapContext(); // adding some hoisted vars to get it confused try { @@ -231,7 +230,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testIdentity() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript script; Object result; @@ -243,7 +242,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testCurry1() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript script; Object result; String[] parms; @@ -266,7 +265,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testCurry2() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript script; Object result; String[] parms; @@ -282,7 +281,7 @@ public class LambdaTest extends JexlTestCase { @Test public void testCurry3() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript script; Object result; @@ -294,7 +293,7 @@ public class LambdaTest extends JexlTestCase { @Test public void test270() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript base = jexl.createScript("(x, y, z)->{ x + y + z }"); String text = base.toString(); JexlScript script = base.curry(5, 15); @@ -313,7 +312,7 @@ public class LambdaTest extends JexlTestCase { @Test public void test271a() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript base = jexl.createScript("var base = 1; var x = (a)->{ var y = (b) -> {base + b}; return base + y(a)}; x(40)"); Object result = base.execute(null); Assert.assertEquals(42, result); @@ -321,7 +320,7 @@ public class LambdaTest extends JexlTestCase { @Test public void test271b() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript base = jexl.createScript("var base = 2; var sum = (x, y, z)->{ base + x + y + z }; var y = sum.curry(1); y(2,3)"); Object result = base.execute(null); Assert.assertEquals(8, result); @@ -329,7 +328,7 @@ public class LambdaTest extends JexlTestCase { @Test public void test271c() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript base = jexl.createScript("(x, y, z)->{ 2 + x + y + z };"); JexlScript y = base.curry(1); Object result = y.execute(null, 2, 3); @@ -338,7 +337,7 @@ public class LambdaTest extends JexlTestCase { @Test public void test271d() throws Exception { - JexlEngine jexl = new Engine(); + JexlEngine jexl = createEngine(); JexlScript base = jexl.createScript("var base = 2; return (x, y, z)->{ base + x + y + z };"); JexlScript y = ((JexlScript) base.execute(null)).curry(1); Object result = y.execute(null, 2, 3);