Merge pull request #105 from nbaars/master

Fixed classloading issues with Goathills lessons

webgoat-container/.gitignore

@@ -4,4 +4,5 @@ target/
 /src/main/webapp/plugin_lessons/*.jar
 /src/main/webapp/plugin_extracted/*
 dependency-reduced-pom.xml
-src/main/webapp/users/guest.org.owasp.webgoat.lessons.BackDoors.props
+src/main/webapp/users/guest.org.owasp.webgoat.lessons.BackDoors.props
+/src/main/webapp/WEB-INF/lib/*.jar

webgoat-container/src/main/java/org/owasp/webgoat/plugins/Plugin.java

@@ -2,12 +2,12 @@ package org.owasp.webgoat.plugins;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.Lists;
+import org.apache.catalina.loader.WebappClassLoader;
 import org.owasp.webgoat.lessons.AbstractLesson;
 import org.springframework.util.StringUtils;
 
 import java.io.File;
 import java.io.IOException;
-import java.net.URLClassLoader;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -43,10 +43,10 @@ public class Plugin {
     private void findLesson(String name) {
         String realClassName = StringUtils.trimLeadingCharacter(name, '/').replaceAll("/", ".").replaceAll(".class", "");
         //TODO should be passed in (refactor)
-        URLClassLoader cl = (URLClassLoader) Thread.currentThread().getContextClassLoader();
+        WebappClassLoader cl = (WebappClassLoader) Thread.currentThread().getContextClassLoader();
 
         try {
-            Class clazz = cl.loadClass(realClassName);
+            Class clazz = cl.loadClass(realClassName, true);
 
             if (AbstractLesson.class.isAssignableFrom(clazz)) {
                 this.lesson = clazz;

webgoat-container/src/main/java/org/owasp/webgoat/plugins/PluginBackgroundLoader.java

@@ -1,34 +0,0 @@
-package org.owasp.webgoat.plugins;
-
-import javax.servlet.ServletContextEvent;
-import javax.servlet.ServletContextListener;
-import javax.servlet.annotation.WebListener;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-
-@WebListener
-/**
- * <p>PluginBackgroundLoader class.</p>
- *
- * @version $Id: $Id
- */
-public class PluginBackgroundLoader implements ServletContextListener {
-
-    private ScheduledExecutorService scheduler;
-
-    /** {@inheritDoc} */
-    @Override
-    public void contextInitialized(ServletContextEvent event) {
-        String pluginPath = event.getServletContext().getRealPath("plugin_lessons");
-        String targetPath = event.getServletContext().getRealPath("plugin_extracted");
-
-        scheduler = Executors.newSingleThreadScheduledExecutor();
-        //scheduler.scheduleAtFixedRate(new PluginsLoader(Paths.get(pluginPath), Paths.get(targetPath)), 10, 5, TimeUnit.MINUTES);
-    }
-
-    /** {@inheritDoc} */
-    @Override
-    public void contextDestroyed(ServletContextEvent event) {
-        scheduler.shutdownNow();
-    }
-}

webgoat-container/src/main/java/org/owasp/webgoat/plugins/PluginsLoader.java

@@ -1,9 +1,8 @@
 package org.owasp.webgoat.plugins;
 
 import com.google.common.collect.Lists;
+import org.apache.catalina.loader.WebappClassLoader;
 import org.apache.commons.io.FileUtils;
-import org.owasp.webgoat.plugins.classloader.PluginClassLoaderFactory;
-import org.owasp.webgoat.plugins.classloader.PluginClassLoaderRepository;
 import org.owasp.webgoat.util.LabelProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -11,10 +10,10 @@ import org.springframework.util.ResourceUtils;
 
 import java.io.IOException;
 import java.net.URL;
-import java.net.URLClassLoader;
 import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.nio.file.SimpleFileVisitor;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.util.List;
@@ -30,12 +29,12 @@ import java.util.concurrent.Executors;
  *
  * @version $Id: $Id
  */
