garydgregory commented on code in PR #400:
URL: https://github.com/apache/commons-jexl/pull/400#discussion_r3143694597
##########
src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java:
##########
@@ -69,71 +89,121 @@ public interface JexlPermissions {
/**
* A permission delegation that augments the RESTRICTED permission with an
explicit
* set of classes.
- * <p>Typical use case is to deny access to a package - and thus all its
classes - but allow
+ * <p>A 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. */
- private final Set<String> allowedClasses;
-
- /**
- * Creates permissions based on the RESTRICTED set but allowing an
explicit set.
- *
- * @param allow the set of allowed classes
- */
- public ClassPermissions(final Class<?>... allow) {
- this(JexlPermissions.RESTRICTED,
- Arrays.stream(Objects.requireNonNull(allow))
- .map(Class::getCanonicalName)
- .collect(Collectors.toList()));
+ final class ClassPermissions extends JexlPermissions.Delegate {
+ /**
+ * The set of explicitly allowed classes, overriding the delegate
permissions.
+ */
+ private final Set<String> allowedClasses;
+
+ /**
+ * Creates permissions based on the RESTRICTED set but allowing an
explicit set.
+ *
+ * @param allow the set of allowed classes
+ */
+ public ClassPermissions(final Class<?>... allow) {
+ this(JexlPermissions.RESTRICTED, allow);
+ }
+
+ /**
+ * Creates permissions by augmenting an existing set with an explicit
set of allowed classes.
+ * @param permissions the base permissions to augment
+ * @param allow the set of allowed classes
+ */
+ public ClassPermissions(final JexlPermissions permissions, final
Class<?>... allow) {
+ this(permissions,
Arrays.stream(Objects.requireNonNull(allow)).map(Class::getCanonicalName).collect(Collectors.toList()));
+ }
+
+ /**
+ * Required for compose().
+ *
+ * @param delegate the base to delegate to
+ * @param allow the list of class canonical names
+ */
+ public ClassPermissions(final JexlPermissions delegate, final
Collection<String> allow) {
+ super(Objects.requireNonNull(delegate));
+ allowedClasses = new HashSet<>(Objects.requireNonNull(allow));
+ }
+
+ @Override
+ public boolean allow(final Constructor<?> constructor) {
Review Comment:
Wow this is confusing! And I've read the last comment twice. This might just
be a vocabulary issue: I see the grant-deny words as a pair in Java, especially
from a JAAS heritage POV where everything is denied unless explicitly granted.
This is not about what happens when you grant or deny a permission, it's about
explaining concepts.
Whether we want to be grant based (deny all, then grant) is a different
story and I'm not sure where we stand there. What does this do? Unless we deny
all, then grant, it seems we will always be open to bugs or requests for
denying more classes by default, say, as soon a new Java version or module is
released.
HTH!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
