henrib commented on code in PR #400:
URL: https://github.com/apache/commons-jexl/pull/400#discussion_r3143730666
##########
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:
Every action is denied unless a specific permissions has been granted; we
are saying the same thing.
Paraphrasing, the revised permissions handling is based on the
'closed-world' permissions scheme; there must be an explicit 'grant' for access
to a class/method/constructor/field to be allowed. The updated RESTRICTED set
is very narrow and shall resist adding new sub-packages to lang since we no
longer grant permissions to its descendants.
--
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]
