[GitHub] [nifi] markap14 commented on a diff in pull request #6317: NIFI-10375: If a class is not allowed in the AllowListClassLoader by …

[GitBox <gi...@apache.org>]

markap14 commented on code in PR #6317:
URL: https://github.com/apache/nifi/pull/6317#discussion_r951707125


##########
nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/AllowListClassLoader.java:
##########
@@ -37,36 +43,92 @@
  * </p>
  */
 public class AllowListClassLoader extends ClassLoader {
-    private final Set<String> allowed;
+    private static final Logger logger = LoggerFactory.getLogger(AllowListClassLoader.class);
+
+    private final Set<String> allowedClassNames;
+    private final List<String> allowedModulePrefixes = Arrays.asList("java.", "jdk.");
 
     public AllowListClassLoader(final ClassLoader parent, final Set<String> allowed) {
         super(parent);
-        this.allowed = allowed;
+        this.allowedClassNames = allowed;
     }
 
     /**
      * @return the set of all Class names that will not be blocked from loading by the parent
      */
     public Set<String> getClassesAllowed() {
-        return Collections.unmodifiableSet(allowed);
+        return Collections.unmodifiableSet(allowedClassNames);
     }
 
     @Override
     protected Class<?> loadClass(final String name, final boolean resolve) throws ClassNotFoundException {
-        if (allowed.contains(name)) {
+        if (allowedClassNames.contains(name)) {
             return super.loadClass(name, resolve);
         }
 
+        final Class<?> found = super.loadClass(name, false);
+        if (found != null) {
+            final boolean allowed = isClassAllowed(name, found);
+            if (allowed) {
+                if (resolve) {
+                    super.resolveClass(found);
+                }
+
+                return found;
+            }
+        }
+
         throw new ClassNotFoundException(name + " was blocked by AllowListClassLoader");
     }
 
     @Override
     protected Class<?> findClass(final String name) throws ClassNotFoundException {
-        if (allowed.contains(name)) {
-            return super.findClass(name);
+        final Class<?> found = super.findClass(name);
+        if (isClassAllowed(name, found)) {
+            return found;
         }
 
         throw new ClassNotFoundException(name + " was blocked by AllowListClassLoader");
     }
 
+    private boolean isClassAllowed(final String name, final Class<?> clazz) {
+        // If the name of the class is in the allowed class names, allow it.
+        if (allowedClassNames.contains(name)) {
+            return true;
+        }
+
+        // If the class has a module whose name is allowed, allow it.
+        // The module is obtained by calling Class.getModule(). However, that method is only available in Java 9.
+        // Since this codebase must be Java 8 compatible we can't make that method call. So we use Reflection to determine
+        // if the getModule method exists (which it will for Java 9+ but not Java 1.8), and if so get the name of the module.
+        try {

Review Comment:
   No, the call to `getMethod` will never return `null`. Instead, it would throw `NoSuchMethodException` if the method doesn't exist. I suppose if we really wanted to be particular we could call `getMethods()` and then look for the method ourselves, in order to avoid the NoSuchMethodException. But I don't think it's necessary, given how infrequently we would expect this to occur.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org