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 2020/02/14 22:13:06 UTC
[commons-jexl] branch master updated: JEXL-302: added a capture
mode to further refine how getVariable should behave wrt constants in array
references Task #JEXL-302 - JexlScript.getVariables returns strange values
for array access
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 8dee2ad JEXL-302: added a capture mode to further refine how getVariable should behave wrt constants in array references Task #JEXL-302 - JexlScript.getVariables returns strange values for array access
8dee2ad is described below
commit 8dee2ad6abe88213625dc126cf811d43925c6317
Author: henrib <he...@apache.org>
AuthorDate: Fri Feb 14 23:12:16 2020 +0100
JEXL-302: added a capture mode to further refine how getVariable should behave wrt constants in array references
Task #JEXL-302 - JexlScript.getVariables returns strange values for array access
---
.../java/org/apache/commons/jexl3/JexlBuilder.java | 35 ++++++++++++++++++----
.../org/apache/commons/jexl3/internal/Engine.java | 29 +++++++++++-------
.../java/org/apache/commons/jexl3/LexicalTest.java | 9 ++----
.../java/org/apache/commons/jexl3/VarTest.java | 33 ++++++++++++++++----
4 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
index 744dfe1..c244ad0 100644
--- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
@@ -83,7 +83,7 @@ public class JexlBuilder {
private final JexlOptions options = new JexlOptions();
/** Whether getVariables considers all potential equivalent syntactic forms. */
- private Boolean collectAll = null;
+ private int collectMode = 1;
/** The map of 'prefix:function' to object implementing the namespaces. */
private Map<String, Object> namespaces = null;
@@ -281,6 +281,7 @@ public class JexlBuilder {
*
* @param flag true means lexical function scope is in effect, false implies non-lexical scoping
* @return this builder
+ * @since 3.2
*/
public JexlBuilder lexical(boolean flag) {
options.setLexical(flag);
@@ -297,6 +298,7 @@ public class JexlBuilder {
*
* @param flag true means lexical shading is in effect, false implies no lexical shading
* @return this builder
+ * @since 3.2
*/
public JexlBuilder lexicalShade(boolean flag) {
options.setLexicalShade(flag);
@@ -403,15 +405,38 @@ public class JexlBuilder {
*
* @param flag true means var collections considers constant array accesses equivalent to dotted references
* @return this builder
+ * @since 3.2
*/
public JexlBuilder collectAll(boolean flag) {
- this.collectAll = flag;
+ return collectMode(flag? 1 : 0);
+ }
+
+ /**
+ * Experimental collector mode setter.
+ *
+ * @param mode 0 or 1 as equivalents to false and true, other values are experimental
+ * @return this builder
+ * @since 3.2
+ */
+ public JexlBuilder collectMode(int mode) {
+ this.collectMode = mode;
return this;
+ }
+
+ /**
+ * @return true if variable collection follows strict syntactic rule
+ * @since 3.2
+ */
+ public boolean collectAll() {
+ return this.collectMode != 0;
}
- /** @return true if collect all, false otherwise */
- public Boolean collectAll() {
- return this.collectAll;
+ /**
+ * @return 0 if variable collection follows strict syntactic rule
+ * @since 3.2
+ */
+ public int collectMode() {
+ return this.collectMode;
}
/**
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 48d9897..20da2a0 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
@@ -49,6 +49,8 @@ import java.util.Set;
import java.nio.charset.Charset;
import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.commons.jexl3.parser.ASTNumberLiteral;
+import org.apache.commons.jexl3.parser.ASTStringLiteral;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
@@ -156,7 +158,7 @@ public class Engine extends JexlEngine {
/**
* Collect all or only dot references.
*/
- protected final boolean collectAll;
+ protected final int collectMode;
/**
* A cached version of the options.
*/
@@ -182,7 +184,7 @@ public class Engine extends JexlEngine {
this.cancellable = option(conf.cancellable(), !silent && strict);
options.setCancellable(cancellable);
this.debug = option(conf.debug(), true);
- this.collectAll = option(conf.collectAll(), true);
+ this.collectMode = conf.collectMode();
this.stackOverflow = conf.stackOverflow() > 0? conf.stackOverflow() : Integer.MAX_VALUE;
// core properties:
JexlUberspect uber = conf.uberspect() == null ? getUberspect(conf.logger(), conf.strategy()) : conf.uberspect();
@@ -578,7 +580,7 @@ public class Engine extends JexlEngine {
* @return a collector instance
*/
protected VarCollector varCollector() {
- return new VarCollector(this.collectAll);
+ return new VarCollector(this.collectMode);
}
/**
@@ -598,16 +600,18 @@ public class Engine extends JexlEngine {
*/
private JexlNode root = null;
/**
- * Whether constant array-access is considered equivalent to dot-access.
+ * Whether constant array-access is considered equivalent to dot-access;
+ * if so, > 1 means collect any constant (set,map,...) instead of just
+ * strings and numbers.
*/
- private boolean semantic = true;
+ private int mode = 1;
/**
* Constructor.
* @param constaa whether constant array-access is considered equivalent to dot-access
*/
- protected VarCollector(boolean constaa) {
- semantic = constaa;
+ protected VarCollector(int constaa) {
+ mode = constaa;
}
/**
@@ -680,15 +684,20 @@ public class Engine extends JexlEngine {
if (collector.isCollecting()) {
collector.add(((ASTIdentifierAccess) node).getName());
}
- } else if (node instanceof ASTArrayAccess && collector.semantic) {
+ } else if (node instanceof ASTArrayAccess && collector.mode > 0) {
int num = node.jjtGetNumChildren();
// collect only if array access is const and follows an identifier
boolean collecting = collector.isCollecting();
for (int i = 0; i < num; ++i) {
JexlNode child = node.jjtGetChild(i);
if (collecting && child.isConstant()) {
- String image = child.toString();
- collector.add(image);
+ // collect all constants or only string and number literals
+ boolean collect = collector.mode > 1
+ || (child instanceof ASTStringLiteral || child instanceof ASTNumberLiteral);
+ if (collect) {
+ String image = child.toString();
+ collector.add(image);
+ }
} else {
collecting = false;
collector.collect(null);
diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
index 0e5ea2b..ff0b128 100644
--- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
@@ -735,7 +735,7 @@ public class LexicalTest {
JexlScript script = jexl.createScript("@scale(13) @test var i = 42");
JexlContext jc = new OptAnnotationContext();
Object result = script.execute(jc);
- Assert.assertEquals(result, 42);
+ Assert.assertEquals(42, result);
}
@Test
@@ -755,22 +755,19 @@ public class LexicalTest {
f.lexical(true);
JexlEngine jexl = new JexlBuilder().strict(true).features(f).create();
JexlScript script = jexl.createScript(
- "var x = 10;"
- + "var a = function(var b) {for (var q : 1 ..10) {return x + b}}; a(32)");
+ "var x = 10; (b->{ x + b })(32)");
JexlContext jc = new MapContext();
Object result = script.execute(jc);
Assert.assertEquals(42, result);
}
-
@Test
public void testCaptured1() throws Exception {
JexlFeatures f = new JexlFeatures();
f.lexical(true);
JexlEngine jexl = new JexlBuilder().strict(true).features(f).create();
JexlScript script = jexl.createScript(
- "{var x = 10; }"
- + "var a = function(var b) {for (var q : 1 ..10) {return x + b}}; a(32)");
+ "{var x = 10; } (b->{ x + b })(32)");
JexlContext jc = new MapContext();
jc.set("x", 11);
Object result = script.execute(jc);
diff --git a/src/test/java/org/apache/commons/jexl3/VarTest.java b/src/test/java/org/apache/commons/jexl3/VarTest.java
index 04d4859..c1e0a17 100644
--- a/src/test/java/org/apache/commons/jexl3/VarTest.java
+++ b/src/test/java/org/apache/commons/jexl3/VarTest.java
@@ -499,6 +499,7 @@ public class VarTest extends JexlTestCase {
@Test
public void testReferenceLiteral() throws Exception {
+ JexlEngine jexld = new JexlBuilder().collectMode(2).create();
JexlScript script;
List<String> result;
Set<List<String>> vars;
@@ -508,7 +509,7 @@ public class VarTest extends JexlTestCase {
//d.yyyy = 1969; d.MM = 7; d.dd = 20
ctxt.set("moon.landing", new VarDate("1969-07-20"));
- script = JEXL.createScript("moon.landing[['yyyy', 'MM', 'dd']]");
+ script = jexld.createScript("moon.landing[['yyyy', 'MM', 'dd']]");
result = (List<String>) script.execute(ctxt);
Assert.assertEquals(Arrays.asList("1969", "7", "20"), result);
@@ -519,7 +520,7 @@ public class VarTest extends JexlTestCase {
Assert.assertEquals("landing", var.get(1));
Assert.assertArrayEquals(new String[]{"yyyy", "MM", "dd"}, readIdentifiers(var.get(2)));
- script = JEXL.createScript("moon.landing[ { 'yyyy' : 'year', 'MM' : 'month', 'dd' : 'day' } ]");
+ script = jexld.createScript("moon.landing[ { 'yyyy' : 'year', 'MM' : 'month', 'dd' : 'day' } ]");
Map<String, String> mapr = (Map<String, String>) script.execute(ctxt);
Assert.assertEquals(3, mapr.size());
Assert.assertEquals("1969", mapr.get("year"));
@@ -536,22 +537,32 @@ public class VarTest extends JexlTestCase {
@Test
public void testLiteral() throws Exception {
- JexlScript e = JEXL.createScript("x.y[['z', 't']]");
+ JexlBuilder builder = new JexlBuilder().collectMode(2);
+ Assert.assertEquals(2, builder.collectMode());
+ Assert.assertTrue(builder.collectAll());
+
+ JexlEngine jexld = builder.create();
+ JexlScript e = jexld.createScript("x.y[['z', 't']]");
Set<List<String>> vars = e.getVariables();
Assert.assertEquals(1, vars.size());
Assert.assertTrue(eq(mkref(new String[][]{{"x", "y", "[ 'z', 't' ]"}}), vars));
- e = JEXL.createScript("x.y[{'z': 't'}]");
+ e = jexld.createScript("x.y[{'z': 't'}]");
vars = e.getVariables();
Assert.assertEquals(1, vars.size());
Assert.assertTrue(eq(mkref(new String[][]{{"x", "y", "{ 'z' : 't' }"}}), vars));
- e = JEXL.createScript("x.y.'{ \\'z\\' : \\'t\\' }'");
+ e = jexld.createScript("x.y.'{ \\'z\\' : \\'t\\' }'");
vars = e.getVariables();
Assert.assertEquals(1, vars.size());
Assert.assertTrue(eq(mkref(new String[][]{{"x", "y", "{ 'z' : 't' }"}}), vars));
- JexlEngine jexld = new JexlBuilder().collectAll(false).create();
+ // only string or number literals
+ builder = builder.collectAll(true);
+ Assert.assertEquals(1, builder.collectMode());
+ Assert.assertTrue(builder.collectAll());
+
+ jexld = builder.create();
e = jexld.createScript("x.y[{'z': 't'}]");
vars = e.getVariables();
Assert.assertEquals(1, vars.size());
@@ -561,6 +572,16 @@ public class VarTest extends JexlTestCase {
vars = e.getVariables();
Assert.assertEquals(1, vars.size());
Assert.assertTrue(eq(mkref(new String[][]{{"x", "y"}}), vars));
+
+ e = jexld.createScript("x.y['z']");
+ vars = e.getVariables();
+ Assert.assertEquals(1, vars.size());
+ Assert.assertTrue(eq(mkref(new String[][]{{"x", "y", "z"}}), vars));
+
+ e = jexld.createScript("x.y[42]");
+ vars = e.getVariables();
+ Assert.assertEquals(1, vars.size());
+ Assert.assertTrue(eq(mkref(new String[][]{{"x", "y", "42"}}), vars));
}
@Test