[JENKINS-70870] Save libraries as JAR files rather than unpacked

pom.xml

@@ -71,7 +71,7 @@
             <dependency>
                 <groupId>io.jenkins.tools.bom</groupId>
                 <artifactId>bom-2.361.x</artifactId>
-                <version>1750.v0071fa_4c4a_e3</version>
+                <version>1886.va_11c9f461054</version>
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>

src/main/java/org/jenkinsci/plugins/workflow/cps/global/UserDefinedGlobalVariable.java

@@ -22,6 +22,7 @@
  */
 // not @Extension because these are instantiated programmatically
 public class UserDefinedGlobalVariable extends GlobalVariable {
+    // TODO switch to URL
     private final File help;
     private final String name;
 

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

@@ -24,7 +24,6 @@
 
 package org.jenkinsci.plugins.workflow.libs;
 
-import hudson.AbortException;
 import hudson.Extension;
 import hudson.ExtensionList;
 import hudson.FilePath;
@@ -50,6 +49,9 @@
 import java.util.logging.Logger;
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
+import java.util.jar.JarEntry;
+import java.util.jar.JarFile;
+import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.apache.commons.io.IOUtils;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
@@ -97,12 +99,9 @@
         if (action != null) {
             // Resuming a build, so just look up what we loaded before.
             for (LibraryRecord record : action.getLibraries()) {
-                FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
-                for (String root : new String[] {"src", "vars"}) {
-                    FilePath dir = libDir.child(root);
-                    if (dir.isDirectory()) {
-                        additions.add(new Addition(dir.toURI().toURL(), record.trusted));
-                    }
+                FilePath libJar = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName() + ".jar");
+                if (libJar.exists()) {
+                    additions.add(new Addition(libJar.toURI().toURL(), record.trusted));
                 }
                 String unparsed = librariesUnparsed.get(record.name);
                 if (unparsed != null) {
@@ -147,9 +146,7 @@
         // Now actually try to retrieve the libraries.
         for (LibraryRecord record : librariesAdded.values()) {
             listener.getLogger().println("Loading library " + record.name + "@" + record.version);
-            for (URL u : retrieve(record, retrievers.get(record.name), listener, build, execution)) {
-                additions.add(new Addition(u, record.trusted));
-            }
+            additions.add(new Addition(retrieve(record, retrievers.get(record.name), listener, build, execution), record.trusted));
         }
         return additions;
     }
@@ -169,14 +166,14 @@ private enum CacheStatus {
         EXPIRED;
     }
 
-    private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheDir)
+    private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration cachingConfiguration, @NonNull final FilePath versionCacheJar)
           throws IOException, InterruptedException
     {
         if (cachingConfiguration.isRefreshEnabled()) {
             final long cachingMilliseconds = cachingConfiguration.getRefreshTimeMilliseconds();
 
-            if(versionCacheDir.exists()) {
-                if ((versionCacheDir.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) {
+            if(versionCacheJar.exists()) {
+                if ((versionCacheJar.lastModified() + cachingMilliseconds) > System.currentTimeMillis()) {
                     return CacheStatus.VALID;
                 } else {
                     return CacheStatus.EXPIRED;
@@ -185,7 +182,7 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c
                 return CacheStatus.DOES_NOT_EXIST;
             }
         } else {
-            if (versionCacheDir.exists()) {
+            if (versionCacheJar.exists()) {
                 return CacheStatus.VALID;
             } else {
                 return CacheStatus.DOES_NOT_EXIST;
@@ -194,16 +191,16 @@ private static CacheStatus getCacheStatus(@NonNull LibraryCachingConfiguration c
     }
 
     /** Retrieve library files. */
-    static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run<?,?> run, @NonNull CpsFlowExecution execution) throws Exception {
+    static URL retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriever retriever, @NonNull TaskListener listener, @NonNull Run<?,?> run, @NonNull CpsFlowExecution execution) throws Exception {
         String name = record.name;
         String version = record.version;
         boolean changelog = record.changelog;
         LibraryCachingConfiguration cachingConfiguration = record.cachingConfiguration;
-        FilePath libDir = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName());
+        FilePath libJar = new FilePath(execution.getOwner().getRootDir()).child("libs/" + record.getDirectoryName() + ".jar");
         Boolean shouldCache = cachingConfiguration != null;
-        final FilePath versionCacheDir = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName());
+        final FilePath versionCacheJar = new FilePath(LibraryCachingConfiguration.getGlobalLibrariesCacheDir(), record.getDirectoryName() + ".jar");
         ReentrantReadWriteLock retrieveLock = getReadWriteLockFor(record.getDirectoryName());
-        final FilePath lastReadFile = new FilePath(versionCacheDir, LibraryCachingConfiguration.LAST_READ_FILE);
+        final FilePath lastReadFile = versionCacheJar.sibling(record.getDirectoryName() + "." + LibraryCachingConfiguration.LAST_READ_FILE);
 
         if(shouldCache && cachingConfiguration.isExcluded(version)) {
             listener.getLogger().println("Library " + name + "@" + version + " is excluded from caching.");
@@ -213,13 +210,13 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
         if(shouldCache) {
             retrieveLock.readLock().lockInterruptibly();
             try {
-                CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheDir);
+                CacheStatus cacheStatus = getCacheStatus(cachingConfiguration, versionCacheJar);
                 if (cacheStatus == CacheStatus.DOES_NOT_EXIST || cacheStatus == CacheStatus.EXPIRED) {
                     retrieveLock.readLock().unlock();
                     retrieveLock.writeLock().lockInterruptibly();
                     try {
                       boolean retrieve = false;
-                      switch (getCacheStatus(cachingConfiguration, versionCacheDir)) {
+                      switch (getCacheStatus(cachingConfiguration, versionCacheJar)) {
                           case VALID: 
                               listener.getLogger().println("Library " + name + "@" + version + " is cached. Copying from home."); 
                                 break;
@@ -229,18 +226,16 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
                           case EXPIRED:
                               long cachingMinutes = cachingConfiguration.getRefreshTimeMinutes();
                               listener.getLogger().println("Library " + name + "@" + version + " is due for a refresh after " + cachingMinutes + " minutes, clearing.");
-                                if (versionCacheDir.exists()) {
-                                    versionCacheDir.deleteRecursive();
-                                    versionCacheDir.withSuffix("-name.txt").delete();
+                                if (versionCacheJar.exists()) {
+                                    versionCacheJar.delete();
                                 }
                                 retrieve = true;
                                 break;
                       }
 
                         if (retrieve) {
                             listener.getLogger().println("Caching library " + name + "@" + version);                            
-                            versionCacheDir.mkdirs();
-                            retriever.retrieve(name, version, changelog, versionCacheDir, run, listener);
+                            retriever.retrieveJar(name, version, changelog, versionCacheJar, run, listener);
                         }
                         retrieveLock.readLock().lock();
                     } finally {
@@ -251,50 +246,41 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
                 }
 
                 lastReadFile.touch(System.currentTimeMillis());
-                versionCacheDir.withSuffix("-name.txt").write(name, "UTF-8");
-                versionCacheDir.copyRecursiveTo(libDir);
+                versionCacheJar.copyTo(libJar);
             } finally {
               retrieveLock.readLock().unlock();
             }
         } else {
-            retriever.retrieve(name, version, changelog, libDir, run, listener);
+            retriever.retrieveJar(name, version, changelog, libJar, run, listener);
         }
-        // Write the user-provided name to a file as a debugging aid.
-        libDir.withSuffix("-name.txt").write(name, "UTF-8");
 
         // Replace any classes requested for replay:
         if (!record.trusted) {
             for (String clazz : ReplayAction.replacementsIn(execution)) {
-                for (String root : new String[] {"src", "vars"}) {
-                    String rel = root + "/" + clazz.replace('.', '/') + ".groovy";
-                    FilePath f = libDir.child(rel);
-                    if (f.exists()) {
-                        String replacement = ReplayAction.replace(execution, clazz);
-                        if (replacement != null) {
-                            listener.getLogger().println("Replacing contents of " + rel);
-                            f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler
-                        }
+                String rel = clazz.replace('.', '/') + ".groovy";
+                /* TODO need to unpack & repack I guess
+                FilePath f = libDir.child(rel);
+                if (f.exists()) {
+                    String replacement = ReplayAction.replace(execution, clazz);
+                    if (replacement != null) {
+                        listener.getLogger().println("Replacing contents of " + rel);
+                        f.write(replacement, null); // TODO as below, unsure of encoding used by Groovy compiler
                     }
                 }
+                */
             }
         }
-        List<URL> urls = new ArrayList<>();
-        FilePath srcDir = libDir.child("src");
-        if (srcDir.isDirectory()) {
-            urls.add(srcDir.toURI().toURL());
-        }
-        FilePath varsDir = libDir.child("vars");
-        if (varsDir.isDirectory()) {
-            urls.add(varsDir.toURI().toURL());
-            for (FilePath var : varsDir.list("*.groovy")) {
-                record.variables.add(var.getBaseName());
-            }
-        }
-        if (urls.isEmpty()) {
-            throw new AbortException("Library " + name + " expected to contain at least one of src or vars directories");
+        try (JarFile jf = new JarFile(libJar.getRemote())) {
+            jf.stream().forEach(entry -> {
+                Matcher m = ROOT_GROOVY_SOURCE.matcher(entry.getName());
+                if (m.matches()) {
+                    record.variables.add(m.group(1));
+                }
+            });
         }
-        return urls;
+        return libJar.toURI().toURL();
     }
+    private static final Pattern ROOT_GROOVY_SOURCE = Pattern.compile("([^/]+)[.]groovy");
 
     /**
      * Loads resources for {@link ResourceStep}.
@@ -311,29 +297,25 @@ static List<URL> retrieve(@NonNull LibraryRecord record, @NonNull LibraryRetriev
             if (action != null) {
                 FilePath libs = new FilePath(run.getRootDir()).child("libs");
                 for (LibraryRecord library : action.getLibraries()) {
-                    FilePath libResources = libs.child(library.getDirectoryName() + "/resources/");
-                    FilePath f = libResources.child(name);
-                    if (!new File(f.getRemote()).getCanonicalFile().toPath().startsWith(new File(libResources.getRemote()).getCanonicalPath())) {
-                        throw new AbortException(name + " references a file that is not contained within the library: " + library.name);
-                    } else if (f.exists()) {
-                        resources.put(library.name, readResource(f, encoding));
+                    FilePath libJar = libs.child(library.getDirectoryName() + ".jar");
+                    try (JarFile jf = new JarFile(libJar.getRemote())) {
+                        JarEntry je = jf.getJarEntry("resources/" + name);
+                        if (je != null) {
+                            try (InputStream in = jf.getInputStream(je)) {
+                                if ("Base64".equals(encoding)) {
+                                    resources.put(library.name, Base64.getEncoder().encodeToString(IOUtils.toByteArray(in)));
+                                } else {
+                                    resources.put(library.name, IOUtils.toString(in, encoding)); // The platform default is used if encoding is null.
+                                }
+                            }
+                        }
                     }
                 }
             }
         }
         return resources;
     }
 
-    private static String readResource(FilePath file, @CheckForNull String encoding) throws IOException, InterruptedException {
-        try (InputStream in = file.read()) {
-            if ("Base64".equals(encoding)) {
-                return Base64.getEncoder().encodeToString(IOUtils.toByteArray(in));
-            } else {
-                return IOUtils.toString(in, encoding); // The platform default is used if encoding is null.
-            }
-        }
-    }
-
     @Extension public static class GlobalVars extends GlobalVariableSet {
 
         @Override public Collection<GlobalVariable> forRun(Run<?,?> run) {
@@ -347,6 +329,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding)
             List<GlobalVariable> vars = new ArrayList<>();
             for (LibraryRecord library : action.getLibraries()) {
                 for (String variable : library.variables) {
+                    // TODO pass URL of *.jar!/$variable.txt
                     vars.add(new UserDefinedGlobalVariable(variable, new File(run.getRootDir(), "libs/" + library.getDirectoryName() + "/vars/" + variable + ".txt")));
                 }
             }
@@ -367,6 +350,7 @@ private static String readResource(FilePath file, @CheckForNull String encoding)
                     Run<?,?> run = (Run) executable;
                     LibrariesAction action = run.getAction(LibrariesAction.class);
                     if (action != null) {
+                        // TODO handle *.jar
                         FilePath libs = new FilePath(run.getRootDir()).child("libs");
                         for (LibraryRecord library : action.getLibraries()) {
                             if (library.trusted) {

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

@@ -9,6 +9,7 @@
 import java.io.IOException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
+import jenkins.model.Jenkins;
 
 import jenkins.util.SystemProperties;
 
@@ -27,41 +28,28 @@ public LibraryCachingCleanup() {
 
     @Override protected void execute(TaskListener listener) throws IOException, InterruptedException {
         FilePath globalCacheDir = LibraryCachingConfiguration.getGlobalLibrariesCacheDir();
-        for (FilePath library : globalCacheDir.list()) {
-            if (!removeIfExpiredCacheDirectory(library)) {
-                // Prior to the SECURITY-2586 fix, library caches had a two-level directory structure.
-                // These caches will never be used again, so we delete any that we find.
-                for (FilePath version: library.list()) {
-                    if (version.child(LibraryCachingConfiguration.LAST_READ_FILE).exists()) {
-                        library.deleteRecursive();
-                        break;
-                    }
-                }
-            }
+        for (FilePath libJar : globalCacheDir.list()) {
+            removeIfExpiredCacheJar(libJar);
         }
+        // Old cache directory; format has changed, so just delete it:
+        Jenkins.get().getRootPath().child("global-libraries-cache").deleteRecursive();
     }
 
     /**
-     * Delete the specified cache directory if it is outdated.
-     * @return true if specified directory is a cache directory, regardless of whether it was outdated. Used to detect
-     * whether the cache was created before or after the fix for SECURITY-2586.
+     * Delete the specified cache JAR if it is outdated.
      */
-    private boolean removeIfExpiredCacheDirectory(FilePath library) throws IOException, InterruptedException {
-        final FilePath lastReadFile = new FilePath(library, LibraryCachingConfiguration.LAST_READ_FILE);
+    private void removeIfExpiredCacheJar(FilePath libJar) throws IOException, InterruptedException {
+        final FilePath lastReadFile = libJar.sibling(libJar.getBaseName() + "." + LibraryCachingConfiguration.LAST_READ_FILE);
         if (lastReadFile.exists()) {
-            ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(library.getName());
+            ReentrantReadWriteLock retrieveLock = LibraryAdder.getReadWriteLockFor(libJar.getBaseName());
             retrieveLock.writeLock().lockInterruptibly();
             try {
                 if (System.currentTimeMillis() - lastReadFile.lastModified() > TimeUnit.DAYS.toMillis(EXPIRE_AFTER_READ_DAYS)) {
-
-                    library.deleteRecursive();
-                    library.withSuffix("-name.txt").delete();
+                    libJar.delete();
                 }
             } finally {
                 retrieveLock.writeLock().unlock();
             }
-            return true;
         }
-        return false;
     }
 }