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 04da59b JEXL-351: interpret jexl:{print,include} in template scripts and avoid sandboxed introspection, add test, changes and release notes. 04da59b is described below commit 04da59b8ca751196e7493d681d52ef21455f9780 Author: henrib <hen...@apache.org> AuthorDate: Tue Jun 8 18:21:16 2021 +0200 JEXL-351: interpret jexl:{print,include} in template scripts and avoid sandboxed introspection, add test, changes and release notes. --- RELEASE-NOTES.txt | 17 ++++- src/changes/changes.xml | 7 +- .../jexl3/internal/TemplateInterpreter.java | 50 +++++++++++++ .../org/apache/commons/jexl3/Issues300Test.java | 83 +++++++++++----------- .../java/org/apache/commons/jexl3/JXLTTest.java | 53 ++++++++++++++ 5 files changed, 166 insertions(+), 44 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 28126ac..08c105e 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -18,6 +18,20 @@ Its goal is to expose scripting features usable by technical operatives or consu https://commons.apache.org/jexl/ +======================================================================================================================== +Release 3.2.1 +======================================================================================================================== + +Version 3.2.1 is a micro release. + +Compatibility with previous releases +==================================== +Version 3.2.1 is source and binary compatible with 3.2. + +Bugs Fixed in 3.2.1: +================== +* JEXL-351: JXLT Template fails when using sandboxing +* JEXL-350: map[null] throws "unsolvable property" when a Sandbox is used ======================================================================================================================== Release 3.2 @@ -78,8 +92,7 @@ New Features in 3.2: Bugs Fixed in 3.2: ================== -* JEXL-350: map[null] throws "unsolvable property" when a Sandbox is used -* JEXL-349: Script valid in 3.0 no longer valid +* JEXL-349: Script valid in 3.0 no longer valid * JEXL-348: Parsing error when mixing namespaces with conditional expressions * JEXL-347: Missing unsolvable property exception for reference when used with equals * JEXL-346: namespace function syntax leads to strange error for "common case" of ternary operator diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a72fd3f..53b1900 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -25,10 +25,15 @@ <author email="d...@commons.apache.org">Commons Developers</author> </properties> <body> - <release version="3.2"> + <release version="3.2.1"> + <action dev="henrib" type="fix" issue="JEXL-351" due-to="Francesco Chicchiricco"> + JXLT Template fails when using sandboxing + </action> <action dev="henrib" type="fix" issue="JEXL-350" due-to="David Costanzo"> map[null] throws "unsolvable property" when a Sandbox is used </action> + </release> + <release version="3.2" date="2021-06-07"> <action dev="henrib" type="fix" issue="JEXL-349" due-to="Cameron Samak"> Script valid in 3.0 no longer valid </action> diff --git a/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java b/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java index 61f1c20..73d8d5c 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java +++ b/src/main/java/org/apache/commons/jexl3/internal/TemplateInterpreter.java @@ -23,12 +23,15 @@ import org.apache.commons.jexl3.JxltEngine; import org.apache.commons.jexl3.internal.TemplateEngine.TemplateExpression; import org.apache.commons.jexl3.introspection.JexlMethod; import org.apache.commons.jexl3.introspection.JexlUberspect; +import org.apache.commons.jexl3.parser.ASTArguments; +import org.apache.commons.jexl3.parser.ASTFunctionNode; import org.apache.commons.jexl3.parser.ASTIdentifier; import org.apache.commons.jexl3.parser.ASTJexlLambda; import org.apache.commons.jexl3.parser.ASTJexlScript; import org.apache.commons.jexl3.parser.JexlNode; import java.io.Writer; +import java.util.Arrays; /** * The type of interpreter to use during evaluation of templates. @@ -213,6 +216,53 @@ public class TemplateInterpreter extends Interpreter { return super.visit(node, data); } + /** + * Interprets a function node. + * print() and include() must be decoded by this interpreter since delegating to the Uberspect + * may be sandboxing the interpreter itself making it unable to call the function. + * @param node the function node + * @param data the data + * @return the function evaluation result. + */ + @Override + protected Object visit(final ASTFunctionNode node, Object data) { + final int argc = node.jjtGetNumChildren(); + if (argc == 2) { + final ASTIdentifier functionNode = (ASTIdentifier) node.jjtGetChild(0); + if ("jexl".equals(functionNode.getNamespace())) { + final String functionName = functionNode.getName(); + final ASTArguments argNode = (ASTArguments) node.jjtGetChild(1); + if ("print".equals(functionName)) { + // evaluate the arguments + Object[] argv = visit(argNode, null); + if (argv != null && argv.length > 0 && argv[0] instanceof Number) { + print(((Number) argv[0]).intValue()); + return null; + } + } + if ("include".equals(functionName)) { + // evaluate the arguments + Object[] argv = visit(argNode, null); + if (argv != null && argv.length > 0) { + if (argv[0] instanceof TemplateScript) { + TemplateScript script = (TemplateScript) argv[0]; + if (argv.length > 1) { + argv = Arrays.copyOfRange(argv, 1, argv.length); + } else { + argv = null; + } + include(script, argv); + return null; + } + } + } + // fail safe + throw new JxltEngine.Exception(node.jexlInfo(), "no callable template function " + functionName, null); + } + } + return super.visit(node, data); + } + @Override protected Object visit(final ASTJexlScript script, final Object data) { if (script instanceof ASTJexlLambda && !((ASTJexlLambda) script).isTopLevel()) { diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java b/src/test/java/org/apache/commons/jexl3/Issues300Test.java index b5f774d..b682f37 100644 --- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java +++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java @@ -36,7 +36,7 @@ public class Issues300Test { public void testIssue301a() throws Exception { 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]" + "var x = null; x.0", "var x = null; x[0]", "var x = [null,1]; x[0][0]" }; for (int i = 0; i < srcs.length; ++i) { final String src = srcs[i]; @@ -57,7 +57,7 @@ public class Issues300Test { 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[]{ - "x.0", "x[0]", "x[0][0]" + "x.0", "x[0]", "x[0][0]" }; final JexlContext ctxt = new MapContext(); for (int i = 0; i < xs.length; ++i) { @@ -73,21 +73,21 @@ public class Issues300Test { } } - @Test + @Test public void testIssue302() throws Exception { final JexlContext jc = new MapContext(); final String[] strs = new String[]{ - "{if (0) 1 else 2; var x = 4;}", - "if (0) 1; else 2; ", - "{ if (0) 1; else 2; }", - "{ if (0) { if (false) 1 else -3 } else 2; }" + "{if (0) 1 else 2; var x = 4;}", + "if (0) 1; else 2; ", + "{ if (0) 1; else 2; }", + "{ if (0) { if (false) 1 else -3 } else 2; }" }; final JexlEngine jexl = new JexlBuilder().create(); - for(final String str : strs) { - final JexlScript e = jexl.createScript(str); - final Object o = e.execute(jc); - final int oo = ((Number) o).intValue() % 2; - Assert.assertEquals("Block result is wrong " + str, 0, oo); + for (final String str : strs) { + final JexlScript e = jexl.createScript(str); + final Object o = e.execute(jc); + final int oo = ((Number) o).intValue() % 2; + Assert.assertEquals("Block result is wrong " + str, 0, oo); } } @@ -96,11 +96,11 @@ public class Issues300Test { final JexlEngine jexlEngine = new JexlBuilder().strict(false).create(); JexlExpression e304 = jexlEngine.createExpression("overview.limit.var"); - final HashMap<String,Object> map3 = new HashMap<String,Object>(); + final HashMap<String, Object> map3 = new HashMap<String, Object>(); map3.put("var", "4711"); - final HashMap<String,Object> map2 = new HashMap<String,Object>(); + final HashMap<String, Object> map2 = new HashMap<String, Object>(); map2.put("limit", map3); - final HashMap<String,Object> map = new HashMap<String,Object>(); + final HashMap<String, Object> map = new HashMap<String, Object>(); map.put("overview", map2); final JexlContext context = new MapContext(map); @@ -125,7 +125,7 @@ public class Issues300Test { JexlScript e; e = jexl.createScript("{while(false) {}; var x = 1;}"); final String str0 = e.getParsedText(); - e = jexl.createScript(str0); + e = jexl.createScript(str0); Assert.assertNotNull(e); final String str1 = e.getParsedText(); Assert.assertEquals(str0, str1); @@ -246,11 +246,12 @@ public class Issues300Test { VaContext(final Map<String, Object> vars) { super(vars); } + public int cell(final String... ms) { return ms.length; } - public int cell(final List<?> l, final String...ms) { + public int cell(final List<?> l, final String... ms) { return 42 + cell(ms); } } @@ -258,7 +259,7 @@ public class Issues300Test { @Test public void test314() throws Exception { final JexlEngine jexl = new JexlBuilder().strict(true).safe(false).create(); - final Map<String,Object> vars = new HashMap<String, Object>(); + final Map<String, Object> vars = new HashMap<String, Object>(); final JexlContext ctxt = new VaContext(vars); JexlScript script; Object result; @@ -299,7 +300,7 @@ public class Issues300Test { @Test public void test315() throws Exception { final JexlEngine jexl = new JexlBuilder().strict(true).create(); - final Map<String,Object> vars = new HashMap<String, Object>(); + final Map<String, Object> vars = new HashMap<String, Object>(); final JexlContext ctxt = new VaContext(vars); JexlScript script; Object result; @@ -336,7 +337,7 @@ public class Issues300Test { Object result; JexlInfo info = new JexlInfo("test317", 1, 1); script = jexl.createScript(info, "var f = " - + "()-> {x + x }; f", + + "()-> {x + x }; f", "x"); result = script.execute(ctxt, 21); Assert.assertTrue(result instanceof JexlScript); @@ -355,10 +356,10 @@ public class Issues300Test { final JexlContext context = new MapContext(); final String[] ins = new String[]{ - "${'{'}", "${\"{\"}", "${\"{}\"}", "${'{42}'}", "${\"{\\\"\\\"}\"}" + "${'{'}", "${\"{\"}", "${\"{}\"}", "${'{42}'}", "${\"{\\\"\\\"}\"}" }; final String[] ctls = new String[]{ - "{", "{", "{}", "{42}", "{\"\"}" + "{", "{", "{}", "{42}", "{\"\"}" }; StringWriter strw; JxltEngine.Template template; @@ -368,7 +369,7 @@ public class Issues300Test { final String src = ins[i]; try { template = jxlt.createTemplate("$$", new StringReader(src)); - } catch(final JexlException xany) { + } catch (final JexlException xany) { Assert.fail(src); throw xany; } @@ -424,17 +425,17 @@ public class Issues300Test { @Test public void test323() throws Exception { final JexlEngine jexl = new JexlBuilder().safe(false).create(); - final Map<String,Object> vars = new HashMap<String, Object>(); + final Map<String, Object> vars = new HashMap<String, Object>(); final JexlContext jc = new MapContext(vars); JexlScript script; Object result; // nothing in context, ex try { - script = jexl.createScript("a.n.t.variable"); - result = script.execute(jc); - Assert.fail("a.n.t.variable is undefined!"); - } catch(final JexlException.Variable xvar) { + script = jexl.createScript("a.n.t.variable"); + result = script.execute(jc); + Assert.fail("a.n.t.variable is undefined!"); + } catch (final JexlException.Variable xvar) { Assert.assertTrue(xvar.toString().contains("a.n.t")); } @@ -447,20 +448,20 @@ public class Issues300Test { // defined and null, dereference jc.set("a.n.t", null); try { - script = jexl.createScript("a.n.t[0].variable"); - result = script.execute(jc); - Assert.fail("a.n.t is null!"); - } catch(final JexlException.Variable xvar) { + script = jexl.createScript("a.n.t[0].variable"); + result = script.execute(jc); + Assert.fail("a.n.t is null!"); + } catch (final JexlException.Variable xvar) { Assert.assertTrue(xvar.toString().contains("a.n.t")); } // undefined, dereference vars.remove("a.n.t"); try { - script = jexl.createScript("a.n.t[0].variable"); - result = script.execute(jc); - Assert.fail("a.n.t is undefined!"); - } catch(final JexlException.Variable xvar) { + script = jexl.createScript("a.n.t[0].variable"); + result = script.execute(jc); + Assert.fail("a.n.t is undefined!"); + } catch (final JexlException.Variable xvar) { Assert.assertTrue(xvar.toString().contains("a.n.t")); } // defined, derefence undefined property @@ -470,7 +471,7 @@ public class Issues300Test { script = jexl.createScript("a.n.t[0].variable"); result = script.execute(jc); Assert.fail("a.n.t is null!"); - } catch(final JexlException.Property xprop) { + } catch (final JexlException.Property xprop) { Assert.assertTrue(xprop.toString().contains("0")); } // defined, derefence undefined property @@ -479,7 +480,7 @@ public class Issues300Test { script = jexl.createScript("a.n.t[0].variable"); result = script.execute(jc); Assert.fail("a.n.t is null!"); - } catch(final JexlException.Property xprop) { + } catch (final JexlException.Property xprop) { Assert.assertTrue(xprop.toString().contains("variable")); } @@ -568,7 +569,7 @@ public class Issues300Test { JexlScript script = jexl.createScript(src); Object result = script.execute(null); // safe navigation is lenient wrt null - Assert.assertFalse((Boolean)result); + Assert.assertFalse((Boolean) result); jexl = new JexlBuilder().strict(true).safe(false).create(); JexlContext ctxt = new MapContext(); @@ -577,7 +578,7 @@ public class Issues300Test { try { result = script.execute(ctxt); Assert.fail("should only succeed with safe navigation"); - } catch(JexlException xany) { + } catch (JexlException xany) { Assert.assertNotNull(xany); } // A is null, A.B is undefined @@ -585,7 +586,7 @@ public class Issues300Test { try { result = script.execute(ctxt); Assert.fail("should only succeed with safe navigation"); - } catch(JexlException xany) { + } catch (JexlException xany) { Assert.assertNotNull(xany); } // A.B is null diff --git a/src/test/java/org/apache/commons/jexl3/JXLTTest.java b/src/test/java/org/apache/commons/jexl3/JXLTTest.java index 75457e0..67209e4 100644 --- a/src/test/java/org/apache/commons/jexl3/JXLTTest.java +++ b/src/test/java/org/apache/commons/jexl3/JXLTTest.java @@ -18,6 +18,11 @@ package org.apache.commons.jexl3; import org.apache.commons.jexl3.internal.Debugger; import org.apache.commons.jexl3.internal.TemplateDebugger; +import org.apache.commons.jexl3.internal.TemplateInterpreter; +import org.apache.commons.jexl3.internal.TemplateScript; +import org.apache.commons.jexl3.internal.introspection.Permissions; +import org.apache.commons.jexl3.internal.introspection.Uberspect; +import org.apache.commons.jexl3.introspection.JexlMethod; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -25,10 +30,12 @@ import java.io.PrintWriter; import java.io.StringReader; import java.io.StringWriter; import java.io.Writer; +import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Set; +import java.util.logging.Logger; import org.junit.After; import org.junit.Assert; @@ -1180,4 +1187,50 @@ public class JXLTTest extends JexlTestCase { String result = strw.toString(); Assert.assertEquals(src, result); } + + private static final Permissions NOJEXL3 = new Permissions() { + @Override public boolean allow(Class<?> clazz) { + String cname = clazz.getName(); + return !cname.contains("jexl3") || cname.contains("311"); + } + }; + + @Test + public void testSanboxedTemplate() throws Exception { + final String src = "Hello ${user}"; + final JexlContext ctxt = new MapContext(); + ctxt.set("user", "Francesco"); + /// this uberspect can not access jexl3 classes (besides test) + Uberspect uberspect = new Uberspect(LogFactory.getLog(JXLTTest.class), null, NOJEXL3); + Method method = uberspect.getMethod(TemplateInterpreter.class, "print", new Object[]{Integer.TYPE}); + Assert.assertNull(method); + // ensures JXLT sandboxed still executes + final JexlEngine jexl= new JexlBuilder().uberspect(uberspect).create(); + final JxltEngine jxlt = jexl.createJxltEngine(); + + JxltEngine.Template tmplt = jxlt.createTemplate(src); + Writer strw = new StringWriter(); + tmplt.evaluate(ctxt, strw); + String result = strw.toString(); + Assert.assertEquals("Hello Francesco", result); + } + + @Test + public void testSanboxed311i() throws Exception { + /// this uberspect can not access jexl3 classes (besides test) + Uberspect uberspect = new Uberspect(LogFactory.getLog(JXLTTest.class), null, NOJEXL3); + Method method = uberspect.getMethod(TemplateInterpreter.class, "print", new Object[]{Integer.TYPE}); + final JexlEngine jexl= new JexlBuilder().uberspect(uberspect).create(); + final JxltEngine jxlt = jexl.createJxltEngine(); + final JexlContext ctx311 = new Context311(); + final String rpt + = "$$var u = 'Universe'; exec('4').execute((a, b)->{" + + "\n<p>${u} ${a}${b}</p>" + + "\n$$}, '2')"; + final JxltEngine.Template t = jxlt.createTemplate("$$", new StringReader(rpt)); + final StringWriter strw = new StringWriter(); + t.evaluate(ctx311, strw, 42); + final String output = strw.toString(); + Assert.assertEquals("<p>Universe 42</p>\n", output); + } }