From 21b15558a29e325d424dc6ad0f4c4192ba168955 Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 20 Aug 2024 15:47:44 -0500 Subject: [PATCH] Use classpath index when building classpath in PropertiesLauncher Fixes gh-41719 --- .../launch/ExecutableArchiveLauncher.java | 46 -------------- .../boot/loader/launch/JarLauncher.java | 18 ------ .../boot/loader/launch/Launcher.java | 59 ++++++++++++++++++ .../loader/launch/PropertiesLauncher.java | 10 ++- .../boot/loader/launch/WarLauncher.java | 8 +-- ...rTests.java => AbstractLauncherTests.java} | 9 ++- .../boot/loader/launch/JarLauncherTests.java | 6 +- .../launch/PropertiesLauncherTests.java | 62 +++++++++++++++++-- .../boot/loader/launch/WarLauncherTests.java | 6 +- 9 files changed, 135 insertions(+), 89 deletions(-) rename spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/{AbstractExecutableArchiveLauncherTests.java => AbstractLauncherTests.java} (93%) diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/ExecutableArchiveLauncher.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/ExecutableArchiveLauncher.java index fd6bd8cf527c..6992b6718d8b 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/ExecutableArchiveLauncher.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/ExecutableArchiveLauncher.java @@ -16,16 +16,12 @@ package org.springframework.boot.loader.launch; -import java.io.IOException; import java.net.URL; import java.util.ArrayList; import java.util.Collection; import java.util.Set; -import java.util.jar.Attributes; import java.util.jar.Manifest; -import org.springframework.boot.loader.launch.Archive.Entry; - /** * Base class for a {@link Launcher} backed by an executable archive. * @@ -41,14 +37,8 @@ public abstract class ExecutableArchiveLauncher extends Launcher { private static final String START_CLASS_ATTRIBUTE = "Start-Class"; - protected static final String BOOT_CLASSPATH_INDEX_ATTRIBUTE = "Spring-Boot-Classpath-Index"; - - protected static final String DEFAULT_CLASSPATH_INDEX_FILE_NAME = "classpath.idx"; - private final Archive archive; - private final ClassPathIndexFile classPathIndex; - public ExecutableArchiveLauncher() throws Exception { this(Archive.create(Launcher.class)); } @@ -58,21 +48,6 @@ protected ExecutableArchiveLauncher(Archive archive) throws Exception { this.classPathIndex = getClassPathIndex(this.archive); } - ClassPathIndexFile getClassPathIndex(Archive archive) throws IOException { - if (!archive.isExploded()) { - return null; // Regular archives already have a defined order - } - String location = getClassPathIndexFileLocation(archive); - return ClassPathIndexFile.loadIfPossible(archive.getRootDirectory(), location); - } - - private String getClassPathIndexFileLocation(Archive archive) throws IOException { - Manifest manifest = archive.getManifest(); - Attributes attributes = (manifest != null) ? manifest.getMainAttributes() : null; - String location = (attributes != null) ? attributes.getValue(BOOT_CLASSPATH_INDEX_ATTRIBUTE) : null; - return (location != null) ? location : getEntryPathPrefix() + DEFAULT_CLASSPATH_INDEX_FILE_NAME; - } - @Override protected ClassLoader createClassLoader(Collection urls) throws Exception { if (this.classPathIndex != null) { @@ -102,13 +77,6 @@ protected Set getClassPathUrls() throws Exception { return this.archive.getClassPathUrls(this::isIncludedOnClassPathAndNotIndexed, this::isSearchedDirectory); } - private boolean isIncludedOnClassPathAndNotIndexed(Entry entry) { - if (!isIncludedOnClassPath(entry)) { - return false; - } - return (this.classPathIndex == null) || !this.classPathIndex.containsEntry(entry.name()); - } - /** * Determine if the specified directory entry is a candidate for further searching. * @param entry the entry to check @@ -119,18 +87,4 @@ protected boolean isSearchedDirectory(Archive.Entry entry) { && !isIncludedOnClassPath(entry); } - /** - * Determine if the specified entry is a nested item that should be added to the - * classpath. - * @param entry the entry to check - * @return {@code true} if the entry is a nested item (jar or directory) - */ - protected abstract boolean isIncludedOnClassPath(Archive.Entry entry); - - /** - * Return the path prefix for relevant entries in the archive. - * @return the entry path prefix - */ - protected abstract String getEntryPathPrefix(); - } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/JarLauncher.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/JarLauncher.java index 3a6d1339ca11..7569fa4f2668 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/JarLauncher.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/JarLauncher.java @@ -36,24 +36,6 @@ protected JarLauncher(Archive archive) throws Exception { super(archive); } - @Override - protected boolean isIncludedOnClassPath(Archive.Entry entry) { - return isLibraryFileOrClassesDirectory(entry); - } - - @Override - protected String getEntryPathPrefix() { - return "BOOT-INF/"; - } - - static boolean isLibraryFileOrClassesDirectory(Archive.Entry entry) { - String name = entry.name(); - if (entry.isDirectory()) { - return name.equals("BOOT-INF/classes/"); - } - return name.startsWith("BOOT-INF/lib/"); - } - public static void main(String[] args) throws Exception { new JarLauncher().launch(args); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/Launcher.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/Launcher.java index 2cae9b06b916..54ae5ef8eae9 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/Launcher.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/Launcher.java @@ -16,12 +16,16 @@ package org.springframework.boot.loader.launch; +import java.io.IOException; import java.io.UncheckedIOException; import java.lang.reflect.Method; import java.net.URL; import java.util.Collection; import java.util.Set; +import java.util.jar.Attributes; +import java.util.jar.Manifest; +import org.springframework.boot.loader.launch.Archive.Entry; import org.springframework.boot.loader.net.protocol.Handlers; /** @@ -30,12 +34,19 @@ * * @author Phillip Webb * @author Dave Syer + * @author Scott Frederick * @since 3.2.0 */ public abstract class Launcher { private static final String JAR_MODE_RUNNER_CLASS_NAME = JarModeRunner.class.getName(); + protected static final String BOOT_CLASSPATH_INDEX_ATTRIBUTE = "Spring-Boot-Classpath-Index"; + + protected static final String DEFAULT_CLASSPATH_INDEX_FILE_NAME = "classpath.idx"; + + protected ClassPathIndexFile classPathIndex; + /** * Launch the application. This method is the initial entry point that should be * called by a subclass {@code public static void main(String[] args)} method. @@ -102,6 +113,21 @@ protected boolean isExploded() { return (archive != null) && archive.isExploded(); } + ClassPathIndexFile getClassPathIndex(Archive archive) throws IOException { + if (!archive.isExploded()) { + return null; // Regular archives already have a defined order + } + String location = getClassPathIndexFileLocation(archive); + return ClassPathIndexFile.loadIfPossible(archive.getRootDirectory(), location); + } + + private String getClassPathIndexFileLocation(Archive archive) throws IOException { + Manifest manifest = archive.getManifest(); + Attributes attributes = (manifest != null) ? manifest.getMainAttributes() : null; + String location = (attributes != null) ? attributes.getValue(BOOT_CLASSPATH_INDEX_ATTRIBUTE) : null; + return (location != null) ? location : getEntryPathPrefix() + DEFAULT_CLASSPATH_INDEX_FILE_NAME; + } + /** * Return the archive being launched or {@code null} if there is no archive. * @return the launched archive @@ -122,4 +148,37 @@ protected boolean isExploded() { */ protected abstract Set getClassPathUrls() throws Exception; + /** + * Return the path prefix for relevant entries in the archive. + * @return the entry path prefix + */ + protected String getEntryPathPrefix() { + return "BOOT-INF/"; + } + + /** + * Determine if the specified entry is a nested item that should be added to the + * classpath. + * @param entry the entry to check + * @return {@code true} if the entry is a nested item (jar or directory) + */ + protected boolean isIncludedOnClassPath(Archive.Entry entry) { + return isLibraryFileOrClassesDirectory(entry); + } + + protected boolean isLibraryFileOrClassesDirectory(Archive.Entry entry) { + String name = entry.name(); + if (entry.isDirectory()) { + return name.equals("BOOT-INF/classes/"); + } + return name.startsWith("BOOT-INF/lib/"); + } + + protected boolean isIncludedOnClassPathAndNotIndexed(Entry entry) { + if (!isIncludedOnClassPath(entry)) { + return false; + } + return (this.classPathIndex == null) || !this.classPathIndex.containsEntry(entry.name()); + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/PropertiesLauncher.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/PropertiesLauncher.java index efa9d80c0b3f..c3ad4eb31e05 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/PropertiesLauncher.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/PropertiesLauncher.java @@ -70,6 +70,7 @@ * @author Janne Valkealahti * @author Andy Wilkinson * @author Phillip Webb + * @author Scott Frederick * @since 3.2.0 */ public class PropertiesLauncher extends Launcher { @@ -148,6 +149,7 @@ public PropertiesLauncher() throws Exception { this.homeDirectory = getHomeDirectory(); initializeProperties(); this.paths = getPaths(); + this.classPathIndex = getClassPathIndex(this.archive); } protected File getHomeDirectory() throws Exception { @@ -330,6 +332,10 @@ private String cleanupPath(String path) { @Override protected ClassLoader createClassLoader(Collection urls) throws Exception { String loaderClassName = getProperty("loader.classLoader"); + if (this.classPathIndex != null) { + urls = new ArrayList<>(urls); + urls.addAll(this.classPathIndex.getUrls()); + } if (loaderClassName == null) { return super.createClassLoader(urls); } @@ -537,9 +543,9 @@ private Set getClassPathUrlsForNested(String path) throws Exception { } } - private Set getClassPathUrlsForRoot() throws IOException { + private Set getClassPathUrlsForRoot() throws Exception { debug.log("Adding classpath entries from root archive %s", this.archive); - return this.archive.getClassPathUrls(JarLauncher::isLibraryFileOrClassesDirectory); + return this.archive.getClassPathUrls(this::isIncludedOnClassPathAndNotIndexed, Archive.ALL_ENTRIES); } private Predicate includeByPrefix(String prefix) { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/WarLauncher.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/WarLauncher.java index 38318ba222c8..d32aa85a7a4e 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/WarLauncher.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/launch/WarLauncher.java @@ -35,17 +35,13 @@ protected WarLauncher(Archive archive) throws Exception { super(archive); } - @Override - public boolean isIncludedOnClassPath(Archive.Entry entry) { - return isLibraryFileOrClassesDirectory(entry); - } - @Override protected String getEntryPathPrefix() { return "WEB-INF/"; } - static boolean isLibraryFileOrClassesDirectory(Archive.Entry entry) { + @Override + protected boolean isLibraryFileOrClassesDirectory(Archive.Entry entry) { String name = entry.name(); if (entry.isDirectory()) { return name.equals("WEB-INF/classes/"); diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/AbstractExecutableArchiveLauncherTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/AbstractLauncherTests.java similarity index 93% rename from spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/AbstractExecutableArchiveLauncherTests.java rename to spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/AbstractLauncherTests.java index efdc7012d2ea..2fd0f9b80ce6 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/AbstractExecutableArchiveLauncherTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/AbstractLauncherTests.java @@ -24,6 +24,7 @@ import java.io.Writer; import java.net.MalformedURLException; import java.net.URL; +import java.net.URLClassLoader; import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.Enumeration; @@ -40,13 +41,13 @@ import org.springframework.util.FileCopyUtils; /** - * Base class for testing {@link ExecutableArchiveLauncher} implementations. + * Base class for testing {@link Launcher} implementations. * * @author Andy Wilkinson * @author Madhura Bhave * @author Scott Frederick */ -abstract class AbstractExecutableArchiveLauncherTests { +abstract class AbstractLauncherTests { @TempDir File tempDir; @@ -133,4 +134,8 @@ protected final URL toUrl(File file) { } } + protected URLClassLoader createClassLoader(Launcher launcher) throws Exception { + return (URLClassLoader) launcher.createClassLoader(launcher.getClassPathUrls()); + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/JarLauncherTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/JarLauncherTests.java index 7e231949bea4..dd2d300f2c3e 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/JarLauncherTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/JarLauncherTests.java @@ -49,7 +49,7 @@ * @author Phillip Webb */ @AssertFileChannelDataBlocksClosed -class JarLauncherTests extends AbstractExecutableArchiveLauncherTests { +class JarLauncherTests extends AbstractLauncherTests { @Test void explodedJarHasOnlyBootInfClassesAndContentsOfBootInfLibOnClasspath() throws Exception { @@ -115,10 +115,6 @@ void explodedJarDefinedPackagesIncludeManifestAttributes() { })); } - private URLClassLoader createClassLoader(JarLauncher launcher) throws Exception { - return (URLClassLoader) launcher.createClassLoader(launcher.getClassPathUrls()); - } - private URL[] getExpectedFileUrls(File explodedRoot) { return getExpectedFiles(explodedRoot).stream().map(this::toUrl).toArray(URL[]::new); } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/PropertiesLauncherTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/PropertiesLauncherTests.java index 9bac00e9e74c..1a6dd9395166 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/PropertiesLauncherTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/PropertiesLauncherTests.java @@ -22,7 +22,9 @@ import java.net.URL; import java.net.URLClassLoader; import java.time.Duration; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -38,7 +40,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; -import org.junit.jupiter.api.io.TempDir; import org.springframework.boot.loader.net.protocol.jar.JarUrl; import org.springframework.boot.loader.testsupport.TestJar; @@ -58,13 +59,11 @@ * * @author Dave Syer * @author Andy Wilkinson + * @author Scott Frederick */ @ExtendWith(OutputCaptureExtension.class) @AssertFileChannelDataBlocksClosed -class PropertiesLauncherTests { - - @TempDir - File tempDir; +class PropertiesLauncherTests extends AbstractLauncherTests { private PropertiesLauncher launcher; @@ -388,13 +387,66 @@ void classPathWithoutLoaderPathDefaultsToJarLauncherIncludes() throws Exception this.launcher = new PropertiesLauncher(archive); this.launcher.launch(new String[0]); waitFor("Hello World"); + } + + @Test + void explodedJarShouldPreserveClasspathOrderWhenIndexPresent() throws Exception { + File explodedRoot = explode(createJarArchive("archive.jar", "BOOT-INF", true, Collections.emptyList())); + PropertiesLauncher launcher = new PropertiesLauncher(new ExplodedArchive(explodedRoot)); + URLClassLoader classLoader = createClassLoader(launcher); + assertThat(classLoader.getURLs()).containsExactly(getExpectedFileUrls(explodedRoot)); + } + + @Test + void customClassLoaderAndExplodedJarAndShouldPreserveClasspathOrderWhenIndexPresent() throws Exception { + System.setProperty("loader.classLoader", URLClassLoader.class.getName()); + File explodedRoot = explode(createJarArchive("archive.jar", "BOOT-INF", true, Collections.emptyList())); + PropertiesLauncher launcher = new PropertiesLauncher(new ExplodedArchive(explodedRoot)); + URLClassLoader classLoader = createClassLoader(launcher); + assertThat(classLoader.getParent()).isInstanceOf(URLClassLoader.class); + assertThat(((URLClassLoader) classLoader.getParent()).getURLs()) + .containsExactly(getExpectedFileUrls(explodedRoot)); + } + @Test + void jarFilesPresentInBootInfLibsAndNotInClasspathIndexShouldBeAddedAfterBootInfClasses() throws Exception { + ArrayList extraLibs = new ArrayList<>(Arrays.asList("extra-1.jar", "extra-2.jar")); + File explodedRoot = explode(createJarArchive("archive.jar", "BOOT-INF", true, extraLibs)); + PropertiesLauncher launcher = new PropertiesLauncher(new ExplodedArchive(explodedRoot)); + URLClassLoader classLoader = createClassLoader(launcher); + List expectedFiles = getExpectedFilesWithExtraLibs(explodedRoot); + URL[] expectedFileUrls = expectedFiles.stream().map(this::toUrl).toArray(URL[]::new); + assertThat(classLoader.getURLs()).containsExactly(expectedFileUrls); } private void waitFor(String value) { Awaitility.waitAtMost(Duration.ofSeconds(5)).until(this.output::toString, containsString(value)); } + private URL[] getExpectedFileUrls(File explodedRoot) { + return getExpectedFiles(explodedRoot).stream().map(this::toUrl).toArray(URL[]::new); + } + + private List getExpectedFiles(File parent) { + List expected = new ArrayList<>(); + expected.add(new File(parent, "BOOT-INF/classes")); + expected.add(new File(parent, "BOOT-INF/lib/foo.jar")); + expected.add(new File(parent, "BOOT-INF/lib/bar.jar")); + expected.add(new File(parent, "BOOT-INF/lib/baz.jar")); + return expected; + } + + private List getExpectedFilesWithExtraLibs(File parent) { + List expected = new ArrayList<>(); + expected.add(new File(parent, "BOOT-INF/classes")); + expected.add(new File(parent, "BOOT-INF/lib/extra-1.jar")); + expected.add(new File(parent, "BOOT-INF/lib/extra-2.jar")); + expected.add(new File(parent, "BOOT-INF/lib/foo.jar")); + expected.add(new File(parent, "BOOT-INF/lib/bar.jar")); + expected.add(new File(parent, "BOOT-INF/lib/baz.jar")); + return expected; + } + private Condition endingWith(String value) { return new Condition<>() { diff --git a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/WarLauncherTests.java b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/WarLauncherTests.java index cea89eabe7c7..609fa6ea417f 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/WarLauncherTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/launch/WarLauncherTests.java @@ -40,7 +40,7 @@ * @author Phillip Webb */ @AssertFileChannelDataBlocksClosed -class WarLauncherTests extends AbstractExecutableArchiveLauncherTests { +class WarLauncherTests extends AbstractLauncherTests { @Test void explodedWarHasOnlyWebInfClassesAndContentsOfWebInfLibOnClasspath() throws Exception { @@ -86,10 +86,6 @@ void warFilesPresentInWebInfLibsAndNotInClasspathIndexShouldBeAddedAfterWebInfCl assertThat(urls).containsExactly(expectedFileUrls); } - private URLClassLoader createClassLoader(Launcher launcher) throws Exception { - return (URLClassLoader) launcher.createClassLoader(launcher.getClassPathUrls()); - } - private URL[] getExpectedFileUrls(File explodedRoot) { return getExpectedFiles(explodedRoot).stream().map(this::toUrl).toArray(URL[]::new); }