-public class PluginsLoader implements Runnable {
+public class PluginsLoader {
 
     private static final String WEBGOAT_PLUGIN_EXTENSION = "jar";
+    private static boolean alreadyLoaded = false;
     private final Logger logger = LoggerFactory.getLogger(this.getClass());
     private final Path pluginSource;
-    private final PluginClassLoaderRepository repository;
     private Path pluginTarget;
 
     /**
@@ -44,26 +43,48 @@ public class PluginsLoader implements Runnable {
      * @param pluginSource a {@link java.nio.file.Path} object.
      * @param pluginTarget a {@link java.nio.file.Path} object.
      */
-    public PluginsLoader(PluginClassLoaderRepository repository, Path pluginSource, Path pluginTarget) {
+    public PluginsLoader(Path pluginSource, Path pluginTarget) {
         this.pluginSource = Objects.requireNonNull(pluginSource, "plugin source cannot be null");
         this.pluginTarget = Objects.requireNonNull(pluginTarget, "plugin target cannot be null");
-        this.repository = Objects.requireNonNull(repository, "repository cannot be null");
+    }
+
+    /**
+     * Copy jars to the lib directory
+     */
+    public void copyJars() {
+        try {
+            if (!alreadyLoaded) {
+                WebappClassLoader cl = (WebappClassLoader) Thread.currentThread().getContextClassLoader();
+                cl.setAntiJARLocking(true);
+
+                List<URL> jars = listJars();
+
+                Path webInfLib = pluginTarget.getParent().resolve(cl.getJarPath().replaceFirst("\\/", ""));
+                for (URL jar : jars) {
+                    Path sourceJarFile = Paths.get(jar.toURI());
+                    FileUtils.copyFileToDirectory(sourceJarFile.toFile(), webInfLib.toFile());
+                }
+                alreadyLoaded = true;
+            }
+        } catch (Exception e) {
+            logger.error("Copying plugins failed", e);
+        }
     }
 
     /**
      * <p>loadPlugins.</p>
      *
-     * @param reload a boolean.
      * @return a {@link java.util.List} object.
      */
-    public List<Plugin> loadPlugins(final boolean reload) {
+    public List<Plugin> loadPlugins() {
+        copyJars();
         List<Plugin> plugins = Lists.newArrayList();
 
         try {
             PluginFileUtils.createDirsIfNotExists(pluginTarget);
             cleanupExtractedPluginsDirectory();
             List<URL> jars = listJars();
-            initClassLoader(jars);
+
             plugins = processPlugins(jars);
         } catch (Exception e) {
             logger.error("Loading plugins failed", e);
@@ -71,12 +92,6 @@ public class PluginsLoader implements Runnable {
         return plugins;
     }
 
-    private void initClassLoader(List<URL> jars) {
-        URLClassLoader classLoader = PluginClassLoaderFactory.createClassLoader(jars);
-        this.repository.replaceClassLoader(classLoader);
-        Thread.currentThread().setContextClassLoader(classLoader);
-    }
-
     private void cleanupExtractedPluginsDirectory() {
         Path i18nDirectory = pluginTarget.resolve("plugin/i18n/");
         FileUtils.deleteQuietly(i18nDirectory.toFile());
@@ -131,10 +146,4 @@ public class PluginsLoader implements Runnable {
         }
         return extractorCallables;
     }
-
-    /** {@inheritDoc} */
-    @Override
-    public void run() {
-        loadPlugins(true);
-    }
 }

webgoat-container/src/main/java/org/owasp/webgoat/plugins/classloader/PluginClassLoaderFactory.java

@@ -1,31 +0,0 @@
-package org.owasp.webgoat.plugins.classloader;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.List;
-
-/**
- * Create a classloader for the plugins
- */
-public class PluginClassLoaderFactory {
-
-    private static final Logger logger = LoggerFactory.getLogger(PluginClassLoaderFactory.class);
-
-    public static URLClassLoader createClassLoader(List<URL> urls) {
-        return new URLClassLoader(urls.toArray(new URL[urls.size()]), determineParentClassLoader());
-    }
-
-    private static ClassLoader determineParentClassLoader() {
-        ClassLoader parent = Thread.currentThread().getContextClassLoader();
-        try {
-            parent = Thread.currentThread().getContextClassLoader().getParent()
-                    .loadClass("org.apache.jasper.runtime.JspContextWrapper").getClassLoader();
-        } catch (ClassNotFoundException e) {
-            logger.info("Tomcat JspContextWrapper not found, probably not running on Tomcat...");
-        }
-        return parent;
-    }
-}

webgoat-container/src/main/java/org/owasp/webgoat/plugins/classloader/PluginClassLoaderRepository.java

@@ -1,35 +0,0 @@
-package org.owasp.webgoat.plugins.classloader;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import java.io.IOException;
-import java.net.URLClassLoader;
-
-/**
- * Holds the classloaders for the plugins. For now all the plugins are loaded by the same
- * classloader. This class can be extended to contain a classloader per plugin.
- */
-public class PluginClassLoaderRepository {
-
-    private static final Logger logger = LoggerFactory.getLogger(PluginClassLoaderRepository.class);
-    private URLClassLoader currentPluginLoader;
-
-    /**
-     * @return the plugin classloader
-     */
-    public URLClassLoader get() {
-        return currentPluginLoader;
-    }
-
-    public void replaceClassLoader(URLClassLoader classLoader) {
-        if (this.currentPluginLoader != null) {
-            try {
-                this.currentPluginLoader.close();
-            } catch (IOException e) {
-                logger.warn("Unable to close the current classloader", e);
-            }
-        }
-        this.currentPluginLoader = classLoader;
-    }
-}

webgoat-container/src/main/java/org/owasp/webgoat/service/PluginReloadService.java

@@ -30,6 +30,7 @@
  */
 package org.owasp.webgoat.service;
 
+import org.owasp.webgoat.plugins.PluginsLoader;
 import org.owasp.webgoat.session.WebSession;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,6 +41,7 @@ import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.ResponseBody;
 
 import javax.servlet.http.HttpSession;
+import java.nio.file.Paths;
 
 /**
  * <p>PluginReloadService class.</p>
@@ -61,6 +63,11 @@ public class PluginReloadService extends BaseService {
     public @ResponseBody
     ResponseEntity<String> reloadPlugins(HttpSession session) {
         WebSession webSession = (WebSession) session.getAttribute(WebSession.SESSION);
+        logger.debug("Loading plugins into cache");
+        String pluginPath = session.getServletContext().getRealPath("plugin_lessons");
+        String targetPath = session.getServletContext().getRealPath("plugin_extracted");
+        new PluginsLoader(Paths.get(pluginPath), Paths.get(targetPath)).copyJars();
+
         webSession.getCourse().loadLessonFromPlugin(session.getServletContext());
         return new ResponseEntity("Plugins reload refresh the WebGoat page!",HttpStatus.OK);
     }

webgoat-container/src/main/java/org/owasp/webgoat/session/Course.java

@@ -5,7 +5,6 @@ import org.owasp.webgoat.lessons.AbstractLesson;
 import org.owasp.webgoat.lessons.Category;
 import org.owasp.webgoat.plugins.Plugin;
 import org.owasp.webgoat.plugins.PluginsLoader;
-import org.owasp.webgoat.plugins.classloader.PluginClassLoaderRepository;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -66,7 +65,6 @@ public class Course {
 
     final Logger logger = LoggerFactory.getLogger(Course.class);
 
-    private final PluginClassLoaderRepository repository = new PluginClassLoaderRepository();
     private final List<AbstractLesson> lessons = new LinkedList<AbstractLesson>();
 
     private final static String PROPERTIES_FILENAME = HammerHead.propertiesPath;
@@ -337,7 +335,7 @@ public class Course {
             return;
         }
         lessons.clear();
-        List<Plugin> plugins = new PluginsLoader(repository, Paths.get(pluginPath), Paths.get(targetPath)).loadPlugins(true);
+        List<Plugin> plugins = new PluginsLoader(Paths.get(pluginPath), Paths.get(targetPath)).loadPlugins();
         for (Plugin plugin : plugins) {
             try {
                 AbstractLesson lesson = plugin.getLesson().get();