From bfa816f2a30dbc188ca563da8f28c22417d907e5 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Tue, 14 Jul 2015 10:19:47 +0100 Subject: [PATCH] Maintain classpath order in PropertiesLauncher I think this is safe, judging by the integration tests, but I'm not putting it in 1.2.x until we've had some feedback on it. The integration tests actually had a bug that was masking this problem because they were merging Properties from the whole classpath instead of picking the first available resource (which is generally what we do in Spring Boot applications for application.properties for instance). Fixes gh-3048 --- .../main/asciidoc/appendix-executable-jar-format.adoc | 3 ++- .../launcher/it/props/SpringConfiguration.java | 7 +++++-- .../src/it/executable-props/verify.groovy | 1 - .../org/springframework/boot/loader/Launcher.java | 2 -- .../boot/loader/PropertiesLauncher.java | 2 -- .../boot/loader/PropertiesLauncherTests.java | 11 +++++++++++ 6 files changed, 18 insertions(+), 8 deletions(-) diff --git a/spring-boot-docs/src/main/asciidoc/appendix-executable-jar-format.adoc b/spring-boot-docs/src/main/asciidoc/appendix-executable-jar-format.adoc index aed8cbeae37..9dd787df476 100644 --- a/spring-boot-docs/src/main/asciidoc/appendix-executable-jar-format.adoc +++ b/spring-boot-docs/src/main/asciidoc/appendix-executable-jar-format.adoc @@ -202,7 +202,7 @@ properties (System properties, environment variables, manifest entries or |Key |Purpose |`loader.path` -|Comma-separated Classpath, e.g. `lib:${HOME}/app/lib`. +|Comma-separated Classpath, e.g. `lib,${HOME}/app/lib`. Earlier entries take precedence, just like a regular `-classpath` on the `javac` command line. |`loader.home` |Location of additional properties file, e.g. `file:///opt/app` @@ -236,6 +236,7 @@ Environment variables can be capitalized with underscore separators instead of p the default) as long as `loader.config.location` is not specified. * `loader.path` can contain directories (scanned recursively for jar and zip files), archive paths, or wildcard patterns (for the default JVM behavior). +* `loader.path` (if empty) defaults to `lib` (meaning a local directory or a nested one if running from an archive). Because of this `PropertiesLauncher` behaves the same as `JarLauncher` when no additional configuration is provided. * Placeholder replacement is done from System and environment variables plus the properties file itself on all values before use. diff --git a/spring-boot-tools/spring-boot-loader/src/it/executable-props/src/main/java/org/springframework/launcher/it/props/SpringConfiguration.java b/spring-boot-tools/spring-boot-loader/src/it/executable-props/src/main/java/org/springframework/launcher/it/props/SpringConfiguration.java index 5396340644a..5f41c182507 100644 --- a/spring-boot-tools/spring-boot-loader/src/it/executable-props/src/main/java/org/springframework/launcher/it/props/SpringConfiguration.java +++ b/spring-boot-tools/spring-boot-loader/src/it/executable-props/src/main/java/org/springframework/launcher/it/props/SpringConfiguration.java @@ -17,12 +17,13 @@ package org.springframework.boot.load.it.props; import java.io.IOException; +import java.util.Properties; import javax.annotation.PostConstruct; import org.springframework.context.annotation.ComponentScan; import org.springframework.context.annotation.Configuration; -import org.springframework.core.io.support.PropertiesLoaderUtils; +import org.springframework.core.io.ClassPathResource; /** * Spring configuration. @@ -37,7 +38,9 @@ public class SpringConfiguration { @PostConstruct public void init() throws IOException { - String value = PropertiesLoaderUtils.loadAllProperties("application.properties").getProperty("message"); + Properties props = new Properties(); + props.load(new ClassPathResource("application.properties").getInputStream()); + String value = props.getProperty("message"); if (value!=null) { this.message = value; } diff --git a/spring-boot-tools/spring-boot-loader/src/it/executable-props/verify.groovy b/spring-boot-tools/spring-boot-loader/src/it/executable-props/verify.groovy index 80892f628e8..0a0c034267a 100644 --- a/spring-boot-tools/spring-boot-loader/src/it/executable-props/verify.groovy +++ b/spring-boot-tools/spring-boot-loader/src/it/executable-props/verify.groovy @@ -26,7 +26,6 @@ assert out.contains('Hello Embedded Foo!'), 'With loader.path=.,lib should use the application.properties from the local filesystem\n' + out new File("${basedir}/target/application.properties").withWriter { it -> it << "message: Spam" } - out = exec("java -Dloader.path=target,.,lib -jar ${jarfile}") assert out.contains('Hello Embedded Spam!'), 'With loader.path=target,.,lib should use the application.properties from the target directory\n' + out \ No newline at end of file diff --git a/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/Launcher.java b/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/Launcher.java index 89ebc50ab68..81111534772 100644 --- a/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/Launcher.java +++ b/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/Launcher.java @@ -75,8 +75,6 @@ public abstract class Launcher { protected ClassLoader createClassLoader(List archives) throws Exception { List urls = new ArrayList(archives.size()); for (Archive archive : archives) { - // Add the current archive at end (it will be reversed and end up taking - // precedence) urls.add(archive.getUrl()); } return createClassLoader(urls.toArray(new URL[urls.size()])); diff --git a/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/PropertiesLauncher.java b/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/PropertiesLauncher.java index ca20a6b100a..6de5327151a 100644 --- a/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/PropertiesLauncher.java +++ b/spring-boot-tools/spring-boot-loader/src/main/java/org/springframework/boot/loader/PropertiesLauncher.java @@ -448,8 +448,6 @@ public class PropertiesLauncher extends Launcher { } } addParentClassLoaderEntries(lib); - // Entries are reversed when added to the actual classpath - Collections.reverse(lib); return lib; } diff --git a/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/PropertiesLauncherTests.java b/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/PropertiesLauncherTests.java index 72b79e24abe..a7bde9b17f7 100644 --- a/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/PropertiesLauncherTests.java +++ b/spring-boot-tools/spring-boot-loader/src/test/java/org/springframework/boot/loader/PropertiesLauncherTests.java @@ -134,6 +134,17 @@ public class PropertiesLauncherTests { waitFor("Hello World"); } + @Test + public void testUserSpecifiedClassPathOrder() throws Exception { + System.setProperty("loader.path", "more-jars/app.jar,jars/app.jar"); + System.setProperty("loader.classLoader", URLClassLoader.class.getName()); + PropertiesLauncher launcher = new PropertiesLauncher(); + assertEquals("[more-jars/app.jar, jars/app.jar]", + ReflectionTestUtils.getField(launcher, "paths").toString()); + launcher.launch(new String[0]); + waitFor("Hello Other World"); + } + @Test public void testCustomClassLoaderCreation() throws Exception { System.setProperty("loader.classLoader", TestLoader.class.getName());