Updated jexl parsing to disable some features which we aren't using t… (

#2299) * Updated jexl parsing to disable some features which we aren't using to improve parsing performance. * Updated to use custom configured JexlFeatures everywhere --------- Co-authored-by: hgklohr <[email protected]>
NationalSecurityAgency · Mar 11, 2024 · 25da5cf · 25da5cf
1 parent de2528c
commit 25da5cf
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 58 deletions.
diff --git a/warehouse/query-core/src/main/java/datawave/query/jexl/JexlASTHelper.java b/warehouse/query-core/src/main/java/datawave/query/jexl/JexlASTHelper.java
@@ -135,6 +135,54 @@ public static ASTJexlScript parseAndFlattenJexlQuery(String query) throws ParseE
         return TreeFlatteningRebuildingVisitor.flatten(script);
     }
 
+    public static JexlFeatures jexlFeatures() {
+        // @formatter:off
+        return new JexlFeatures()
+                // mostly used internally by Jexl
+                .register(false) // default false
+                // allow usage of let, const, and var variables
+                .localVar(false) // default true
+                // allow side-effect operators (e.g. +=, -=, |=, <<=, etc) for global vars.  needed to assign values (e.g. _Value_ = true)
+                .sideEffectGlobal(true) // default true
+                // allow side-effect operators (e.g. +=, -=, |=, <<=, etc).  needed to assign values (e.g. _Value_ = true)
+                .sideEffect(true) // default true
+                // allow array indexing via reference expression (e.g. array[some expression])
+                .arrayReferenceExpr(false) // default true
+                // allow method calls on expressions
+                .methodCall(true) // default true
+                // allow array/map/set literal expressions (e.g. [1, 2, "three"], {"one": 1, "two": 2}, {"one", 2, "more"}
+                .structuredLiteral(false) // default true
+                // allow creation of new instances
+                .newInstance(false) // default true
+                // allow loop constructs
+                .loops(false) // default true
+                // allow lambda/function constructs (not the same as namespaced functions)
+                .lambda(false) // default true
+                // allow thin-arrow lambda syntax (e.g. ->)
+                .thinArrow(false) // default true
+                // allow fat-arrow lambda syntax (e.g. =>)
+                .fatArrow(false) // default false
+                // allow comparator names (e.g. eq, ne, lt, gt, etc)
+                .comparatorNames(false) // default true
+                // allow pragma constructs (e.g. #pragma some.option some-value)
+                .pragma(false) // default true
+                // allow pragma constructs anywhere in the code
+                .pragmaAnywhere(false) // default true
+                // allow namespace pragmas (e.g. '#pragma jexl.namespace.str java.lang.String' to use 'str' in place of 'java.lang.String')
+                .namespacePragma(false) // default true
+                // allow import pragmas (e.g. '#pragma jexl.import java.net' to use new URL() instead of new java.net.URL())
+                .importPragma(false) // default true
+                // allow annotations
+                .annotation(false) // default true
+                // allow multiple semicolon-terminated expressions per jexl string
+                .script(false) // default true
+                // whether redefining local variables is an error
+                .lexical(false) // default false
+                // whether local variables shade global variables
+                .lexicalShade(false); // default false
+        // @formatter: on
+    }
+
     /**
      * Parse a query string using a JEXL parser and transform it into a parse tree of our RefactoredDatawaveTreeNodes. This also sets all convenience maps that
      * the analyzer provides.
@@ -163,7 +211,7 @@ public static ASTJexlScript parseJexlQuery(String query) throws ParseException {
         } else {
             // Parse the original query
             try {
-                return parser.parse(null, new JexlFeatures(), caseFixQuery, null);
+                return parser.parse(null, jexlFeatures(), caseFixQuery, null);
             } catch (TokenMgrException | JexlException e) {
                 throw new ParseException(e.getMessage());
             }
@@ -195,7 +243,7 @@ private static ASTJexlScript parseQueryWithBackslashes(String query, Parser pars
         // Parse the query with the placeholders
         ASTJexlScript jexlScript;
         try {
-            jexlScript = parser.parse(null, new JexlFeatures(), query, null);
+            jexlScript = parser.parse(null, jexlFeatures(), query, null);
         } catch (TokenMgrException e) {
             throw new ParseException(e.getMessage());
         }

diff --git a/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/PrintingVisitor.java b/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/PrintingVisitor.java
@@ -1,5 +1,7 @@
 package datawave.query.jexl.visitors;
 
+import static datawave.query.jexl.JexlASTHelper.jexlFeatures;
+
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import java.util.Collections;
@@ -193,7 +195,7 @@ public static void printQuery(String query) throws ParseException {
 
         // Parse the query
         try {
-            printQuery(parser.parse(null, new JexlFeatures(), query, null));
+            printQuery(parser.parse(null, jexlFeatures(), query, null));
         } catch (TokenMgrException e) {
             throw new ParseException(e.getMessage());
         }
@@ -254,7 +256,7 @@ public static String formattedQueryString(String query, int maxChildNodes) throw
 
         // Parse the query
         try {
-            return formattedQueryString(parser.parse(null, new JexlFeatures(), query, null), maxChildNodes);
+            return formattedQueryString(parser.parse(null, jexlFeatures(), query, null), maxChildNodes);
         } catch (TokenMgrException e) {
             throw new ParseException(e.getMessage());
         }

diff --git a/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/VariableNameVisitor.java b/warehouse/query-core/src/main/java/datawave/query/jexl/visitors/VariableNameVisitor.java
@@ -1,5 +1,6 @@
 package datawave.query.jexl.visitors;
 
+import static datawave.query.jexl.JexlASTHelper.jexlFeatures;
 import static datawave.query.jexl.nodes.QueryPropertyMarker.MarkerType.EXCEEDED_OR;
 
 import java.util.Set;
@@ -42,7 +43,7 @@ public static Set<String> parseQuery(String query) throws ParseException {
 
         // Parse the query
         try {
-            return parseQuery(parser.parse(null, new JexlFeatures(), query, null));
+            return parseQuery(parser.parse(null, jexlFeatures(), query, null));
         } catch (TokenMgrException e) {
             throw new ParseException(e.getMessage());
         }

diff --git a/warehouse/query-core/src/main/java/datawave/query/tables/edge/EdgeQueryLogic.java b/warehouse/query-core/src/main/java/datawave/query/tables/edge/EdgeQueryLogic.java
@@ -1,5 +1,7 @@
 package datawave.query.tables.edge;
 
+import static datawave.query.jexl.JexlASTHelper.jexlFeatures;
+
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.ObjectOutputStream;
@@ -348,7 +350,7 @@ protected QueryData configureRanges(String queryString) throws ParseException {
         Parser parser = new Parser(new StringProvider(";"));
         ASTJexlScript script;
         try {
-            script = parser.parse(null, new JexlFeatures(), queryString, null);
+            script = parser.parse(null, jexlFeatures(), queryString, null);
         } catch (Exception e) {
             throw new IllegalArgumentException("Invalid jexl supplied. " + e.getMessage());
         }

diff --git a/...ry-core/src/test/java/datawave/query/jexl/visitors/EdgeTableRangeBuildingVisitorTest.java b/...ry-core/src/test/java/datawave/query/jexl/visitors/EdgeTableRangeBuildingVisitorTest.java
@@ -2,6 +2,7 @@
 
 import static java.util.Collections.emptyList;
 
+import static datawave.query.jexl.JexlASTHelper.jexlFeatures;
 import static org.junit.Assert.assertThrows;
 
 import org.apache.commons.jexl3.JexlFeatures;
@@ -35,6 +36,6 @@ public void shouldEnforceTermLimit() {
     }
 
     private ASTJexlScript parseQuery(String query) {
-        return parser.parse(null, new JexlFeatures(), EdgeQueryLogic.fixQueryString(query), null);
+        return parser.parse(null, jexlFeatures(), EdgeQueryLogic.fixQueryString(query), null);
     }
 }
diff --git a/...house/query-core/src/test/java/datawave/query/jexl/visitors/NodeTypeCountVisitorTest.java b/...house/query-core/src/test/java/datawave/query/jexl/visitors/NodeTypeCountVisitorTest.java
@@ -254,16 +254,6 @@ public void testASTStringLiteral() {
         assertEquals(1, count(JexlNodes.makeStringLiteral()).getTotal(ASTStringLiteral.class));
     }
 
-    @Test
-    public void testASTArrayLiteral() throws ParseException {
-        assertEquals(1, count("[1, 2, 3]").getTotal(ASTArrayLiteral.class));
-    }
-
-    @Test
-    public void testASTMapLiteral() throws ParseException {
-        assertEquals(1, count("{'one':1, 'two':2, 'three':3}").getTotal(ASTMapLiteral.class));
-    }
-
     @Test
     public void testASTMapEntry() {
         assertEquals(1, count(new ASTMapEntry(ParserTreeConstants.JJTMAPENTRY)).getTotal(ASTMapEntry.class));

diff --git a/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/QueryPruningVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/QueryPruningVisitorTest.java
@@ -502,27 +502,6 @@ public void nestedMarkerBoundedRangeTest() throws ParseException {
         assertEquals(query, JexlStringBuildingVisitor.buildQuery(QueryPruningVisitor.reduce(script, false)));
     }
 
-    @Test
-    public void dualStatementQueryTest() throws ParseException {
-        String query = "(Expression = 'somevalue'); FIELD == 'x'";
-        ASTJexlScript script = JexlASTHelper.parseJexlQuery(query);
-
-        assertEquals(QueryPruningVisitor.TruthState.UNKNOWN, QueryPruningVisitor.getState(script));
-        assertEquals(query, JexlStringBuildingVisitor.buildQuery(QueryPruningVisitor.reduce(script, false)));
-
-        query = "(Expression = 'somevalue'); FIELD == 'x' || true";
-        script = JexlASTHelper.parseJexlQuery(query);
-
-        assertEquals(QueryPruningVisitor.TruthState.TRUE, QueryPruningVisitor.getState(script));
-        assertEquals("(Expression = 'somevalue'); true", JexlStringBuildingVisitor.buildQuery(QueryPruningVisitor.reduce(script, false)));
-
-        query = "(Expression = 'somevalue'); FIELD == 'x' && false";
-        script = JexlASTHelper.parseJexlQuery(query);
-
-        assertEquals(QueryPruningVisitor.TruthState.UNKNOWN, QueryPruningVisitor.getState(script));
-        assertEquals("(Expression = 'somevalue'); false", JexlStringBuildingVisitor.buildQuery(QueryPruningVisitor.reduce(script, false)));
-    }
-
     @Test
     public void propertyMarkerTest() throws ParseException {
         String query = "((_Value_ = true) && (FIELD = 'x'))";

diff --git a/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/TreeEqualityVisitorTest.java b/warehouse/query-core/src/test/java/datawave/query/jexl/visitors/TreeEqualityVisitorTest.java
@@ -45,24 +45,6 @@ public void testNonEqualImage() throws ParseException {
                         "Did not find a matching child for EQNode in [EQNode]: Did not find a matching child for bar in [bat]: Node images differ: bar vs bat");
     }
 
-    /**
-     * Verify that two nodes with differing number of children are considered non-equal.
-     */
-    @Test
-    public void testNonEqualChildrenSize() throws ParseException {
-        assertNotEquivalent("[1, 2, 3, 4]", "[1, 2]",
-                        "Did not find a matching child for [ 1, 2, 3, 4 ] in [[ 1, 2 ]]: Num children differ: [1, 2, 3, 4] vs [1, 2]");
-    }
-
-    /**
-     * Verify that two nodes with different children are considered non-equal.
-     */
-    @Test
-    public void testNonEqualChildren() throws ParseException {
-        assertNotEquivalent("[1, 2]", "[1, 3]",
-                        "Did not find a matching child for [ 1, 2 ] in [[ 1, 3 ]]: Did not find a matching child for 2 in [3]: Node images differ: 2 vs 3");
-    }
-
     /**
      * Verify that two identical trees are considered equal.
      */

diff --git a/...-core/src/test/java/datawave/query/jexl/visitors/TreeFlatteningRebuildingVisitorTest.java b/...-core/src/test/java/datawave/query/jexl/visitors/TreeFlatteningRebuildingVisitorTest.java
@@ -1,5 +1,6 @@
 package datawave.query.jexl.visitors;
 
+import static datawave.query.jexl.JexlASTHelper.jexlFeatures;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
@@ -164,8 +165,8 @@ public void depthNoStackTraceOrTest() throws Exception {
         for (int i = 2; i <= numTerms; i++) {
             sb.append(" OR ").append(i);
         }
-        assertNotNull(TreeFlatteningRebuildingVisitor.flattenAll(new Parser(new StringProvider(";")).parse(null, new JexlFeatures(),
-                        new LuceneToJexlQueryParser().parse(sb.toString()).toString(), null)));
+        assertNotNull(TreeFlatteningRebuildingVisitor.flattenAll(
+                        new Parser(new StringProvider(";")).parse(null, jexlFeatures(), new LuceneToJexlQueryParser().parse(sb.toString()).toString(), null)));
     }
 
     @Test