[GitHub] [kafka] gharris1727 commented on a diff in pull request #13356: KAFKA-14789: Prevent mis-attributing classpath plugins, allow discovery of classpath RestExtension and ConfigProvider

["gharris1727 (via GitHub)" <gi...@apache.org>]

gharris1727 commented on code in PR #13356:
URL: https://github.com/apache/kafka/pull/13356#discussion_r1205800242


##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/DelegatingClassLoader.java:
##########
@@ -431,15 +427,29 @@ private <T> Collection<PluginDesc<T>> getServiceLoaderPluginDesc(Class<T> klass,
                     log.error("Failed to discover {}{}", klass.getSimpleName(), reflectiveErrorDescription(t.getCause()), t);
                     continue;
                 }
-                result.add(pluginDesc((Class<? extends T>) pluginImpl.getClass(),
-                    versionFor(pluginImpl), loader));
+                Class<? extends T> pluginKlass = (Class<? extends T>) pluginImpl.getClass();
+                if (!isParentClassloader(pluginKlass.getClassLoader(), loader)) {
+                    log.debug("Exclude {} that is from classloader {}", pluginKlass.getSimpleName(), pluginKlass.getClassLoader());
+                    continue;
+                }
+                result.add(pluginDesc(pluginKlass, versionFor(pluginImpl), loader));
             }
         } finally {
             Plugins.compareAndSwapLoaders(savedLoader);
         }
         return result;
     }
 
+    private static boolean isParentClassloader(ClassLoader loader, ClassLoader parent) {
+        while (loader != null) {
+            if (loader == parent) {
+                return true;
+            }
+            loader = loader.getParent();
+        }
+        return false;

Review Comment:
   I wrote this in case a plugin came from a classloader which was a child of the PluginClassLoader. Thinking about it again, I don't know how a plugin would manage that, since the PluginClassLoader doesn't have any child-first logic that would permit it to load classes from deeper in the classloading hierarchy.
   
   I'll simplify this logic.



-- 
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: jira-unsubscribe@kafka.apache.org

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