From 681a866cceae12d69b46a7cd047eed14a898c579 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Thu, 14 Jan 2016 17:05:08 +0000 Subject: [PATCH] Exclude Java agents from class loader created by PropertiesLauncher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PropertiesLauncher creates a ClassLoader that is used by the Launcher to load an application’s classes. During the creation of this ClassLoader URLs from its ClassLoader. This resulted resulting in Java agents that are added to the system class loader via the -javaagent launch option being available on both the system class loader and the created class loader. Java agents are intended to always be loaded by the system class loader. Making them available on another class loader breaks this model. This is the same problem that was in ExecutableArchiveLauncher and that was fixed in ee08667e (see gh-863). This commit updates PropertiesLauncher so that it skips the URLs of any Java agents (found by examining the JVM’s input arguments) when copying URLs over to the new ClassLoader, thereby ensuring that Java agents are only ever loaded by the system class loader. Closes gh-4911 --- .../boot/loader/PropertiesLauncher.java | 47 +++++++++++++------ .../boot/loader/PropertiesLauncherTests.java | 31 +++++++++++- 2 files changed, 63 insertions(+), 15 deletions(-) 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 93096c73903..248a058000d 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2015 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -72,6 +72,7 @@ import org.springframework.boot.loader.util.SystemPropertyUtils; * * @author Dave Syer * @author Janne Valkealahti + * @author Andy Wilkinson */ public class PropertiesLauncher extends Launcher { @@ -132,6 +133,8 @@ public class PropertiesLauncher extends Launcher { private final File home; + private final JavaAgentDetector javaAgentDetector; + private List paths = new ArrayList(DEFAULT_PATHS); private final Properties properties = new Properties(); @@ -139,11 +142,16 @@ public class PropertiesLauncher extends Launcher { private Archive parent; public PropertiesLauncher() { + this(new InputArgumentsJavaAgentDetector()); + } + + PropertiesLauncher(JavaAgentDetector javaAgentDetector) { if (!isDebug()) { this.logger.setLevel(Level.SEVERE); } try { this.home = getHomeDirectory(); + this.javaAgentDetector = javaAgentDetector; initializeProperties(this.home); initializePaths(); this.parent = createArchive(); @@ -517,21 +525,12 @@ public class PropertiesLauncher extends Launcher { ClassLoader parentClassLoader = getClass().getClassLoader(); List urls = new ArrayList(); for (URL url : getURLs(parentClassLoader)) { - if (url.toString().endsWith(".jar") || url.toString().endsWith(".zip")) { - urls.add(new JarFileArchive(new File(url.toURI()))); - } - else if (url.toString().endsWith("/*")) { - String name = url.getFile(); - File dir = new File(name.substring(0, name.length() - 1)); - if (dir.exists()) { - urls.add(new ExplodedArchive( - new File(name.substring(0, name.length() - 1)), false)); + if (!this.javaAgentDetector.isJavaAgentJar(url)) { + Archive archive = createArchiveIfPossible(url); + if (archive != null) { + urls.add(archive); } } - else { - String filename = URLDecoder.decode(url.getFile(), "UTF-8"); - urls.add(new ExplodedArchive(new File(filename))); - } } // The parent archive might have a "lib/" directory, meaning we are running from // an executable JAR. We add nested entries from there with low priority (i.e. at @@ -545,6 +544,26 @@ public class PropertiesLauncher extends Launcher { } } + private Archive createArchiveIfPossible(URL url) + throws IOException, URISyntaxException { + if (url.toString().endsWith(".jar") || url.toString().endsWith(".zip")) { + return new JarFileArchive(new File(url.toURI())); + } + else if (url.toString().endsWith("/*")) { + String name = url.getFile(); + File dir = new File(name.substring(0, name.length() - 1)); + if (dir.exists()) { + return new ExplodedArchive(new File(name.substring(0, name.length() - 1)), + false); + } + } + else { + String filename = URLDecoder.decode(url.getFile(), "UTF-8"); + return new ExplodedArchive(new File(filename)); + } + return null; + } + private void addNestedArchivesFromParent(List urls) { int index = findArchive(urls, this.parent); if (index >= 0) { 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 64367294e84..356f71efcea 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 @@ -1,5 +1,5 @@ /* - * Copyright 2012-2013 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,32 +22,45 @@ import java.net.URL; import java.net.URLClassLoader; import java.util.Arrays; import java.util.Collections; +import java.util.List; import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.springframework.boot.loader.archive.Archive; import org.springframework.test.util.ReflectionTestUtils; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.verify; /** * Tests for {@link PropertiesLauncher}. * * @author Dave Syer + * @author Andy Wilkinson */ public class PropertiesLauncherTests { + @Mock + private JavaAgentDetector javaAgentDetector; + @Rule public OutputCapture output = new OutputCapture(); @Before public void setup() throws IOException { + MockitoAnnotations.initMocks(this); System.setProperty("loader.home", new File("src/test/resources").getAbsolutePath()); } @@ -175,6 +188,22 @@ public class PropertiesLauncherTests { assertEquals("[foo, bar]", Arrays.asList(launcher.getArgs("bar")).toString()); } + @Test + public void testJavaAgentJarsAreExcludedFromClasspath() throws Exception { + List allArchives = new PropertiesLauncher().getClassPathArchives(); + URL[] parentUrls = ((URLClassLoader) getClass().getClassLoader()).getURLs(); + for (URL url : parentUrls) { + given(this.javaAgentDetector.isJavaAgentJar(url)).willReturn(true); + } + List nonAgentArchives = new PropertiesLauncher(this.javaAgentDetector) + .getClassPathArchives(); + assertThat(nonAgentArchives.size(), + is(equalTo(allArchives.size() - parentUrls.length))); + for (URL url : parentUrls) { + verify(this.javaAgentDetector).isJavaAgentJar(url); + } + } + private void waitFor(String value) throws Exception { int count = 0; boolean timeout = false;