[GitHub] [nifi] turcsanyip commented on a diff in pull request #5925: NIFI-9861: Removed BlockListClassLoader in favor of AllowListClassLoader

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

turcsanyip commented on code in PR #5925:
URL: https://github.com/apache/nifi/pull/5925#discussion_r850790494


##########
nifi-stateless/nifi-stateless-bootstrap/src/main/java/org/apache/nifi/stateless/bootstrap/StatelessBootstrap.java:
##########
@@ -181,58 +166,75 @@ private static BlockListClassLoader createExtensionRootClassLoader(final File na
                 filesAllowed.add(file.getName());
             }
         }
-        logger.debug("The following JAR files are proposed to be blocked from being loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesBlocked);
-        logger.debug("Of the full list above, the following JAR files will be explicitly allowed to be loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesAllowed);
 
-        classesBlocked.removeAll(classesAllowed);
-        filesBlocked.removeAll(filesAllowed);
-        logger.debug("The final list of JAR files blocked from being loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesBlocked);
+        final Set<File> javaHomeFiles = findJavaHomeFiles();
+        final Set<String> javaHomeFilenames = new HashSet<>();
+        for (final File file : javaHomeFiles) {
+            findLoadableClasses(file, classesAllowed);
+            javaHomeFilenames.add(file.getName());
+        }
 
-        logger.debug("The final list of classes blocked from being loaded by Stateless Extension ClassLoaders from parent {}: {}", parent, classesBlocked);
+        logger.debug("The following JAR files will be explicitly allowed to be loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, filesAllowed);
+        logger.debug("The following JAR/JMOD files from ${JAVA_HOME} will be explicitly allowed to be loaded by Stateless Extensions ClassLoaders from parent {}: {}", parent, javaHomeFilenames);
+        logger.debug("The final list of classes allowed to be loaded by Stateless Extension ClassLoaders from parent {}: {}", parent, classesAllowed);
 
-        final BlockListClassLoader blockingClassLoader = new BlockListClassLoader(parent, classesBlocked);
-        return blockingClassLoader;
+        final AllowListClassLoader allowListClassLoader = new AllowListClassLoader(parent, classesAllowed);
+        return allowListClassLoader;
     }
 
-    private static void findClassNamesInJars(final Collection<URL> jarUrls, final Set<String> classesFound, final Set<String> jarFilesFound) throws IOException {
-        final String javaHome = System.getProperty("java.home");
-        final File javaHomeDir = new File(javaHome);
-        final File javaLib = new File(javaHomeDir, "lib");
-        final String javaLibPath = javaLib.getAbsolutePath();
-
-        for (final URL url : jarUrls) {
-            final File file;
-            try {
-                file = new File(url.toURI());
-            } catch (URISyntaxException e) {
-                logger.warn("Could not find file for {} in classpath", url);
-                continue;
-            }
+    private static Set<File> findJavaHomeFiles() {
+        final String javaHomeValue = System.getProperty("java.home");
+        if (javaHomeValue == null) {
+            logger.warn("Could not find java.home system property so will not allow any classes explicitly from java.home in AllowListClassLoader");
+            return Collections.emptySet();
+        }
 
-            final String absolutePath = file.getAbsolutePath();
-            if (absolutePath.startsWith(javaLibPath)) {
-                continue;
-            }
+        final File javaHome = new File(javaHomeValue);
+        if (!javaHome.exists()) {
+            logger.warn("System property for java.home is {} but that directory does not exist so will not allow any classes explicitly from java.home in AllowListClassLoader", javaHomeValue);
+            return Collections.emptySet();
+        }
 
-            findClassNamesInJar(file, classesFound);
-            jarFilesFound.add(file.getName());
+        final File[] javaHomeFiles = javaHome.listFiles();
+        if (javaHomeFiles == null) {
+            logger.warn("System property for java.home is {} but that directory is not readable so will not allow any classes explicitly from java.home in AllowListClassLoader", javaHomeValue);
+            return Collections.emptySet();
         }
+
+        final Set<File> loadableFiles = new HashSet<>();
+        for (final File file : javaHomeFiles) {
+            findLoadableFiles(file, loadableFiles);
+        }
+
+        return loadableFiles;
     }
 
-    private static void findClassLoaderUrls(final ClassLoader classLoader, final Set<URL> urls) {
-        if (classLoader == null) {
+    private static void findLoadableFiles(final File file, final Set<File> loadable) {
+        if (file.isDirectory()) {
+            final File[] children = file.listFiles();
+            if (children == null) {
+                return;
+            }
+
+            for (final File child : children) {
+                findLoadableFiles(child, loadable);
+            }
+
             return;
         }
 
-        if (classLoader instanceof URLClassLoader) {
-            final URLClassLoader urlClassLoader = (URLClassLoader) classLoader;
-            urls.addAll(Arrays.asList(urlClassLoader.getURLs()));
+        final String filename = file.getName();
+        if (filename.endsWith(".class") || filename.endsWith(".jar") || filename.endsWith(".jmod")) {
+            loadable.add(file);
         }
+    }
 
-        // If the classLoader is the system class loader, we are done. We don't want to process the parent of
-        // the system class loader (which would be the Launcher$ExtClassLoader that contains the JDK/JRE classes, etc)
-        if (classLoader != ClassLoader.getSystemClassLoader()) {
-            findClassLoaderUrls(classLoader.getParent(), urls);
+    private static void findLoadableClasses(final File file, final Set<String> classNames) throws IOException {
+        final String filename = file.getName();
+        if (filename.endsWith(".jar")) {
+            findClassNamesInJar(file, classNames);
+        } else if (filename.endsWith(".jmod")) {
+            findClassesInJmod(file, classNames);

Review Comment:
   `.class` files are collected in `findLoadableFiles()` (line 227) but they are not handled here. It did not cause any issues in my flows but there can be other use cases when those classes are needed.



-- 
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