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 788c4059 JEXL-424 : permissions must be independent of resolution
order;
788c4059 is described below
commit 788c4059eda8baf45150c2fb205793dae5c7b590
Author: Henrib <hbies...@gmail.com>
AuthorDate: Sat Aug 31 08:31:37 2024 +0200
JEXL-424 : permissions must be independent of resolution order;
---
.../commons/jexl3/introspection/JexlSandbox.java | 73 +++++++++-------------
.../commons/jexl3/introspection/SandboxTest.java | 22 +++----
2 files changed, 39 insertions(+), 56 deletions(-)
diff --git
a/src/main/java/org/apache/commons/jexl3/introspection/JexlSandbox.java
b/src/main/java/org/apache/commons/jexl3/introspection/JexlSandbox.java
index 2ce5db98..efb4077c 100644
--- a/src/main/java/org/apache/commons/jexl3/introspection/JexlSandbox.java
+++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlSandbox.java
@@ -249,6 +249,16 @@ public final class JexlSandbox {
return "".equals(name) && m != null ? clazz : m;
}
+ /**
+ * Gets the set of permissions associated to a class.
+ *
+ * @param clazz the class name
+ * @return the defined permissions or an all-allow permission instance if
none were defined
+ */
+ public Permissions get(final String clazz) {
+ return get(forName(clazz));
+ }
+
/**
* Gets the permissions associated to a class.
*
@@ -259,80 +269,55 @@ public final class JexlSandbox {
public Permissions get(final Class<?> clazz) {
// argument clazz can not be null since permissions would be not null
and block:
// we only store the result for classes we actively seek permissions
for.
- return clazz == null ? BLOCK_ALL : compute(clazz);
+ return compute(clazz, true);
}
- private static Permissions inheritable(Permissions p) {
+ private static Permissions inheritable(final Permissions p) {
return p != null && p.isInheritable() ? p : null;
}
- /**
- * Find first inherited interface that defines permissions through
recursion.
- * @param clazz the clazz
- * @return the array of all its interfaces
- */
- private Permissions computeInterfaces(final Class<?> clazz) {
- Permissions permissions = inheritable(sandbox.get(clazz.getName()));
- if (permissions == null) {
- final Class<?>[] interfaces = clazz.getInterfaces();
- for (int i = 0; permissions == null && i < interfaces.length; ++i)
{
- permissions = computeInterfaces(interfaces[i]);
- }
- }
- return permissions;
- }
-
/**
* Computes and optionally stores the permissions associated to a class.
*
* @param clazz the class
+ * @param store whether the computed permissions should be stored in the
sandbox
* @return the permissions
*/
- private Permissions compute(final Class<?> clazz) {
- Permissions permissions = sandbox.get(clazz.getName());
+ private Permissions compute(final Class<?> clazz, final boolean store) {
+ // belt and suspender; recursion should not lead here
+ if (clazz == null) {
+ return BLOCK_ALL;
+ }
+ final String className = clazz.getName();
+ Permissions permissions = sandbox.get(className);
if (permissions == null) {
if (inherit) {
// find first inherited interface that defines permissions
final Class<?>[] interfaces = clazz.getInterfaces();
for (int i = 0; permissions == null && i < interfaces.length;
++i) {
- permissions = computeInterfaces(interfaces[i]);
+ permissions = inheritable(compute(interfaces[i], false));
}
// nothing defined yet, find first superclass that defines
permissions
if (permissions == null) {
- // let's walk all super classes
- for (Class<?> zuper = clazz.getSuperclass();
- permissions == null && zuper != null;
- zuper = zuper.getSuperclass()) {
- permissions =
inheritable(sandbox.get(zuper.getName()));
+ // let's recurse on super classes
+ Class<?> superClazz = clazz.getSuperclass();
+ if (Object.class != superClazz) {
+ permissions = inheritable(compute(superClazz, false));
}
}
}
- // nothing was determined through inheritance
+ // nothing was inheritable
if (permissions == null) {
permissions = allow ? ALLOW_ALL : BLOCK_ALL;
}
// store the info to avoid doing this costly look-up
- sandbox.put(clazz.getName(), permissions);
+ if (store) {
+ sandbox.put(className, permissions);
+ }
}
return permissions;
}
- /**
- * Gets the set of permissions associated to a class.
- *
- * @param clazz the class name
- * @return the defined permissions or an all-allow permission instance if
none were defined
- */
- public Permissions get(final String clazz) {
- if (inherit) {
- return get(forName(clazz));
- }
- final Permissions permissions = sandbox.get(clazz);
- if (permissions == null) {
- return allow ? ALLOW_ALL : BLOCK_ALL;
- }
- return permissions;
- }
/**
* Creates the set of permissions for a given class.
diff --git
a/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java
b/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java
index 229d21c5..aea2cac6 100644
--- a/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java
+++ b/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java
@@ -604,18 +604,16 @@ class SandboxTest extends JexlTestCase {
static class B extends A{}
@Test
- void testPermission0() {
- JexlSandbox sandbox = new JexlSandbox(false, true);
- sandbox.permissions(I.class.getName(), true, true, true, false);
- System.out.println("permission A=" +
sandbox.get(A.class.getName()).write());
- System.out.println("permission B=" +
sandbox.get(B.class.getName()).write());
- }
- @Test
- void testPermission1() {
- JexlSandbox sandbox = new JexlSandbox(false, true);
- sandbox.permissions(I.class.getName(), true, true, true, false);
- System.out.println("permission B=" +
sandbox.get(B.class.getName()).write());
- System.out.println("permission A=" +
sandbox.get(A.class.getName()).write());
+ void testPermissionOrder() {
+ // permissions should not be dependent on order of evaluation
+ JexlSandbox sandboxAB = new JexlSandbox(false, true);
+ sandboxAB.permissions(I.class.getName(), true, true, true, false);
+ assertEquals("allow{all}",
sandboxAB.get(A.class.getName()).write().toString());
+ assertEquals("allow{all}",
sandboxAB.get(B.class.getName()).write().toString());
+ JexlSandbox sandboxBA = new JexlSandbox(false, true);
+ sandboxBA.permissions(I.class.getName(), true, true, true, false);
+ assertEquals("allow{all}",
sandboxBA.get(B.class.getName()).write().toString());
+ assertEquals("allow{all}",
sandboxBA.get(A.class.getName()).write().toString());
}
@Test
