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 1aeec988 JEXL-419: added positive permission syntax; - added tests,
updated javadoc, release notes and changes;
1aeec988 is described below
commit 1aeec98893992c799b42fcca7cb1a914a250b33e
Author: Henri Biestro <hbies...@cloudera.com>
AuthorDate: Sun Jan 28 11:35:51 2024 +0100
JEXL-419: added positive permission syntax;
- added tests, updated javadoc, release notes and changes;
---
RELEASE-NOTES.txt | 1 +
src/changes/changes.xml | 3 +
.../jexl3/internal/introspection/ClassMap.java | 3 +-
.../jexl3/internal/introspection/Permissions.java | 28 +++++--
.../internal/introspection/PermissionsParser.java | 50 ++++++++-----
.../jexl3/introspection/JexlPermissions.java | 39 ++++------
.../org/apache/commons/jexl3/Issues400Test.java | 41 ++++++++++
.../internal/introspection/PermissionsTest.java | 87 +++++++++++++++++++++-
8 files changed, 199 insertions(+), 53 deletions(-)
diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt
index b53f7d24..a059780d 100644
--- a/RELEASE-NOTES.txt
+++ b/RELEASE-NOTES.txt
@@ -30,6 +30,7 @@ Version 3.3.1 is source and binary compatible with 3.3.
New Features in 3.3.1:
====================
+* JEXL-419: Add permission syntax to allow class/method/field
* JEXL-408: Using JexlFeatures is tedious
* JEXL-404: Support array-access safe navigation (x?[y])
* JEXL-401: Captured variables should be read-only
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 03e9d0dc..3bfb20bd 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -29,6 +29,9 @@
<body>
<release version="3.3.1" date="20YY-MM-DD">
<!-- ADD -->
+ <action dev="henrib" type="add" issue="JEXL-419">
+ Add permission syntax to allow class/method/field
+ </action>
<action dev="henrib" type="add" issue="JEXL-408" due-to="sebb">
Using JexlFeatures is tedious
</action>
diff --git
a/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java
b/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java
index fac798e5..f3bd5372 100644
---
a/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java
+++
b/src/main/java/org/apache/commons/jexl3/internal/introspection/ClassMap.java
@@ -262,8 +262,7 @@ final class ClassMap {
}
// now that we've got all methods keyed in, lets organize them by name
if (!cache.byKey.isEmpty()) {
- final List<Method> lm = new ArrayList<>(cache.byKey.size());
- lm.addAll(cache.byKey.values());
+ final List<Method> lm = new ArrayList<>(cache.byKey.values());
// sort all methods by name
lm.sort(Comparator.comparing(Method::getName));
// put all lists of methods with same name in byName cache
diff --git
a/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java
b/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java
index 692a35e8..e3ca6f1a 100644
---
a/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java
+++
b/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java
@@ -56,21 +56,21 @@ public class Permissions implements JexlPermissions {
*/
static class NoJexlPackage {
// the NoJexl class names
- protected Map<String, NoJexlClass> nojexl;
+ protected final Map<String, NoJexlClass> nojexl;
/**
* Ctor.
* @param map the map of NoJexl classes
*/
NoJexlPackage(final Map<String, NoJexlClass> map) {
- this.nojexl = map;
+ this.nojexl = new ConcurrentHashMap<>(map == null ?
Collections.emptyMap() : map);
}
/**
* Default ctor.
*/
NoJexlPackage() {
- this(new ConcurrentHashMap<>());
+ this(null);
}
boolean isEmpty() { return nojexl.isEmpty(); }
@@ -80,7 +80,11 @@ public class Permissions implements JexlPermissions {
}
void addNoJexl(final String key, final NoJexlClass njc) {
- nojexl.put(key, njc);
+ if (njc == null) {
+ nojexl.remove(key);
+ } else {
+ nojexl.put(key, njc);
+ }
}
}
@@ -120,12 +124,13 @@ public class Permissions implements JexlPermissions {
/**
* Equivalent of @NoJexl on a ctor, a method or a field in a class.
+ * <p>Field or method that are named are denied access.</p>
*/
static class NoJexlClass {
// the NoJexl method names (including ctor, name of class)
- protected Set<String> methodNames;
+ protected final Set<String> methodNames;
// the NoJexl field names
- protected Set<String> fieldNames;
+ protected final Set<String> fieldNames;
NoJexlClass(final Set<String> methods, final Set<String> fields) {
methodNames = methods;
@@ -151,6 +156,16 @@ public class Permissions implements JexlPermissions {
}
}
+ /**
+ * A positive NoJexl construct that defines what is denied by absence in
the set.
+ * <p>Field or method that are named are the only one allowed access.</p>
+ */
+ static class JexlClass extends NoJexlClass {
+ @Override boolean deny(final Field field) { return !super.deny(field);
}
+ @Override boolean deny(final Method method) { return
!super.deny(method); }
+ @Override boolean deny(final Constructor<?> method) { return
!super.deny(method); }
+ }
+
/** Marker for whole NoJexl class. */
static final NoJexlClass NOJEXL_CLASS = new
NoJexlClass(Collections.emptySet(), Collections.emptySet()) {
@Override boolean deny(final Field field) {
@@ -501,7 +516,6 @@ public class Permissions implements JexlPermissions {
}
// let's walk all super classes
clazz = clazz.getSuperclass();
- // walk all superclasses
while (clazz != null) {
if (!allow(clazz, method, explicit)) {
return false;
diff --git
a/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java
b/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java
index 7341ee15..12494b78 100644
---
a/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java
+++
b/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java
@@ -45,9 +45,12 @@ import java.util.concurrent.ConcurrentHashMap;
* }
* # and eol comment
* class0(); # constructors
- * method(); # method
+ * method(); # method is not allowed
* field; # field
* } # end class0
+ * +class1 {
+ * method(); // only allowed method of class1
+ * }
* } # end package my.package
* </pre>
*/
@@ -65,6 +68,7 @@ public class PermissionsParser {
* Basic ctor.
*/
public PermissionsParser() {
+ // nothing besides default member initialization
}
/**
@@ -98,13 +102,12 @@ public class PermissionsParser {
}
this.packages = packages;
this.wildcards = wildcards;
- for (final String src : srcs) {
- this.src = src;
- this.size = src.length();
+ for (final String source : srcs) {
+ this.src = source;
+ this.size = source.length();
readPackages();
}
- final Permissions permissions = new Permissions(wildcards,
packages);
- return permissions;
+ return new Permissions(wildcards, packages);
} finally {
clear();
}
@@ -239,7 +242,7 @@ public class PermissionsParser {
pname = temp.toString();
temp.setLength(0);
i = next;
- // consume it if it is a wildcard decl
+ // consume it if it is a wildcard declaration
if (pname.endsWith(".*")) {
wildcards.add(pname);
pname = null;
@@ -250,8 +253,9 @@ public class PermissionsParser {
// package mode
if (njpackage == null) {
if (c == '{') {
- njpackage = new Permissions.NoJexlPackage();
- packages.put(pname, njpackage);
+ njpackage = packages.compute(pname,
+ (n, p) -> new Permissions.NoJexlPackage(p == null?
null : p.nojexl)
+ );
i += 1;
}
} else if (c == '}') {
@@ -263,7 +267,7 @@ public class PermissionsParser {
pname = null;
i += 1;
} else {
- i = readClass(njpackage, null, null, i);
+ i = readClass(njpackage, true,null, null, i);
}
}
}
@@ -271,16 +275,18 @@ public class PermissionsParser {
/**
* Reads a class permission.
* @param njpackage the owning package
+ * @param nojexl whether the restriction is explicitly denying (true) or
allowing (false) members
* @param outer the outer class (if any)
* @param inner the inner class name (if any)
* @param offset the initial parsing position in the source
* @return the new parsing position
*/
- private int readClass(final Permissions.NoJexlPackage njpackage, final
String outer, final String inner, final int offset) {
+ private int readClass(final Permissions.NoJexlPackage njpackage, final
boolean nojexl, final String outer, final String inner, final int offset) {
final StringBuilder temp = new StringBuilder();
Permissions.NoJexlClass njclass = null;
String njname = null;
String identifier = inner;
+ boolean deny = nojexl;
int i = offset;
int j = -1;
boolean isMethod = false;
@@ -303,15 +309,18 @@ public class PermissionsParser {
}
// end of class ?
if (njclass != null && c == '}') {
- // restrict the whole class
- if (njclass.isEmpty()) {
- njpackage.addNoJexl(njname, Permissions.NOJEXL_CLASS);
- }
i += 1;
break;
}
// read an identifier, the class name
if (identifier == null) {
+ // negative or positive set ?
+ if (c == '-') {
+ i += 1;
+ } else if (c == '+') {
+ deny = false;
+ i += 1;
+ }
final int next = readIdentifier(temp, i);
if (i != next) {
identifier = temp.toString();
@@ -327,7 +336,7 @@ public class PermissionsParser {
throw new IllegalStateException(unexpected(c, i));
}
// if we have a class, it has a name
- njclass = new Permissions.NoJexlClass();
+ njclass = deny ? new Permissions.NoJexlClass() : new
Permissions.JexlClass();
njname = outer != null ? outer + "$" + identifier : identifier;
njpackage.addNoJexl(njname, njclass);
identifier = null;
@@ -335,7 +344,7 @@ public class PermissionsParser {
// class member mode
if (c == '{') {
// inner class
- i = readClass(njpackage, njname, identifier, i - 1);
+ i = readClass(njpackage, deny, njname, identifier, i - 1);
identifier = null;
continue;
}
@@ -358,6 +367,13 @@ public class PermissionsParser {
}
i += 1;
}
+ // empty class means allow or deny all
+ if (njname != null && njclass.isEmpty()) {
+ njpackage.addNoJexl(njname, njclass instanceof
Permissions.JexlClass
+ ? Permissions.JEXL_CLASS
+ : Permissions.NOJEXL_CLASS);
+
+ }
return i;
}
}
diff --git
a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
index aa4759c3..71eeb5d5 100644
--- a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
+++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
@@ -147,11 +147,16 @@ public interface JexlPermissions {
* <li>Syntax for restrictions is a list of package restrictions.</li>
* <li>A package restriction is a package name followed by a block (as in
curly-bracket block {})
* that contains a list of class restrictions.</li>
- * <li>A class restriction is a class name followed by a block of member
restrictions.</li>
+ * <li>A class restriction is a class name prefixed by an optional
<code>-</code> or <code>+</code> sign
+ * followed by a block of member restrictions.</li>
* <li>A member restriction can be a class restriction - to restrict
* nested classes -, a field which is the Java field name suffixed with
<code>;</code>, a method composed of
* its Java name suffixed with <code>();</code>. Constructor restrictions
are specified like methods using the
* class name as method name.</li>
+ * <li>By default or when prefixed with a <code>-</code>, a class
restriction is explicitly denying the members
+ * declared in its block (or the whole class)</li>
+ * <li>When prefixed with a <code>+</code>, a class restriction is
explicitly allowing the members
+ * declared in its block (or the whole class)</li>
* </ul>
* <p>
* All overrides and overloads of a constructors or method are allowed or
restricted at the same time,
@@ -165,6 +170,7 @@ public interface JexlPermissions {
* # nojexl like restrictions
* my.package.internal {} # the whole package is hidden
* my.package {
+ * +class4 { theMethod(); } # only theMethod can be called in class4
* class0 {
* class1 {} # the whole class1 is hidden
* class2 {
@@ -293,30 +299,16 @@ public interface JexlPermissions {
* @return true if constructor is not null and public, false otherwise
*/
default boolean validate(final Constructor<?> constructor) {
- if (constructor == null) {
- return false;
- }
- // field must be public
- if (!Modifier.isPublic(constructor.getModifiers())) {
- return false;
- }
- return true;
+ return constructor != null &&
Modifier.isPublic(constructor.getModifiers());
}
/**
* Checks that a method is valid for permission check.
* @param method the method
- * @return true if method is not null, public, ,ot-synthetic, not-bridge,
false otherwise
+ * @return true if method is not null and public, false otherwise
*/
default boolean validate(final Method method) {
- if (method == null) {
- return false;
- }
- // method must be public
- if (!Modifier.isPublic(method.getModifiers())) {
- return false;
- }
- return true;
+ return method != null && Modifier.isPublic(method.getModifiers());
}
/**
@@ -325,14 +317,7 @@ public interface JexlPermissions {
* @return true if field is not null and public, false otherwise
*/
default boolean validate(final Field field) {
- if (field == null) {
- return false;
- }
- // field must be public
- if (!Modifier.isPublic(field.getModifiers())) {
- return false;
- }
- return true;
+ return field != null && Modifier.isPublic(field.getModifiers());
}
/**
@@ -383,6 +368,8 @@ public interface JexlPermissions {
* set of classes.
* <p>Typical use case is to deny access to a package - and thus all its
classes - but allow
* a few specific classes.</p>
+ * <p>Note that the newer positive restriction syntax is preferable as in:
+ * <code>RESTRICTED.compose("java.lang { +Class {} }")</code>.</p>
*/
final class ClassPermissions extends JexlPermissions.Delegate {
/** The set of explicitly allowed classes, overriding the delegate
permissions. */
diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
index e1f79bb1..0a5695bc 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
@@ -16,17 +16,26 @@
*/
package org.apache.commons.jexl3;
+import java.lang.reflect.Method;
import java.math.BigDecimal;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
+import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Objects;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.commons.jexl3.internal.introspection.Permissions;
+import org.apache.commons.jexl3.introspection.JexlPermissions;
+import org.apache.commons.jexl3.introspection.JexlSandbox;
import org.junit.Assert;
import org.junit.Test;
+import static
org.apache.commons.jexl3.introspection.JexlPermissions.RESTRICTED;
+import static org.hamcrest.MatcherAssert.assertThat;
+
/**
* Test cases for reported issue between JEXL-300 and JEXL-399.
*/
@@ -333,4 +342,36 @@ public class Issues400Test {
result = script.execute(null, 42);
Assert.assertEquals("--#42",result.toString() );
}
+
+ @Test
+ public void test419() throws NoSuchMethodException {
+ // check RESTRICTED permissions denies call to System::currentTimeMillis()
+ Method currentTimeMillis = System.class.getMethod("currentTimeMillis");
+ Assert.assertFalse(RESTRICTED.allow(currentTimeMillis));
+ // compose using a positive class permission to allow just
System::currentTimeMillis()
+ JexlPermissions permissions = RESTRICTED.compose("java.lang { +System {
currentTimeMillis(); } }");
+ // check no side effect on compose
+ Assert.assertTrue(permissions.allow(currentTimeMillis));
+ Assert.assertFalse(RESTRICTED.allow(currentTimeMillis));
+
+ // An engine with the System class as namespace and the positive
permissions
+ JexlEngine jexl = new JexlBuilder()
+ .namespaces(Collections.singletonMap("sns", System.class))
+ .permissions(permissions)
+ .create();
+
+ AtomicLong result = new AtomicLong();
+ Assert.assertEquals(0, result.get());
+ long now = System.currentTimeMillis();
+ // calling System::currentTimeMillis() is allowed and behaves as expected
+ jexl.createScript("result.set(sns:currentTimeMillis())",
"result").execute(null, result);
+ Assert.assertTrue(result.get() >= now);
+
+ // we still cant call anything else
+ try {
+ jexl.createScript("sns:gc()").execute(null);
+ } catch(JexlException.Method method) {
+ Assert.assertEquals("gc", method.getMethod());
+ }
+ }
}
diff --git
a/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java
b/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java
index 62e88ec1..d6bc7532 100644
---
a/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java
+++
b/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java
@@ -41,6 +41,8 @@ import org.apache.commons.jexl3.introspection.JexlUberspect;
import org.junit.Assert;
import org.junit.Test;
+import static
org.apache.commons.jexl3.introspection.JexlPermissions.RESTRICTED;
+
/**
* Checks the CacheMap.MethodKey implementation
*/
@@ -229,7 +231,7 @@ public class PermissionsTest {
}
@Test
- public void testParsePermissions0() throws Exception {
+ public void testParsePermissions0a() throws Exception {
final String src = "java.lang { Runtime { exit(); exec(); }
}\njava.net { URL {} }";
final Permissions p = (Permissions) JexlPermissions.parse(src);
final Map<String, Permissions.NoJexlPackage> nojexlmap =
p.getPackages();
@@ -242,10 +244,93 @@ public class PermissionsTest {
final Method exec = getMethod(java.lang.Runtime.class,"exec");
Assert.assertNotNull(exec);
Assert.assertFalse(p.allow(exec));
+ final Method avp =
getMethod(java.lang.Runtime.class,"availableProcessors");
+ Assert.assertNotNull(avp);
+ Assert.assertTrue(p.allow(avp));
final JexlUberspect uber = new Uberspect(null, null, p);
Assert.assertNull(uber.getClassByName("java.net.URL"));
}
+ @Test
+ public void testParsePermissions0b() throws Exception {
+ final String src = "java.lang { -Runtime { exit(); } }";
+ final Permissions p = (Permissions) JexlPermissions.parse(src);
+ final Method exit = getMethod(java.lang.Runtime.class,"exit");
+ Assert.assertNotNull(exit);
+ Assert.assertFalse(p.allow(exit));
+ }
+
+ @Test
+ public void testParsePermissions0c() throws Exception {
+ final String src = "java.lang { +Runtime { availableProcessorCount();
} }";
+ final Permissions p = (Permissions) JexlPermissions.parse(src);
+ final Method exit = getMethod(java.lang.Runtime.class,"exit");
+ Assert.assertNotNull(exit);
+ Assert.assertFalse(p.allow(exit));
+ }
+
+ @Test
+ public void testParsePermissions0d() throws Exception {
+ final String src = "java.lang { +System { currentTimeMillis(); } }";
+ final JexlPermissions p = RESTRICTED.compose(src);
+ final Field in = System.class.getField("in");
+ Assert.assertNotNull(in);
+ Assert.assertFalse(p.allow(in));
+ final Method ctm =
getMethod(java.lang.System.class,"currentTimeMillis");
+ Assert.assertNotNull(ctm);
+ Assert.assertTrue(p.allow(ctm));
+ }
+
+ @Test
+ public void testParsePermissions0e() throws Exception {
+ final String src = "java.lang { +System { in; } }";
+ final JexlPermissions p = RESTRICTED.compose(src);
+ final Field in = System.class.getField("in");
+ Assert.assertNotNull(in);
+ Assert.assertTrue(p.allow(in));
+ final Method ctm =
getMethod(java.lang.System.class,"currentTimeMillis");
+ Assert.assertNotNull(ctm);
+ Assert.assertFalse(p.allow(ctm));
+ }
+
+ @Test
+ public void testParsePermissions0f() throws Exception {
+ final String src = "java.lang { +Class { getName(); getSimpleName(); }
}";
+ final JexlPermissions p = RESTRICTED.compose(src);
+ final Method getName = getMethod(java.lang.Class.class,"getName");
+ Assert.assertNotNull(getName);
+ Assert.assertTrue(p.allow(getName));
+ Assert.assertFalse(RESTRICTED.allow(getName));
+ final Method getSimpleName =
getMethod(java.lang.Class.class,"getSimpleName");
+ Assert.assertNotNull(getSimpleName);
+ Assert.assertTrue(p.allow(getSimpleName));
+ Assert.assertFalse(RESTRICTED.allow(getSimpleName));
+
+ final Method getMethod = getMethod(java.lang.Class.class,"getMethod");
+ Assert.assertNotNull(getMethod);
+ Assert.assertFalse(p.allow(getMethod));
+
+ final Method exit = getMethod(java.lang.Runtime.class,"exit");
+ Assert.assertNotNull(exit);
+ Assert.assertFalse(p.allow(exit));
+ }
+
+ @Test
+ public void testParsePermissions0g() throws Exception {
+ final String src = "java.lang { +Class { } }";
+ final JexlPermissions p = RESTRICTED.compose(src);
+ final Method getName = getMethod(java.lang.Class.class,"getName");
+ Assert.assertNotNull(getName);
+ Assert.assertTrue(p.allow(getName));
+ final Method getMethod = getMethod(java.lang.Class.class,"getMethod");
+ Assert.assertNotNull(getMethod);
+ Assert.assertTrue(p.allow(getMethod));
+
+ final Method exit = getMethod(java.lang.Runtime.class,"exit");
+ Assert.assertNotNull(exit);
+ Assert.assertFalse(p.allow(exit));
+ }
+
@Test
public void testParsePermissions1() {
final String[] src = {
