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 ae3b1cbe JEXL-431: catch statement must be a lexical statement (different scope than try); JEXL-432: clean cached NamespaceFunctor in clearCache (sic); JEXL-433: protect against case of AST return node having no children; JEXL-434: check for safe-array access on null dereference as left-hand side of expression; ae3b1cbe is described below commit ae3b1cbe611229bb402f46b09a85858c3a8d2bb2 Author: Henrib <hbies...@gmail.com> AuthorDate: Tue Dec 10 11:23:18 2024 +0100 JEXL-431: catch statement must be a lexical statement (different scope than try); JEXL-432: clean cached NamespaceFunctor in clearCache (sic); JEXL-433: protect against case of AST return node having no children; JEXL-434: check for safe-array access on null dereference as left-hand side of expression; --- src/changes/changes.xml | 9 ++++ .../apache/commons/jexl3/internal/Debugger.java | 10 ++-- .../commons/jexl3/parser/ASTArrayAccess.java | 5 ++ .../org/apache/commons/jexl3/parser/JexlNode.java | 4 +- .../org/apache/commons/jexl3/parser/Parser.jjt | 12 ++--- .../org/apache/commons/jexl3/scripting/Main.java | 59 ++++++++++---------- .../org/apache/commons/jexl3/ClassCreatorTest.java | 46 +++++++++++++--- .../org/apache/commons/jexl3/Issues400Test.java | 63 ++++++++++++++++++---- .../jexl3/scripting/JexlScriptEngineTest.java | 52 +++++++++++++----- 9 files changed, 191 insertions(+), 69 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 415f72f0..953a3115 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -29,6 +29,15 @@ <body> <release version="3.4.1" date="YYYY-MM-DD" description="This is a feature and maintenance release. Java 8 or later is required."> <!-- FIX --> + <action dev="henrib" type="fix" issue="JEXL-434" due-to="Vincent Bussol"> + The safe-access array operator is not safe + </action> + <action dev="henrib" type="fix" issue="JEXL-433" due-to="Vincent Bussol"> + Debugger does not accept empty return statement + </action> + <action dev="henrib" type="fix" issue="JEXL-432" due-to="Vincent Bussol"> + PNamespace functors are not cleared when the classloader is updated + </action> <action dev="henrib" type="fix" issue="JEXL-431" due-to="Vincent Bussol"> Parse error with variables declared in a catch clause </action> diff --git a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java index 6d6e3bab..91456410 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/Debugger.java +++ b/src/main/java/org/apache/commons/jexl3/internal/Debugger.java @@ -736,7 +736,8 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail { } } } - if (!Character.isSpaceChar(builder.charAt(builder.length() - 1))) { + char lastChar = builder.charAt(builder.length() - 1); + if (!Character.isSpaceChar(lastChar) && lastChar != '\n') { builder.append(' '); } builder.append('}'); @@ -1262,8 +1263,11 @@ public class Debugger extends ParserVisitor implements JexlInfo.Detail { @Override protected Object visit(final ASTReturnStatement node, final Object data) { - builder.append("return "); - accept(node.jjtGetChild(0), data); + builder.append("return"); + if (node.jjtGetNumChildren() > 0) { + builder.append(' '); + accept(node.jjtGetChild(0), data); + } return data; } diff --git a/src/main/java/org/apache/commons/jexl3/parser/ASTArrayAccess.java b/src/main/java/org/apache/commons/jexl3/parser/ASTArrayAccess.java index 8c5efc1e..a2fdadb2 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/ASTArrayAccess.java +++ b/src/main/java/org/apache/commons/jexl3/parser/ASTArrayAccess.java @@ -40,6 +40,11 @@ public class ASTArrayAccess extends JexlLexicalNode { return (safe & 1L << c) != 0; } + @Override + public boolean isSafeLhs(boolean safe) { + return isSafeChild(0) || super.isSafeLhs(safe); + } + @Override public Object jjtAccept(final ParserVisitor visitor, final Object data) { return visitor.visit(this, data); 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 5b30c413..6fc2d7e4 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java +++ b/src/main/java/org/apache/commons/jexl3/parser/JexlNode.java @@ -18,6 +18,7 @@ package org.apache.commons.jexl3.parser; import org.apache.commons.jexl3.JexlArithmetic; import org.apache.commons.jexl3.JexlCache; +import org.apache.commons.jexl3.JexlContext.NamespaceFunctor; import org.apache.commons.jexl3.JexlInfo; import org.apache.commons.jexl3.JxltEngine; import org.apache.commons.jexl3.introspection.JexlMethod; @@ -159,7 +160,8 @@ public abstract class JexlNode extends SimpleNode implements JexlCache.Reference || value instanceof JexlPropertySet || value instanceof JexlMethod || value instanceof Funcall - || value instanceof Class ) { + || value instanceof Class + || value instanceof NamespaceFunctor) { jjtSetValue(null); } for (int n = 0; n < jjtGetNumChildren(); ++n) { diff --git a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt index 9eeac794..9b1d5c25 100644 --- a/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt +++ b/src/main/java/org/apache/commons/jexl3/parser/Parser.jjt @@ -444,15 +444,9 @@ void IfStatement() : {} void TryStatement() : {} { - { - pushUnit(jjtThis); - } - <TRY> (LOOKAHEAD(1) (TryResources() ) | Block()) - (LOOKAHEAD(1) <CATCH> <LPAREN> InlineVar() <RPAREN> Block() { jjtThis.catchClause(); })? + <TRY> (LOOKAHEAD(1) TryResources() | Block()) + (LOOKAHEAD(1) <CATCH> { pushUnit(jjtThis); } <LPAREN> InlineVar() <RPAREN> Block() { jjtThis.catchClause(); popUnit(jjtThis);})? (LOOKAHEAD(1) <FINALLY> Block() { jjtThis.finallyClause(); })? - { - popUnit(jjtThis); - } } void TryResources() : {} @@ -471,7 +465,7 @@ void TryResources() : {} void TryResource() #void : {} { -(LOOKAHEAD(2) Var() | Identifier(true)) +LOOKAHEAD(2) Var() | Identifier(true) } void WhileStatement() : {} diff --git a/src/main/java/org/apache/commons/jexl3/scripting/Main.java b/src/main/java/org/apache/commons/jexl3/scripting/Main.java index 72eeadb7..86e076f7 100644 --- a/src/main/java/org/apache/commons/jexl3/scripting/Main.java +++ b/src/main/java/org/apache/commons/jexl3/scripting/Main.java @@ -18,11 +18,13 @@ package org.apache.commons.jexl3.scripting; import java.io.BufferedReader; -import java.io.File; -import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStreamReader; -import java.io.PrintStream; +import java.io.PrintWriter; import java.nio.charset.Charset; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; import javax.script.ScriptEngine; import javax.script.ScriptException; @@ -36,35 +38,44 @@ public class Main { /** * Test application for JexlScriptEngine (JSR-223 implementation). - * + * <p> * If a single argument is present, it is treated as a file name of a JEXL * script to be evaluated. Any exceptions terminate the application. - * + * </p> * Otherwise, lines are read from standard input and evaluated. * ScriptExceptions are logged, and do not cause the application to exit. * This is done so that interactive testing is easier. + * The line //q! ends the loop. * * @param args (optional) file name to evaluate. Stored in the args variable. * @throws Exception if parsing or IO fail */ public static void main(final String[] args) throws Exception { + try(BufferedReader in = args.length == 1? read(Paths.get(args[0])) : read(null); + PrintWriter out = new PrintWriter(System.out);) { + run(in, out, args); + } + } + + static void run(BufferedReader in, PrintWriter out, final Object... args) throws Exception { final JexlScriptEngineFactory fac = new JexlScriptEngineFactory(); final ScriptEngine engine = fac.getScriptEngine(); - final PrintStream out = System.out; - engine.put("args", args); - if (args.length == 1){ - final Object value = engine.eval(read(null, args[0])); - out.println("Return value: "+value); + if (args.length > 0) { + engine.put("args", args); + final Object value = engine.eval(in); + out.println(">>: " + value); } else { - final BufferedReader console = read(null, null); String line; - System.out.print("> "); - while(null != (line=console.readLine())){ + out.print("> "); + while (null != (line = in.readLine())) { + if ("//q!".equals(line)) { + break; + } try { final Object value = engine.eval(line); - out.println("Return value: "+value); + out.println(">> " + value); } catch (final ScriptException e) { - out.println(e.getLocalizedMessage()); + out.println("!!>" + e.getLocalizedMessage()); } out.print("> "); } @@ -74,19 +85,13 @@ public class Main { /** * Reads an input. * - * @param charset the charset or null for default charset - * @param fileName the file name or null for stdin + * @param path the file path or null for stdin * @return the reader - * @throws Exception if anything goes wrong + * @throws IOException if anything goes wrong */ - static BufferedReader read(final Charset charset, final String fileName) throws Exception { - return new BufferedReader( - new InputStreamReader( - fileName == null - ? System.in - : new FileInputStream(new File(fileName)), - charset == null - ? Charset.defaultCharset() - : charset)); + static BufferedReader read(final Path path) throws IOException { + return new BufferedReader(new InputStreamReader(path == null + ? System.in + : Files.newInputStream(path), Charset.defaultCharset())); } } diff --git a/src/test/java/org/apache/commons/jexl3/ClassCreatorTest.java b/src/test/java/org/apache/commons/jexl3/ClassCreatorTest.java index 000640a4..c689ab3c 100644 --- a/src/test/java/org/apache/commons/jexl3/ClassCreatorTest.java +++ b/src/test/java/org/apache/commons/jexl3/ClassCreatorTest.java @@ -197,7 +197,7 @@ public class ClassCreatorTest extends JexlTestCase { } @Test - public void testBasicCtor() throws Exception { + void testBasicCtor() throws Exception { final JexlScript s = jexl.createScript("(c, v)->{ var ct2 = new(c, v); ct2.value; }"); Object r = s.execute(null, TwoCtors.class, 10); assertEquals(10, r); @@ -210,7 +210,7 @@ public class ClassCreatorTest extends JexlTestCase { } @Test - public void testContextualCtor() throws Exception { + void testContextualCtor() throws Exception { final MapContext ctxt = new MapContext(); ctxt.set("value", 42); JexlScript s = jexl.createScript("(c)->{ new(c).value }"); @@ -222,17 +222,17 @@ public class ClassCreatorTest extends JexlTestCase { } @Test - public void testFunctor2Class() throws Exception { + void testFunctor2Class() throws Exception { functorTwo(new NsTest(ClassCreator.GEN_CLASS + "foo2")); } @Test - public void testFunctor2Name() throws Exception { + void testFunctor2Name() throws Exception { functorTwo(ClassCreator.GEN_CLASS + "foo2"); } @Test - public void testFunctorOne() throws Exception { + void testFunctorOne() throws Exception { final JexlContext ctxt = new MapContext(); ctxt.set("value", 1000); @@ -275,7 +275,7 @@ public class ClassCreatorTest extends JexlTestCase { } @Test - public void testFunctorThree() throws Exception { + void testFunctorThree() throws Exception { final JexlContext ctxt = new MapContext(); ctxt.set("value", 1000); @@ -314,7 +314,7 @@ public class ClassCreatorTest extends JexlTestCase { } @Test - public void testMany() throws Exception { + void testMany() throws Exception { // abort test if class creator cannot run if (!ClassCreator.canRun) { return; @@ -406,7 +406,7 @@ public class ClassCreatorTest extends JexlTestCase { } @Test - public void testOne() throws Exception { + void testOne() throws Exception { // abort test if class creator cannot run if (!ClassCreator.canRun) { logger.warn("unable to create classes"); @@ -418,4 +418,34 @@ public class ClassCreatorTest extends JexlTestCase { assertEquals("foo1", foo1.getSimpleName()); cctor.clear(); } + @Test + void test432() throws Exception { + final ClassCreator cctor = new ClassCreator(jexl, base); + cctor.setSeed(2); + cctor.setCtorBody("value = (Integer) ctxt.get(\"value\") + 10;"); + Class<?> foo1 = cctor.createClass(true); + assertSame(foo1.getClassLoader(), cctor.getClassLoader()); + assertEquals("foo2", foo1.getSimpleName()); + final Map<String, Object> ns = new HashMap<>(); + ns.put("test", foo1.getName()); + // use cache + final JexlEngine jexl2 = new JexlBuilder().namespaces(ns).cache(16).create(); + jexl2.setClassLoader(cctor.getClassLoader()); + cctor.clear(); + final JexlContext ctxt = new MapContext(); + ctxt.set("value", 1000); + final JexlScript script = jexl2.createScript("test:getValue()"); + Object result = script.execute(ctxt); + assertEquals(1010, result); cctor.setSeed(2); + cctor.setCtorBody("value = (Integer) ctxt.get(\"value\") + 99;"); + final Class<?> foo11 = cctor.createClass(true); + assertEquals("foo2", foo1.getSimpleName()); + assertNotSame(foo11, foo1); + foo1 = foo11; + // drum rolll.... + jexl2.setClassLoader(foo1.getClassLoader()); + result = script.execute(ctxt); + // tada! + assertEquals(1099, result); + } } diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java b/src/test/java/org/apache/commons/jexl3/Issues400Test.java index f599fbc8..f3330b3c 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java @@ -17,15 +17,9 @@ package org.apache.commons.jexl3; import static org.apache.commons.jexl3.introspection.JexlPermissions.RESTRICTED; -import static org.junit.jupiter.api.Assertions.assertArrayEquals; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertInstanceOf; -import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; +import java.io.Closeable; import java.lang.reflect.Method; import java.math.BigDecimal; import java.util.Arrays; @@ -37,6 +31,7 @@ import java.util.Map; import java.util.Objects; import java.util.concurrent.atomic.AtomicLong; +import org.apache.commons.jexl3.internal.Debugger; import org.apache.commons.jexl3.introspection.JexlPermissions; import org.junit.jupiter.api.Test; @@ -495,7 +490,7 @@ public class Issues400Test { } @Test - void test431() { + void test431a() { JexlEngine jexl = new JexlBuilder().create(); final String src = "let x = 0; try { x += 19 } catch (let error) { return 169 } try { x += 23 } catch (let error) { return 169 }"; final JexlScript script = jexl.createScript(src); @@ -503,4 +498,54 @@ public class Issues400Test { final Object result = script.execute(null); assertEquals(42, result); } + + Closeable foo() { + return null; + } + + @Test + void test431b() { + JexlEngine jexl = new JexlBuilder().create(); + final String src = "let x = 0; try(let error) { x += 19 } catch (let error) { return 169 } try { x += 23 } catch (let error) { return 169 }"; + final JexlScript script = jexl.createScript(src); + assertNotNull(script); + final Object result = script.execute(null); + assertEquals(42, result); + } + + @Test + void test431c() { + JexlEngine jexl = new JexlBuilder().create(); + final String src = "let xx = 0; try { xx += 19 } catch (let xx) { return 169 }"; + try { + final JexlScript script = jexl.createScript(src); + fail("xx is already defined in scope"); + } catch(JexlException.Parsing parsing) { + assertTrue(parsing.getDetail().contains("xx")); + } + } + + @Test + void test433() { + JexlEngine jexl = new JexlBuilder().create(); + final String src = "let condition = true; if (condition) { return; }"; + final JexlScript script = jexl.createScript(src); + assertNotNull(script); + Object result = script.execute(null); + assertNull(result); + Debugger debugger = new Debugger(); + assertTrue(debugger.debug(script)); + String dbgStr = debugger.toString(); + assertTrue(JexlTestCase.equalsIgnoreWhiteSpace(src, dbgStr)); + } + + @Test + void test434() { + JexlEngine jexl = new JexlBuilder().safe(false).strict(true).create(); + final String src = "let foo = null; let value = foo?[bar]"; + final JexlScript script = jexl.createScript(src); + assertNotNull(script); + final Object result = script.execute(null); + assertNull(result); + } } diff --git a/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java b/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java index 7f5f1a65..c9917781 100644 --- a/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java +++ b/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java @@ -24,8 +24,11 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.BufferedReader; +import java.io.PrintWriter; import java.io.Reader; import java.io.StringReader; +import java.io.StringWriter; import java.util.Arrays; import java.util.List; import java.util.Map; @@ -40,6 +43,7 @@ import org.apache.commons.jexl3.JexlBuilder; import org.apache.commons.jexl3.JexlException; import org.apache.commons.jexl3.introspection.JexlPermissions; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; public class JexlScriptEngineTest { @@ -61,14 +65,14 @@ public class JexlScriptEngineTest { "application/x-jexl3"); @AfterEach - public void tearDown() { + void tearDown() { JexlBuilder.setDefaultPermissions(null); JexlScriptEngine.setInstance(null); JexlScriptEngine.setPermissions(null); } @Test - public void testCompile() throws Exception { + void testCompile() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); final JexlScriptEngine engine = (JexlScriptEngine) manager.getEngineByName("JEXL"); final ScriptContext ctxt = engine.getContext(); @@ -106,14 +110,14 @@ public class JexlScriptEngineTest { } @Test - public void testDirectNew() throws Exception { + void testDirectNew() throws Exception { final ScriptEngine engine = new JexlScriptEngine(); final Integer initialValue = 123; assertEquals(initialValue,engine.eval("123")); } @Test - public void testDottedNames() throws Exception { + void testDottedNames() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); assertNotNull(manager, "Manager should not be null"); final ScriptEngine engine = manager.getEngineByName("JEXL"); @@ -127,7 +131,7 @@ public class JexlScriptEngineTest { } @Test - public void testErrors() throws Exception { + void testErrors() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); final JexlScriptEngine engine = (JexlScriptEngine) manager.getEngineByName("JEXL"); final ScriptContext ctxt = engine.getContext(); @@ -141,7 +145,7 @@ public class JexlScriptEngineTest { } @Test - public void testNulls() throws Exception { + void testNulls() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); assertNotNull(manager, "Manager should not be null"); final ScriptEngine engine = manager.getEngineByName("jexl3"); @@ -155,7 +159,7 @@ public class JexlScriptEngineTest { } @Test - public void testScopes() throws Exception { + void testScopes() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); assertNotNull(manager, "Manager should not be null"); final ScriptEngine engine = manager.getEngineByName("jexl3"); @@ -177,7 +181,7 @@ public class JexlScriptEngineTest { } @Test - public void testScriptEngineFactory() throws Exception { + void testScriptEngineFactory() throws Exception { final JexlScriptEngineFactory factory = new JexlScriptEngineFactory(); assertEquals("JEXL Engine", factory.getParameter(ScriptEngine.ENGINE)); assertEquals("3.4", factory.getParameter(ScriptEngine.ENGINE_VERSION)); @@ -193,7 +197,7 @@ public class JexlScriptEngineTest { } @Test - public void testScripting() throws Exception { + void testScripting() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); assertNotNull(manager, "Manager should not be null"); final ScriptEngine engine = manager.getEngineByName("jexl3"); @@ -219,7 +223,7 @@ public class JexlScriptEngineTest { } @Test - public void testScriptingGetBy() throws Exception { + void testScriptingGetBy() throws Exception { final ScriptEngineManager manager = new ScriptEngineManager(); assertNotNull(manager, "Manager should not be null"); for (final String name : NAMES) { @@ -236,7 +240,7 @@ public class JexlScriptEngineTest { } } @Test - public void testScriptingInstance0() throws Exception { + void testScriptingInstance0() throws Exception { JexlScriptEngine.setPermissions(JexlPermissions.UNRESTRICTED); final ScriptEngineManager manager = new ScriptEngineManager(); final ScriptEngine engine = manager.getEngineByName("jexl3"); @@ -247,7 +251,7 @@ public class JexlScriptEngineTest { } @Test - public void testScriptingPermissions1() throws Exception { + void testScriptingPermissions1() throws Exception { JexlBuilder.setDefaultPermissions(JexlPermissions.UNRESTRICTED); JexlScriptEngine.setPermissions(null); final ScriptEngineManager manager = new ScriptEngineManager(); @@ -257,4 +261,28 @@ public class JexlScriptEngineTest { + "now=sys.currentTimeMillis();"); assertTrue(time2 <= System.currentTimeMillis()); } + + + @Test + void testMain0() throws Exception { + StringWriter strw = new StringWriter(); + StringReader strr = new StringReader("a=20\nb=22\na+b\n//q!\n"); + Main.run(new BufferedReader(strr), new PrintWriter(strw), new String[0]); + String ctl = "> >> 20\n" + + "> >> 22\n" + + "> >> 42\n" + + "> "; + Assertions.assertEquals(ctl, strw.toString()); + } + + + @Test + void testMain1() throws Exception { + StringWriter strw = new StringWriter(); + StringReader strr = new StringReader("args[0]+args[1]"); + Main.run(new BufferedReader(strr), new PrintWriter(strw), 20, 22); + String ctl = ">>: 42\n"; + Assertions.assertEquals(ctl, strw.toString()); + } + }