From 5c66652705937dbb24b6728180c63b1c951051d3 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Thu, 16 Aug 2012 17:47:49 +0200 Subject: [PATCH] SONAR-3697 Classloader of batch bootstrapper is not fully isolated --- sonar-batch-bootstrapper/pom.xml | 4 +-- .../bootstrapper/BootstrapClassLoader.java | 14 ++++++-- .../BootstrapClassLoaderTest.java | 32 ++++++++++++++++--- .../batch/bootstrapper/BootstrapperTest.java | 15 ++++----- .../bootstrapper/BootstrapperVersionTest.java | 10 +++--- 5 files changed, 52 insertions(+), 23 deletions(-) diff --git a/sonar-batch-bootstrapper/pom.xml b/sonar-batch-bootstrapper/pom.xml index b1e0366c655..136dd99f491 100644 --- a/sonar-batch-bootstrapper/pom.xml +++ b/sonar-batch-bootstrapper/pom.xml @@ -19,8 +19,8 @@ test - org.hamcrest - hamcrest-all + org.easytesting + fest-assert test diff --git a/sonar-batch-bootstrapper/src/main/java/org/sonar/batch/bootstrapper/BootstrapClassLoader.java b/sonar-batch-bootstrapper/src/main/java/org/sonar/batch/bootstrapper/BootstrapClassLoader.java index 3b302743ad3..53f0d413cbc 100644 --- a/sonar-batch-bootstrapper/src/main/java/org/sonar/batch/bootstrapper/BootstrapClassLoader.java +++ b/sonar-batch-bootstrapper/src/main/java/org/sonar/batch/bootstrapper/BootstrapClassLoader.java @@ -29,7 +29,7 @@ import java.util.Enumeration; */ public class BootstrapClassLoader extends URLClassLoader { - private String[] unmaskedPackages; + private final String[] unmaskedPackages; public BootstrapClassLoader(ClassLoader parent, String... unmaskedPackages) { super(new URL[0], parent); @@ -74,11 +74,19 @@ public class BootstrapClassLoader extends URLClassLoader { if (c == null) { try { // Load from parent - if ((getParent() != null) && canLoadFromParent(name)) { + if (getParent() != null && canLoadFromParent(name)) { c = getParent().loadClass(name); } else { // Load from system - c = getSystemClassLoader().loadClass(name); + + // I don't know for other vendors, but for Oracle JVM : + // - ClassLoader.getSystemClassLoader() is sun.misc.Launcher$AppClassLoader. It contains app classpath. + // - ClassLoader.getSystemClassLoader().getParent() is sun.misc.Launcher$ExtClassLoader. It contains core JVM + ClassLoader systemClassLoader = getSystemClassLoader(); + if (systemClassLoader.getParent() != null) { + systemClassLoader = systemClassLoader.getParent(); + } + c = systemClassLoader.loadClass(name); } } catch (ClassNotFoundException e) { // If still not found, then invoke findClass in order diff --git a/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapClassLoaderTest.java b/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapClassLoaderTest.java index a96d50c3a9b..05c4718b4a2 100644 --- a/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapClassLoaderTest.java +++ b/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapClassLoaderTest.java @@ -19,18 +19,42 @@ */ package org.sonar.batch.bootstrapper; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; +import static org.fest.assertions.Assertions.assertThat; public class BootstrapClassLoaderTest { + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Test public void shouldRestrictLoadingFromParent() throws Exception { BootstrapClassLoader classLoader = new BootstrapClassLoader(getClass().getClassLoader(), "org.sonar.ant"); - assertThat(classLoader.canLoadFromParent("org.sonar.ant.Launcher"), is(true)); - assertThat(classLoader.canLoadFromParent("org.objectweb.asm.ClassVisitor"), is(false)); + assertThat(classLoader.canLoadFromParent("org.sonar.ant.Launcher")).isTrue(); + assertThat(classLoader.canLoadFromParent("org.objectweb.asm.ClassVisitor")).isFalse(); + } + + @Test + public void use_isolated_system_classloader_when_parent_is_excluded() throws ClassNotFoundException { + thrown.expect(ClassNotFoundException.class); + thrown.expectMessage("org.junit.Test"); + ClassLoader parent = getClass().getClassLoader(); + BootstrapClassLoader classLoader = new BootstrapClassLoader(parent); + + // JUnit is available in the parent classloader (classpath used to execute this test) but not in the core JVM + assertThat(classLoader.loadClass("java.lang.String", false)).isNotNull(); + classLoader.loadClass("org.junit.Test", false); } + @Test + public void find_in_parent_when_matches_unmasked_packages() throws ClassNotFoundException { + ClassLoader parent = getClass().getClassLoader(); + BootstrapClassLoader classLoader = new BootstrapClassLoader(parent, "org.junit"); + + // JUnit is available in the parent classloader (classpath used to execute this test) but not in the core JVM + assertThat(classLoader.loadClass("org.junit.Test", false)).isNotNull(); + } } diff --git a/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperTest.java b/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperTest.java index 8f3cc41871e..d649b4e3faa 100644 --- a/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperTest.java +++ b/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperTest.java @@ -19,20 +19,19 @@ */ package org.sonar.batch.bootstrapper; -import static org.hamcrest.Matchers.*; -import static org.junit.Assert.assertThat; +import org.junit.Test; import java.io.File; import java.io.IOException; -import org.junit.Test; +import static org.fest.assertions.Assertions.assertThat; public class BootstrapperTest { @Test public void shouldRemoveLastUrlSlash() { Bootstrapper bootstrapper = new Bootstrapper("", "http://test/", new File("target")); - assertThat(bootstrapper.getServerUrl(), is("http://test")); + assertThat(bootstrapper.getServerUrl()).isEqualTo("http://test"); } @Test(expected = Exception.class) @@ -46,9 +45,9 @@ public class BootstrapperTest { Bootstrapper bootstrapper = new Bootstrapper("test/0.1", "http://unknown.foo", new File("target")); String userAgent = bootstrapper.getUserAgent(); - assertThat(userAgent.length(), greaterThan(0)); - assertThat(userAgent, startsWith("sonar-bootstrapper/")); - assertThat(userAgent, endsWith(" test/0.1")); + assertThat(userAgent.length()).isGreaterThan(0); + assertThat(userAgent).startsWith("sonar-bootstrapper/"); + assertThat(userAgent).endsWith(" test/0.1"); } @Test @@ -59,7 +58,7 @@ public class BootstrapperTest { return "2.6"; } }; - assertThat(bootstrapper.getServerVersion(), is("2.6")); + assertThat(bootstrapper.getServerVersion()).isEqualTo("2.6"); } } diff --git a/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperVersionTest.java b/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperVersionTest.java index 22115b05df8..f9a7651f2f7 100644 --- a/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperVersionTest.java +++ b/sonar-batch-bootstrapper/src/test/java/org/sonar/batch/bootstrapper/BootstrapperVersionTest.java @@ -19,19 +19,17 @@ */ package org.sonar.batch.bootstrapper; -import static org.hamcrest.Matchers.containsString; -import static org.hamcrest.Matchers.not; -import static org.junit.Assert.assertThat; - import org.junit.Test; +import static org.fest.assertions.Assertions.assertThat; + public class BootstrapperVersionTest { @Test public void shouldLoadVersion() { String version = BootstrapperVersion.getVersion(); - assertThat(version, containsString(".")); - assertThat(version, not(containsString("$"))); + assertThat(version).contains("."); + assertThat(version).doesNotContain("$"); } } -- 2.39.5