You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2022/11/08 12:46:19 UTC
[commons-jexl] 01/03: JEXL-384: enforce strictness of operators by checking null arguments in all operator calls;
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 913608a807f296f4501f1e1be0d3bcd45f03ab27
Author: henrib <he...@apache.org>
AuthorDate: Fri Nov 4 17:10:27 2022 +0100
JEXL-384: enforce strictness of operators by checking null arguments in all operator calls;
---
.../org/apache/commons/jexl3/JexlArithmetic.java | 13 +-
.../apache/commons/jexl3/internal/Operators.java | 30 +++-
.../org/apache/commons/jexl3/Issues300Test.java | 178 +++++++++++++++++++--
.../java/org/apache/commons/jexl3/JexlTest.java | 16 +-
4 files changed, 211 insertions(+), 26 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
index 4610d227..e813151a 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlArithmetic.java
@@ -379,7 +379,8 @@ public class JexlArithmetic {
* <p>When an operator is strict, it does <em>not</em> accept null arguments when the arithmetic is strict.
* 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.</p>
+ * <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>
* @param operator the operator to check for null-argument(s) handling
@@ -387,7 +388,15 @@ public class JexlArithmetic {
* for null argument(s)
*/
public boolean isStrict(JexlOperator operator) {
- return operator == JexlOperator.EQ? false : isStrict();
+ switch(operator) {
+ case EQ:
+ case ARRAY_GET:
+ case ARRAY_SET:
+ case PROPERTY_GET:
+ case PROPERTY_SET:
+ return false;
+ }
+ return isStrict();
}
/**
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Operators.java b/src/main/java/org/apache/commons/jexl3/internal/Operators.java
index f0da705e..b81d1540 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Operators.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Operators.java
@@ -88,18 +88,44 @@ public class Operators {
return false;
}
+ /**
+ * Throw a NPE if operator is strict and one of the arguments is null.
+ * @param arithmetic the JEXL arithmetic instance
+ * @param operator the operator to check
+ * @param args the operands
+ * @throws JexlArithmetic.NullOperand if operator is strict and an operand is null
+ */
+ protected void controlNullOperands(JexlArithmetic arithmetic, JexlOperator operator, Object...args) {
+ for (Object arg : args) {
+ // only check operator if necessary
+ if (arg == null) {
+ // check operator only once if it is not strict
+ if (arithmetic.isStrict(operator)) {
+ throw new JexlArithmetic.NullOperand();
+ } else {
+ break;
+ }
+ }
+ }
+ }
+
/**
* Attempts to call an operator.
* <p>
- * This takes care of finding and caching the operator method when appropriate
+ * This performs the null argument control against the strictness of the operator.
+ * </p>
+ * <p>
+ * This takes care of finding and caching the operator method when appropriate.
+ * </p>
* @param node the syntactic node
* @param operator the operator
* @param args the arguments
* @return the result of the operator evaluation or TRY_FAILED
*/
protected Object tryOverload(final JexlNode node, final JexlOperator operator, Object... args) {
+ final JexlArithmetic arithmetic = interpreter.arithmetic;
+ controlNullOperands(arithmetic, operator, args);
if (operators != null && operators.overloads(operator)) {
- final JexlArithmetic arithmetic = interpreter.arithmetic;
final boolean cache = interpreter.cache;
try {
if (cache) {
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
index 78a799e7..f80ac72d 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -41,7 +41,7 @@ import static org.junit.Assert.assertEquals;
*/
public class Issues300Test {
@Test
- public void testIssue301a() {
+ public void test301a() {
final JexlEngine jexl = new JexlBuilder().safe(false).arithmetic(new JexlArithmetic(false)).create();
final String[] srcs = new String[]{
"var x = null; x.0", "var x = null; x[0]", "var x = [null,1]; x[0][0]"
@@ -61,7 +61,7 @@ public class Issues300Test {
}
@Test
- public void testIssues301b() {
+ public void tests301b() {
final JexlEngine jexl = new JexlBuilder().safe(false).arithmetic(new JexlArithmetic(false)).create();
final Object[] xs = new Object[]{null, null, new Object[]{null, 1}};
final String[] srcs = new String[]{
@@ -82,7 +82,7 @@ public class Issues300Test {
}
@Test
- public void testIssue302() {
+ public void test302() {
final JexlContext jc = new MapContext();
final String[] strs = new String[]{
"{if (0) 1 else 2; var x = 4;}",
@@ -100,7 +100,7 @@ public class Issues300Test {
}
@Test
- public void testIssue304() {
+ public void test304() {
final JexlEngine jexlEngine = new JexlBuilder().strict(false).create();
JexlExpression e304 = jexlEngine.createExpression("overview.limit.var");
@@ -150,7 +150,7 @@ public class Issues300Test {
}
@Test
- public void testIssue305() {
+ public void test305() {
final JexlEngine jexl = new JexlBuilder().create();
JexlScript e;
e = jexl.createScript("{while(false) {}; var x = 1;}");
@@ -162,7 +162,7 @@ public class Issues300Test {
}
@Test
- public void testIssue306() {
+ public void test306() {
final JexlContext ctxt = new MapContext();
final JexlEngine jexl = new JexlBuilder().create();
final JexlScript e = jexl.createScript("x.y ?: 2");
@@ -174,7 +174,7 @@ public class Issues300Test {
}
@Test
- public void testIssue306a() {
+ public void test306a() {
final JexlEngine jexl = new JexlBuilder().create();
final JexlScript e = jexl.createScript("x.y ?: 2", "x");
Object o = e.execute(null, new Object());
@@ -184,7 +184,7 @@ public class Issues300Test {
}
@Test
- public void testIssue306b() {
+ public void test306b() {
final JexlEngine jexl = new JexlBuilder().create();
final JexlScript e = jexl.createScript("x?.y ?: 2", "x");
final Object o1 = e.execute(null, new Object());
@@ -194,7 +194,7 @@ public class Issues300Test {
}
@Test
- public void testIssue306c() {
+ public void test306c() {
final JexlEngine jexl = new JexlBuilder().safe(true).create();
final JexlScript e = jexl.createScript("x.y ?: 2", "x");
Object o = e.execute(null, new Object());
@@ -204,7 +204,7 @@ public class Issues300Test {
}
@Test
- public void testIssue306d() {
+ public void test306d() {
final JexlEngine jexl = new JexlBuilder().safe(true).create();
final JexlScript e = jexl.createScript("x.y[z.t] ?: 2", "x");
Object o = e.execute(null, new Object());
@@ -214,7 +214,7 @@ public class Issues300Test {
}
@Test
- public void testIssue309a() {
+ public void test309a() {
final String src = "<html lang=\"en\">\n"
+ " <body>\n"
+ " <h1>Hello World!</h1>\n"
@@ -233,7 +233,7 @@ public class Issues300Test {
}
@Test
- public void testIssue309b() {
+ public void test309b() {
final String src = "<html lang=\"en\">\n"
+ " <body>\n"
+ " <h1>Hello World!</h1>\n"
@@ -252,7 +252,7 @@ public class Issues300Test {
}
@Test
- public void testIssue309c() {
+ public void test309c() {
final String src = "<html lang=\"en\">\n"
+ " <body>\n"
+ " <h1>Hello World!</h1>\n"
@@ -715,7 +715,7 @@ public class Issues300Test {
@Test
public void test361b_32() {
- JexlEngine jexl = new Engine32(new JexlBuilder().safe(false));
+ JexlEngine jexl = new Engine32(new JexlBuilder().safe(false).strict(false));
Object result = run361b(jexl);
Assert.assertNotNull(result);
}
@@ -748,7 +748,7 @@ public class Issues300Test {
@Test
public void test361c_32() {
- JexlEngine jexl = new Engine32(new JexlBuilder().safe(false));
+ JexlEngine jexl = new Engine32(new JexlBuilder().safe(false).strict(false));
String result = run361c(jexl);
Assert.assertNotNull(result);
}
@@ -968,7 +968,7 @@ public class Issues300Test {
public static class Context0930 extends MapContext {
/**
- * This allows using a JEXL lmabda as a filter.
+ * This allows using a JEXL lambda as a filter.
* @param stream the stream
* @param filter the lambda to use as filter
* @return the filtered stream
@@ -979,7 +979,7 @@ public class Issues300Test {
}
@Test
- public void testStackOvflw20220930() {
+ public void testSO20220930() {
// fill some drivers in a list
List<Driver0930> values = new ArrayList<>();
for(int i = 0; i < 8; ++i) {
@@ -1010,4 +1010,148 @@ public class Issues300Test {
}
}
}
+
+ public static class Arithmetic383 extends JexlArithmetic {
+ public Arithmetic383(boolean astrict) {
+ 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;
+ }
+ return super.isStrict(op);
+ }
+ }
+
+ @Test public void test383() {
+ JexlEngine jexl = new JexlBuilder().safe(false).arithmetic(new Arithmetic383(true)).create();
+ String src0 = "if (a) 1; else 2;";
+ String src1 = "if (!a) 1; else 2;";
+ // local var
+ JexlScript s0 = jexl.createScript(src0, "a");
+ JexlScript s1 = jexl.createScript(src1, "a");
+ Assert.assertEquals(2, s0.execute(null, null));
+ Assert.assertEquals(1, s1.execute(null, null));
+ // global var undefined
+ s0 = jexl.createScript(src0);
+ s1 = jexl.createScript(src1);
+ try {
+ Assert.assertEquals(2, s0.execute(null, null));
+ } catch(JexlException.Variable xvar) {
+ Assert.assertEquals("a", xvar.getVariable());
+ }
+ try {
+ Assert.assertEquals(1, s1.execute(null, null));
+ } catch(JexlException.Variable xvar) {
+ Assert.assertEquals("a", xvar.getVariable());
+ }
+ // global var null
+ MapContext ctxt = new MapContext();
+ ctxt.set("a", null);
+ Assert.assertEquals(2, s0.execute(ctxt, null));
+ Assert.assertEquals(1, s1.execute(ctxt, null));
+ }
+
+ public static class Arithmetic384 extends JexlArithmetic {
+ public Arithmetic384(boolean astrict) {
+ super(astrict);
+ }
+
+ @Override
+ public boolean isStrict(JexlOperator op) {
+ if (JexlOperator.ADD == op) {
+ return false;
+ }
+ return super.isStrict(op);
+ }
+ }
+ @Test
+ public void test384a() {
+ JexlEngine jexl = new JexlBuilder()
+ .safe(false)
+ .strict(true)
+ .create();
+ // constant
+ for(String src0 : Arrays.asList("'ABC' + null", "null + 'ABC'")) {
+ JexlContext ctxt = new MapContext();
+ JexlScript s0 = jexl.createScript(src0);
+ try {
+ s0.execute(ctxt, null);
+ Assert.fail("null argument should throw");
+ } catch (JexlException xvar) {
+ Assert.assertTrue(xvar.toString().contains("+"));
+ }
+ }
+ // null local a
+ for(String src1 : Arrays.asList("'ABC' + a", "a + 'ABC'")) {
+ JexlContext ctxt = new MapContext();
+ JexlScript s1 = jexl.createScript(src1, "a");
+ try {
+ s1.execute(ctxt, null);
+ Assert.fail("null argument should throw");
+ } catch (JexlException.Variable xvar) {
+ Assert.assertEquals("a", xvar.getVariable());
+ }
+ // undefined a
+ s1 = jexl.createScript(src1);
+ try {
+ s1.execute(ctxt, null);
+ Assert.fail("null argument should throw");
+ } catch (JexlException.Variable xvar) {
+ Assert.assertEquals("a", xvar.getVariable());
+ Assert.assertTrue(xvar.isUndefined());
+ }
+ // null a
+ ctxt.set("a", null);
+ try {
+ s1.execute(ctxt, null);
+ Assert.fail("null argument should throw");
+ } catch (JexlException.Variable xvar) {
+ Assert.assertEquals("a", xvar.getVariable());
+ Assert.assertFalse(xvar.isUndefined());
+ }
+ }
+ }
+ @Test
+ public void test384b() {
+ // be explicit about + handling null
+ JexlEngine jexl = new JexlBuilder()
+ .arithmetic(new Arithmetic384(true))
+ .safe(false)
+ .strict(true)
+ .create();
+ // constant
+ for(String src0 : Arrays.asList("'ABC' + null", "null + 'ABC'")) {
+ JexlContext ctxt = new MapContext();
+ JexlScript s0 = jexl.createScript(src0);
+ Assert.assertEquals("ABC", s0.execute(ctxt));
+ }
+ // null local a
+ for(String src1 : Arrays.asList("'ABC' + a", "a + 'ABC'")) {
+ JexlContext ctxt = new MapContext();
+ JexlScript s1 = jexl.createScript(src1, "a");
+ Assert.assertEquals("ABC", s1.execute(ctxt, null));
+ // undefined a
+ s1 = jexl.createScript(src1);
+ try {
+ s1.execute(ctxt, null);
+ Assert.fail("null argument should throw");
+ } catch (JexlException.Variable xvar) {
+ Assert.assertEquals("a", xvar.getVariable());
+ Assert.assertTrue(xvar.isUndefined());
+ }
+ // null a
+ ctxt.set("a", null);
+ Assert.assertEquals("ABC", s1.execute(ctxt, null));
+ }
+ }
}
diff --git a/src/test/java/org/apache/commons/jexl3/JexlTest.java b/src/test/java/org/apache/commons/jexl3/JexlTest.java
index dcde99c3..11c11f2d 100644
--- a/src/test/java/org/apache/commons/jexl3/JexlTest.java
+++ b/src/test/java/org/apache/commons/jexl3/JexlTest.java
@@ -19,6 +19,7 @@ package org.apache.commons.jexl3;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.BitSet;
import java.util.Calendar;
import java.util.GregorianCalendar;
@@ -39,12 +40,17 @@ import org.junit.Test;
* @since 1.0
*/
@SuppressWarnings({"UnnecessaryBoxing", "AssertEqualsBetweenInconvertibleTypes"})
-public class JexlTest extends JexlTestCase {
- protected static final String METHOD_STRING = "Method string";
- protected static final String GET_METHOD_STRING = "GetMethod string";
+public final class JexlTest extends JexlTestCase {
+ static final String METHOD_STRING = "Method string";
+ static final String GET_METHOD_STRING = "GetMethod string";
public JexlTest() {
- super("JexlTest");
+ super("JexlTest",
+ new JexlBuilder()
+ .strict(false)
+ .imports(Arrays.asList("java.lang","java.math"))
+ .permissions(null)
+ .cache(128).create());
}
/**
@@ -158,7 +164,7 @@ public class JexlTest extends JexlTestCase {
options.setStrict(false);
jc.set("string", "");
jc.set("array", new Object[0]);
- jc.set("map", new HashMap<Object, Object>());
+ jc.set("map", new HashMap<>());
jc.set("list", new ArrayList<Object>());
jc.set("set", (new HashMap<Object, Object>()).keySet());
jc.set("longstring", "thingthing");