From 788c4059eda8baf45150c2fb205793dae5c7b590 Mon Sep 17 00:00:00 2001 From: Henrib Date: Sat, 31 Aug 2024 08:31:37 +0200 Subject: [PATCH] JEXL-424 : permissions must be independent of resolution order; --- .../jexl3/introspection/JexlSandbox.java | 73 ++++++++----------- .../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 2ce5db986..efb4077cf 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 String execute(final String clazz, final String name) { 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 String execute(final String clazz, final String name) { 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 229d21c5a..aea2cac63 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 @@ static class A implements I{} 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