SONAR-3697 Classloader of batch bootstrapper is not fully isolated

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 @@
       <scope>test</scope>
     </dependency>
     <dependency>
-      <groupId>org.hamcrest</groupId>
-      <artifactId>hamcrest-all</artifactId>
+      <groupId>org.easytesting</groupId>
+      <artifactId>fest-assert</artifactId>
       <scope>test</scope>
     </dependency>
   </dependencies>
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("$");
   }
 
 }