[FIXED JENKINS-44898] Add findResource to PluginFirstClassLoader (#2916)

* [FIXED JENKINS-44898] Add findResource to PluginFirstClassLoader This fixes GroovyClassLoader.loadClass for a .groovy file in a plugin with a PluginFirstClassLoader, specifically by fixing fast-loading via the UberClassLoader. * Move common code to AntWithFindResourceClassLoader. * Adding testing of new PluginFirstClassLoader behavior.
jenkinsci · Jun 17, 2017 · 303a9f7 · 303a9f7
1 parent a5dc255
commit 303a9f7
Show file tree

Hide file tree

Showing 5 changed files with 91 additions and 50 deletions.
diff --git a/core/src/main/java/hudson/ClassicPluginStrategy.java b/core/src/main/java/hudson/ClassicPluginStrategy.java
@@ -26,6 +26,8 @@
 import java.io.InputStream;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
+
+import jenkins.util.AntWithFindResourceClassLoader;
 import jenkins.util.SystemProperties;
 import com.google.common.collect.Lists;
 import hudson.Plugin.DummyImpl;
@@ -830,53 +832,11 @@ protected URL findResource(String name) {
     /**
      * {@link AntClassLoader} with a few methods exposed, {@link Closeable} support, and {@link Transformer} support.
      */
-    private final class AntClassLoader2 extends AntClassLoader implements Closeable {
-        private final Vector pathComponents;
-
+    private final class AntClassLoader2 extends AntWithFindResourceClassLoader implements Closeable {
         private AntClassLoader2(ClassLoader parent) {
-            super(parent,true);
-
-            try {
-                Field $pathComponents = AntClassLoader.class.getDeclaredField("pathComponents");
-                $pathComponents.setAccessible(true);
-                pathComponents = (Vector)$pathComponents.get(this);
-            } catch (NoSuchFieldException | IllegalAccessException e) {
-                throw new Error(e);
-            }
+            super(parent, true);
         }
-
-
-        public void addPathFiles(Collection<File> paths) throws IOException {
-            for (File f : paths)
-                addPathFile(f);
-        }
-
-        public void close() throws IOException {
-            cleanup();
-        }
-
-        /**
-         * As of 1.8.0, {@link AntClassLoader} doesn't implement {@link #findResource(String)}
-         * in any meaningful way, which breaks fast lookup. Implement it properly.
-         */
-        @Override
-        protected URL findResource(String name) {
-            URL url = null;
-
-            // try and load from this loader if the parent either didn't find
-            // it or wasn't consulted.
-            Enumeration e = pathComponents.elements();
-            while (e.hasMoreElements() && url == null) {
-                File pathComponent = (File) e.nextElement();
-                url = getResourceURL(pathComponent, name);
-                if (url != null) {
-                    log("Resource " + name + " loaded from ant loader", Project.MSG_DEBUG);
-                }
-            }
-
-            return url;
-        }
-
+
         @Override
         protected Class defineClassFromData(File container, byte[] classData, String classname) throws IOException {
             if (!DISABLE_TRANSFORMER)

diff --git a/core/src/main/java/hudson/PluginFirstClassLoader.java b/core/src/main/java/hudson/PluginFirstClassLoader.java
@@ -33,7 +33,7 @@
 import java.util.Enumeration;
 import java.util.List;
 
-import org.apache.tools.ant.AntClassLoader;
+import jenkins.util.AntWithFindResourceClassLoader;
 
 /**
  * classLoader which use first /WEB-INF/lib/*.jar and /WEB-INF/classes before core classLoader
@@ -42,10 +42,14 @@
  * @since 1.371
  */
 public class PluginFirstClassLoader
-    extends AntClassLoader
+    extends AntWithFindResourceClassLoader
     implements Closeable
 {
-
+
+    public PluginFirstClassLoader() {
+        super(null, false);
+    }
+
     private List<URL> urls = new ArrayList<URL>();
 
     public void addPathFiles( Collection<File> paths )
@@ -100,6 +104,5 @@ public InputStream getResourceAsStream( String name )
     {
         InputStream is = super.getResourceAsStream( name );
         return is;
-    }   
-
+    }
 }
diff --git a/core/src/main/java/jenkins/util/AntWithFindResourceClassLoader.java b/core/src/main/java/jenkins/util/AntWithFindResourceClassLoader.java
@@ -0,0 +1,60 @@
+package jenkins.util;
+
+import org.apache.tools.ant.Project;
+
+import java.io.Closeable;
+import java.io.File;
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.net.URL;
+import java.util.Collection;
+import java.util.Enumeration;
+import java.util.Vector;
+
+/**
+ * As of 1.8.0, {@link org.apache.tools.ant.AntClassLoader} doesn't implement {@link #findResource(String)}
+ * in any meaningful way, which breaks fast lookup. Implement it properly.
+ */
+public class AntWithFindResourceClassLoader extends AntClassLoader implements Closeable {
+    private final Vector pathComponents;
+
+    public AntWithFindResourceClassLoader(ClassLoader parent, boolean parentFirst) {
+        super(parent, parentFirst);
+
+        try {
+            Field $pathComponents = AntClassLoader.class.getDeclaredField("pathComponents");
+            $pathComponents.setAccessible(true);
+            pathComponents = (Vector)$pathComponents.get(this);
+        } catch (NoSuchFieldException | IllegalAccessException e) {
+            throw new Error(e);
+        }
+    }
+
+    public void addPathFiles(Collection<File> paths) throws IOException {
+        for (File f : paths)
+            addPathFile(f);
+    }
+
+    public void close() throws IOException {
+        cleanup();
+    }
+
+    @Override
+    protected URL findResource(String name) {
+        URL url = null;
+
+        // try and load from this loader if the parent either didn't find
+        // it or wasn't consulted.
+        Enumeration e = pathComponents.elements();
+        while (e.hasMoreElements() && url == null) {
+            File pathComponent = (File) e.nextElement();
+            url = getResourceURL(pathComponent, name);
+            if (url != null) {
+                log("Resource " + name + " loaded from ant loader", Project.MSG_DEBUG);
+            }
+        }
+
+        return url;
+    }
+
+}
diff --git a/test/src/test/java/hudson/PluginManagerTest.java b/test/src/test/java/hudson/PluginManagerTest.java
@@ -43,6 +43,8 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.concurrent.Future;
+
+import jenkins.ClassLoaderReflectionToolkit;
 import jenkins.RestartRequiredException;
 import net.sf.json.JSONArray;
 import net.sf.json.JSONObject;
@@ -515,4 +517,20 @@ private void dynamicLoadAndDisable(String plugin) throws IOException, Interrupte
         assertNotNull(r.jenkins.getPluginManager().getPlugin("Parameterized-Remote-Trigger"));
         assertNotNull(r.jenkins.getPluginManager().getPlugin("token-macro"));
     }
+
+    @Issue("JENKINS-44898")
+    @WithPlugin("plugin-first.hpi")
+    @Test
+    public void findResourceForPluginFirstClassLoader() throws Exception {
+        PluginWrapper w = r.jenkins.getPluginManager().getPlugin("plugin-first");
+        assertNotNull(w);
+
+        URL fromPlugin = w.classLoader.getResource("org/jenkinsci/plugins/pluginfirst/HelloWorldBuilder/config.jelly");
+        assertNotNull(fromPlugin);
+
+        // This is how UberClassLoader.findResource functions.
+        URL fromToolkit = ClassLoaderReflectionToolkit._findResource(w.classLoader, "org/jenkinsci/plugins/pluginfirst/HelloWorldBuilder/config.jelly");
+
+        assertEquals(fromPlugin, fromToolkit);
+    }
 }
diff --git a/test/src/test/resources/plugins/plugin-first.hpi b/test/src/test/resources/plugins/plugin-first.hpi