Giving up on jenkinsci/workflow-cps-plugin#668 and tracking code source more conservatively

Author: jglick

pom.xml

@@ -75,11 +75,6 @@
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>
-            <dependency>
-                <groupId>org.jenkins-ci.plugins.workflow</groupId>
-                <artifactId>workflow-cps</artifactId>
-                <version>3625.v4390b_f51b_50f</version> <!-- TODO https://github.com/jenkinsci/workflow-cps-plugin/pull/668 -->
-            </dependency>
         </dependencies>
     </dependencyManagement>
     <dependencies>

src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

@@ -45,21 +45,20 @@
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.net.URL;
-import java.nio.file.Paths;
-import java.security.CodeSource;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 import java.util.TreeSet;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
 import groovy.lang.MissingPropertyException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 import javax.inject.Inject;
 import jenkins.model.Jenkins;
 import jenkins.scm.impl.SingleSCMSource;
@@ -251,20 +250,20 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S
         private final @NonNull String prefix;
         /** {@link Class#getName} minus package prefix */
         private final @CheckForNull String clazz;
-        /** {@code jar:file:/…/libs/$hash.jar!/} */
-        private final @NonNull String srcUrl;
+        /** {@code /…/libs/$hash.jar}, or null if resuming a pre-dir2Jar build */
+        private final @Nullable String jar;
 
         LoadedClasses(String library, String libraryDirectoryName, boolean trusted, Boolean changelog, Run<?,?> run) {
-            this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ "jar:" + new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").toURI() + "!/");
+            this(library, trusted, changelog, "", null, /* cf. LibraryAdder.retrieve */ new File(run.getRootDir(), "libs/" + libraryDirectoryName + ".jar").getAbsolutePath());
         }
 
-        LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String srcUrl) {
+        LoadedClasses(String library, boolean trusted, Boolean changelog, String prefix, String clazz, String jar) {
             this.library = library;
             this.trusted = trusted;
             this.changelog = changelog;
             this.prefix = prefix;
             this.clazz = clazz;
-            this.srcUrl = srcUrl;
+            this.jar = jar;
         }
 
         @Override public Object getProperty(String property) {
@@ -288,12 +287,12 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S
                 String fullClazz = clazz != null ? clazz + '$' + property : property;
                 loadClass(prefix + fullClazz);
                 // OK, class really exists, stash it and await methods
-                return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, srcUrl);
+                return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, jar);
             } else if (clazz != null) {
                 throw new MissingPropertyException(property, loadClass(prefix + clazz));
             } else {
                 // Still selecting package components.
-                return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, srcUrl);
+                return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, jar);
             }
         }
 
@@ -339,6 +338,8 @@ private static boolean isSandboxed() {
 
         // TODO putProperty for static field set
 
+        private static final Pattern JAR_URL = Pattern.compile("jar:(file:/.+[.]jar)!/.+");
+
         private Class<?> loadClass(String name) {
             CpsFlowExecution exec = CpsThread.current().getExecution();
             GroovyClassLoader loader = (trusted ? exec.getTrustedShell() : exec.getShell()).getClassLoader();
@@ -351,14 +352,22 @@ private Class<?> loadClass(String name) {
                 if (definingLoader != loader) {
                     throw new IllegalAccessException("cannot access " + c + " via library handle: " + definingLoader + " is not " + loader);
                 }
-                // Note that this goes through GroovyCodeSource.<init>(File, String), which unlike (say) URLClassLoader set the “location” to the actual file, *not* the root.
-                CodeSource codeSource = c.getProtectionDomain().getCodeSource();
-                if (codeSource == null) {
-                    throw new IllegalAccessException(name + " had no defined code source");
-                }
-                String loc = codeSource.getLocation().toString();
-                if (!loc.startsWith(srcUrl)) {
-                    throw new IllegalAccessException(name + " was defined in " + loc + " which was not inside " + srcUrl);
+                if (jar != null) {
+                    URL res = loader.getResource(name.replaceFirst("[$][^.]+$", "").replace('.', '/') + ".groovy");
+                    if (res == null) {
+                        throw new IllegalAccessException("Unknown where " + name + " (" + c.getProtectionDomain().getCodeSource().getLocation() + ") was loaded from");
+                    }
+                    Matcher m = JAR_URL.matcher(res.toString());
+                    if (!m.matches()) {
+                        throw new IllegalAccessException("Unexpected URL " + res);
+                    }
+                    File actual = new File(URI.create(m.group(1)));
+                    if (!actual.equals(new File(jar))) {
+                        throw new IllegalAccessException(name + " was defined in " + actual + " rather than the expected " + jar);
+                    }
+                    LOGGER.fine(() -> "loaded " + name + " from " + res + " ~ " + actual + " as expected");
+                } else {
+                    LOGGER.fine(() -> "loaded " + name + " but resuming from an old build which did not properly record JAR location");
                 }
                 if (!Modifier.isPublic(c.getModifiers())) { // unlikely since Groovy makes classes implicitly public
                     throw new IllegalAccessException(c + " is not public");

src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java

@@ -36,6 +36,7 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.logging.Level;
 
 import jenkins.plugins.git.GitSCMSource;
 import jenkins.plugins.git.GitSampleRepoRule;
@@ -56,6 +57,7 @@
 import org.jvnet.hudson.test.BuildWatcher;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
 
 @Issue("JENKINS-39450")
 public class LibraryStepTest {
@@ -64,6 +66,7 @@ public class LibraryStepTest {
     @Rule public JenkinsRule r = new JenkinsRule();
     @Rule public GitSampleRepoRule sampleRepo = new GitSampleRepoRule();
     @Rule public GitSampleRepoRule sampleRepo2 = new GitSampleRepoRule();
+    @Rule public LoggerRule logging = new LoggerRule().record(LibraryStep.class, Level.FINE);
 
     @Test public void configRoundtrip() throws Exception {
         StepConfigTester stepTester = new StepConfigTester(r);