diff --git a/extensions/pom.xml b/extensions/pom.xml index 938e7cc62..513cf3b0f 100644 --- a/extensions/pom.xml +++ b/extensions/pom.xml @@ -64,12 +64,11 @@ copy-runtime-dependencies prepare-package - unpack-dependencies + copy-dependencies runtime ${project.build.directory}/classes - META-INF/*.SF,META-INF/*.DSA diff --git a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java index cd81572ec..2b3f155d0 100644 --- a/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java +++ b/guacamole/src/main/java/org/apache/guacamole/GuacamoleServletContextListener.java @@ -19,6 +19,7 @@ package org.apache.guacamole; +import com.google.common.collect.Lists; import org.apache.guacamole.tunnel.TunnelModule; import com.google.inject.Guice; import com.google.inject.Injector; @@ -117,6 +118,14 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener @Inject private List authProviders; + /** + * All temporary files that should be deleted upon application shutdown, in + * reverse order of desired deletion. This will typically simply be the + * order that each file was created. + */ + @Inject + private List temporaryFiles; + /** * Internal reference to the Guice injector that was lazily created when * getInjector() was first invoked. @@ -194,20 +203,49 @@ public class GuacamoleServletContextListener extends GuiceServletContextListener }); } + /** + * Deletes the given temporary file/directory, if possible. If the deletion + * operation fails, a warning is logged noting the failure. If the given + * file is a directory, it will only be deleted if empty. + * + * @param temp + * The temporary file to delete. + */ + private void deleteTemporaryFile(File temp) { + if (!temp.delete()) { + logger.warn("Temporary file/directory \"{}\" could not be " + + "deleted. The file may remain until the JVM exits, or " + + "may need to be manually deleted.", temp); + } + else + logger.debug("Deleted temporary file/directory \"{}\".", temp); + } + @Override public void contextDestroyed(ServletContextEvent servletContextEvent) { + try { + + // Clean up reference to Guice injector + servletContextEvent.getServletContext().removeAttribute(GUICE_INJECTOR); - // Clean up reference to Guice injector - servletContextEvent.getServletContext().removeAttribute(GUICE_INJECTOR); + // Shutdown TokenSessionMap + if (sessionMap != null) + sessionMap.shutdown(); - // Shutdown TokenSessionMap - if (sessionMap != null) - sessionMap.shutdown(); + // Unload all extensions + if (authProviders != null) { + for (AuthenticationProvider authProvider : authProviders) + authProvider.shutdown(); + } + + } + finally { + + // Regardless of what may succeed/fail here, always attempt to + // clean up ALL temporary files + if (temporaryFiles != null) + Lists.reverse(temporaryFiles).stream().forEachOrdered(this::deleteTemporaryFile); - // Unload all extensions - if (authProviders != null) { - for (AuthenticationProvider authProvider : authProviders) - authProvider.shutdown(); } // Continue any Guice-specific cleanup diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java b/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java index 5c74c897d..f6cf80df2 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/Extension.java @@ -27,6 +27,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; +import java.util.List; import java.util.Map; import java.util.zip.ZipEntry; import java.util.zip.ZipException; @@ -355,12 +356,18 @@ public class Extension { * @param file * The file to load as an extension. * + * @param temporaryFiles + * A modifiable List that should be populated with all temporary files + * created for this extension. These files should be deleted on + * application shutdown in reverse order. + * * @throws GuacamoleException * If the provided file is not a .jar file, does not contain the * guac-manifest.json, or if guac-manifest.json is invalid and cannot * be parsed. */ - public Extension(final ClassLoader parent, final File file) throws GuacamoleException { + public Extension(final ClassLoader parent, final File file, + final List temporaryFiles) throws GuacamoleException { // Associate extension abstraction with original file this.file = file; @@ -390,7 +397,7 @@ public class Extension { } // Create isolated classloader for this extension - classLoader = ExtensionClassLoader.getInstance(file, parent); + classLoader = ExtensionClassLoader.getInstance(file, temporaryFiles, parent); } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java index 137dc7da4..c32fbde3a 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionClassLoader.java @@ -20,14 +20,27 @@ package org.apache.guacamole.extension; import java.io.File; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; import java.net.MalformedURLException; import java.net.URL; import java.net.URLClassLoader; +import java.nio.file.Files; +import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedActionException; import java.security.PrivilegedExceptionAction; +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.List; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; import org.apache.guacamole.GuacamoleException; import org.apache.guacamole.GuacamoleServerException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * ClassLoader implementation which prioritizes the classes defined within a @@ -38,6 +51,23 @@ import org.apache.guacamole.GuacamoleServerException; */ public class ExtensionClassLoader extends URLClassLoader { + /** + * Logger for this class. + */ + private static final Logger logger = LoggerFactory.getLogger(ExtensionClassLoader.class); + + /** + * The prefix that should be given to the temporary directory containing + * all library .jar files that were bundled with the extension. + */ + private static final String EXTENSION_TEMP_DIR_PREFIX = "guac-extension-lib-"; + + /** + * The prefix that should be given to any files created for temporary + * storage of a library .jar file that was bundled with the extension. + */ + private static final String EXTENSION_TEMP_LIB_PREFIX = "bundled-"; + /** * The ClassLoader to use if class resolution through the extension .jar * fails. @@ -54,6 +84,11 @@ public class ExtensionClassLoader extends URLClassLoader { * @param extension * The extension .jar file from which classes should be loaded. * + * @param temporaryFiles + * A modifiable List that should be populated with all temporary files + * created for the given extension. These files should be deleted on + * application shutdown in reverse order. + * * @param parent * The ClassLoader to use if class resolution through the extension * .jar fails. @@ -67,7 +102,8 @@ public class ExtensionClassLoader extends URLClassLoader { * file cannot be read. */ public static ExtensionClassLoader getInstance(final File extension, - final ClassLoader parent) throws GuacamoleException { + final List temporaryFiles, final ClassLoader parent) + throws GuacamoleException { try { // Attempt to create classloader which loads classes from the given @@ -76,7 +112,7 @@ public class ExtensionClassLoader extends URLClassLoader { @Override public ExtensionClassLoader run() throws GuacamoleException { - return new ExtensionClassLoader(extension, parent); + return new ExtensionClassLoader(extension, temporaryFiles, parent); } }); @@ -89,27 +125,26 @@ public class ExtensionClassLoader extends URLClassLoader { } /** - * Returns a URL which points to the given extension .jar file. + * Returns the URL that refers to the given file. If the given file refers + * to a directory, an exception is thrown. * - * @param extension - * The extension .jar file to generate a URL for. + * @param file + * The file to determine the URL of. * * @return - * A URL which points to the given extension .jar. + * A URL that refers to the given file. * * @throws GuacamoleException - * If the given file is not actually a file, or the contents of the - * file cannot be read. + * If the given file refers to a directory. */ - private static URL getExtensionURL(File extension) - throws GuacamoleException { + private static URL getFileURL(File file) throws GuacamoleException { - // Validate extension file is indeed a file - if (!extension.isFile()) - throw new GuacamoleException(extension + " is not a file."); + // Validate extension-related file is indeed a file + if (!file.isFile()) + throw new GuacamoleServerException("\"" + file + "\" is not a file."); try { - return extension.toURI().toURL(); + return file.toURI().toURL(); } catch (MalformedURLException e) { throw new GuacamoleServerException(e); @@ -117,6 +152,152 @@ public class ExtensionClassLoader extends URLClassLoader { } + /** + * Copies all bytes of data from a file within a .jar to a destination + * file. + * + * @param jar + * The JarFile containing the file to be copied. + * + * @param source + * The JarEntry representing the file to be copied within the given + * JarFile. + * + * @param dest + * The destination file that the data should be copied to. + * + * @throws IOException + * If an error occurs reading from the source .jar or writing to the + * destination file. + */ + private static void copyEntryToFile(JarFile jar, JarEntry source, File dest) + throws IOException { + + int length; + byte[] buffer = new byte[8192]; + + try (InputStream input = jar.getInputStream(source)) { + try (OutputStream output = new FileOutputStream(dest)) { + + while ((length = input.read(buffer)) > 0) { + output.write(buffer, 0, length); + } + + } + } + + } + + /** + * Returns the URLs for the .jar files relevant to the given extension .jar + * file. Unless the extension bundles additional Java libraries, only the + * URL of the extension .jar will be returned. If additional Java libraries + * are bundled within the extension, URLs for those libraries will be + * included, as well. Temporary directories and/or files will be created as + * necessary to house bundled libraries. Only .jar files located directly + * within the root of the main extension .jar are considered. + * + * @param extension + * The extension .jar file to generate URLs for. + * + * @param temporaryFiles + * A modifiable List that should be populated with all temporary files + * created for the given extension. These files should be deleted on + * application shutdown in reverse order. + * + * @return + * An array of all URLs relevant to the given extension .jar. + * + * @throws GuacamoleException + * If the given file is not actually a file, the contents of the file + * cannot be read, or any necessary temporary files/directories cannot + * be created. + */ + private static URL[] getExtensionURLs(File extension, + List temporaryFiles) throws GuacamoleException { + + JarFile extensionJar; + try { + extensionJar = new JarFile(extension); + } + catch (IOException e) { + throw new GuacamoleServerException("Contents of extension \"" + + extension + "\" cannot be read.", e); + } + + // Include extension itself within classpath + List urls = new ArrayList<>(); + urls.add(getFileURL(extension)); + + Path extensionTempLibDir = null; + + // Iterate through all entries (files) within the extension .jar, + // adding any nested .jar files within the archive root to the + // classpath + Enumeration entries = extensionJar.entries(); + while (entries.hasMoreElements()) { + + JarEntry entry = entries.nextElement(); + String name = entry.getName(); + + // Consider only .jar files located in root of archive + if (entry.isDirectory() ||! name.endsWith(".jar") || name.indexOf('/') != -1) + continue; + + // Create temporary directory for housing this extension's + // bundled .jar files, if not already created + try { + if (extensionTempLibDir == null) { + extensionTempLibDir = Files.createTempDirectory(EXTENSION_TEMP_DIR_PREFIX); + temporaryFiles.add(extensionTempLibDir.toFile()); + extensionTempLibDir.toFile().deleteOnExit(); + } + } + catch (IOException e) { + throw new GuacamoleServerException("Temporary directory " + + "for libraries bundled with extension \"" + + extension + "\" could not be created.", e); + } + + // Create temporary file to hold the contents of the current + // bundled .jar + File tempLibrary; + try { + tempLibrary = Files.createTempFile(extensionTempLibDir, EXTENSION_TEMP_LIB_PREFIX, ".jar").toFile(); + temporaryFiles.add(tempLibrary); + tempLibrary.deleteOnExit(); + } + catch (IOException e) { + throw new GuacamoleServerException("Temporary file " + + "for library \"" + name + "\" bundled with " + + "extension \"" + extension + "\" could not be " + + "created.", e); + } + + // Copy contents of bundled .jar to temporary file + try { + copyEntryToFile(extensionJar, entry, tempLibrary); + } + catch (IOException e) { + throw new GuacamoleServerException("Contents of library " + + "\"" + name + "\" bundled with extension \"" + + extension + "\" could not be copied to a " + + "temporary file.", e); + } + + // Add temporary .jar file to classpath + urls.add(getFileURL(tempLibrary)); + + } + + if (extensionTempLibDir != null) + logger.debug("Libraries bundled within extension \"{}\" have been " + + "copied to temporary directory \"{}\".", extension, extensionTempLibDir); + + return urls.toArray(new URL[0]); + + } + /** * Creates a new ExtensionClassLoader configured to load classes from the * given extension .jar. If a necessary class cannot be found within the @@ -127,6 +308,11 @@ public class ExtensionClassLoader extends URLClassLoader { * @param extension * The extension .jar file from which classes should be loaded. * + * @param temporaryFiles + * A modifiable List that should be populated with all temporary files + * created for the given extension. These files should be deleted on + * application shutdown in reverse order. + * * @param parent * The ClassLoader to use if class resolution through the extension * .jar fails. @@ -135,9 +321,9 @@ public class ExtensionClassLoader extends URLClassLoader { * If the given file is not actually a file, or the contents of the * file cannot be read. */ - private ExtensionClassLoader(File extension, ClassLoader parent) - throws GuacamoleException { - super(new URL[]{ getExtensionURL(extension) }, null); + private ExtensionClassLoader(File extension, List temporaryFiles, + ClassLoader parent) throws GuacamoleException { + super(getExtensionURLs(extension, temporaryFiles), null); this.parent = parent; } diff --git a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java index d82552dad..0fcff57dd 100644 --- a/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java +++ b/guacamole/src/main/java/org/apache/guacamole/extension/ExtensionModule.java @@ -140,6 +140,13 @@ public class ExtensionModule extends ServletModule { private final List boundListeners = new ArrayList(); + /** + * All temporary files that should be deleted upon application shutdown, in + * reverse order of desired deletion. This will typically simply be the + * order that each file was created. + */ + private final List temporaryFiles = new ArrayList<>(); + /** * Service for adding and retrieving language resources. */ @@ -261,6 +268,20 @@ public class ExtensionModule extends ServletModule { return Collections.unmodifiableList(boundAuthenticationProviders); } + /** + * Returns a list of all temporary files that should be deleted upon + * application shutdown, in reverse order of desired deletion. This will + * typically simply be the order that each file was created. + * + * @return + * A List of all temporary files that should be deleted upon + * application shutdown. The List is not modifiable. + */ + @Provides + public List getTemporaryFiles() { + return Collections.unmodifiableList(temporaryFiles); + } + /** * Binds the given provider class such that a listener is bound for each * listener interface implemented by the provider and such that all bound @@ -479,7 +500,7 @@ public class ExtensionModule extends ServletModule { try { // Load extension from file - Extension extension = new Extension(getParentClassLoader(), extensionFile); + Extension extension = new Extension(getParentClassLoader(), extensionFile, temporaryFiles); // Validate Guacamole version of extension if (!isCompatible(extension.getGuacamoleVersion())) {