[GitHub] [pulsar] merlimat commented on a change in pull request #10922: Explicitly set the Pulsar function classloader

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

merlimat commented on a change in pull request #10922:
URL: https://github.com/apache/pulsar/pull/10922#discussion_r651143362



##########
File path: pulsar-client-api/src/main/java/org/apache/pulsar/client/internal/ReflectionUtils.java
##########
@@ -21,15 +21,30 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
 import lombok.experimental.UtilityClass;
 
 @UtilityClass
-class ReflectionUtils {
-    interface SupplierWithException<T> {
+public class ReflectionUtils {
+
+    private static final Map<String, Class<?>> loadedClasses = new ConcurrentHashMap<>();
+
+    private static volatile ClassLoader classLoader = ReflectionUtils.class.getClassLoader();
+
+    /**
+     * Change the classloader to be used for loading Pulsar client implementation classes
+     * @param newClassLoader
+     */
+    public static void setClassLoader(ClassLoader newClassLoader) {
+        classLoader = newClassLoader;

Review comment:
       > This may sound like a security hole unfortunately.
   In an environment where you are simply running the Pulsar Client and there are multiple users one malicious user may try to override this classloader and inject bad stuff.
   
   If you're running multiple instances of untrusted code within the same JVM, this is going to be the very last of all the issues.
   
   > This mechanism makes sense only in Pulsar Functions runtime.
   We should add something (a system property?) To enable this method
   or at least allow this method to be called only once.
   
   Yes, we just need to do it once, although I don't think it would be a problem to do it once per each function instance since the framework classloader is always going to be the same classloader instance.
   
   Using a system property, I don't think we can specify the instance of the classloader.
   
   




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

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