diff --git a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java index 06abfb3..922bcb1 100644 --- a/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java +++ b/src/main/java/org/cryptomator/integrations/common/ClassLoaderFactory.java @@ -18,7 +18,6 @@ class ClassLoaderFactory { private static final Logger LOG = LoggerFactory.getLogger(ClassLoaderFactory.class); - private static final String USER_HOME = System.getProperty("user.home"); private static final String PLUGIN_DIR_KEY = "cryptomator.pluginDir"; private static final String JAR_SUFFIX = ".jar"; @@ -30,21 +29,27 @@ class ClassLoaderFactory { */ @Contract(value = "-> new", pure = true) public static URLClassLoader forPluginDir() { - String val = System.getProperty(PLUGIN_DIR_KEY, ""); - final Path p; - if (val.startsWith("~/")) { - p = Path.of(USER_HOME).resolve(val.substring(2)); - } else { - p = Path.of(val); + String val = System.getProperty(PLUGIN_DIR_KEY); + if (val == null) { + return URLClassLoader.newInstance(new URL[0]); + } + + try { + if (val.isBlank()) { + throw new IllegalArgumentException("Plugin dir path is blank"); + } + return forPluginDirWithPath(Path.of(val)); //Path.of() might throw InvalidPathException + } catch (IllegalArgumentException e) { + LOG.debug("{} contains illegal value. Skipping plugin directory.", PLUGIN_DIR_KEY, e); + return URLClassLoader.newInstance(new URL[0]); } - return forPluginDirWithPath(p); } @VisibleForTesting @Contract(value = "_ -> new", pure = true) static URLClassLoader forPluginDirWithPath(Path path) throws UncheckedIOException { var jars = findJars(path); - if (LOG.isDebugEnabled()) { + if (LOG.isDebugEnabled() && jars.length != 0) { String jarList = Arrays.stream(jars).map(URL::getPath).collect(Collectors.joining(", ")); LOG.debug("Found jars in cryptomator.pluginDir: {}", jarList); } @@ -56,7 +61,7 @@ static URL[] findJars(Path path) { try (var stream = Files.walk(path)) { return stream.filter(ClassLoaderFactory::isJarFile).map(ClassLoaderFactory::toUrl).toArray(URL[]::new); } catch (IOException | UncheckedIOException e) { - // unable to locate any jars // TODO: log a warning? + LOG.debug("Failed to read plugin dir {}", path, e); return new URL[0]; } } diff --git a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java index 2463966..94ec896 100644 --- a/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java +++ b/src/test/java/org/cryptomator/integrations/common/ClassLoaderFactoryTest.java @@ -1,11 +1,16 @@ package org.cryptomator.integrations.common; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EmptySource; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.MockedStatic; import org.mockito.Mockito; import java.io.ByteArrayInputStream; @@ -17,6 +22,9 @@ import java.util.Arrays; import java.util.Comparator; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; + public class ClassLoaderFactoryTest { @Nested @@ -91,21 +99,49 @@ public void testReadPluginDirFromSysProp() { } } - @Test - @DisplayName("read path from cryptomator.pluginDir and replace ~/ with user.home") - public void testReadPluginDirFromSysPropAndReplaceHome() { - var ucl = Mockito.mock(URLClassLoader.class, "ucl"); - var relPath = "~/there/will/be/plugins"; - var absPath = Path.of(System.getProperty("user.home")).resolve("there/will/be/plugins"); - try (var mockedClass = Mockito.mockStatic(ClassLoaderFactory.class)) { + @Nested + @DisplayName("when the system property contains invalid values") + public class InvalidSystemProperty { + + MockedStatic mockedClass; + + @BeforeEach + public void beforeEach() { + mockedClass = Mockito.mockStatic(ClassLoaderFactory.class); mockedClass.when(() -> ClassLoaderFactory.forPluginDir()).thenCallRealMethod(); - mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(absPath)).thenReturn(ucl); + mockedClass.when(() -> ClassLoaderFactory.forPluginDirWithPath(any())).thenReturn(null); + } - System.setProperty("cryptomator.pluginDir", relPath); + @AfterEach + public void afterEach() { + mockedClass.close(); + } + + + @Test + @DisplayName("Undefined cryptomator.pluginDir returns empty URLClassLoader") + public void testUndefinedSysProp() { + System.clearProperty("cryptomator.pluginDir"); var result = ClassLoaderFactory.forPluginDir(); - Assertions.assertSame(ucl, result); + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any()), never()); + Assertions.assertNotNull(result); + Assertions.assertEquals(0, result.getURLs().length); } + + @ParameterizedTest + @DisplayName("Property cryptomator.pluginDir filled with blanks returns empty URLClassLoader") + @EmptySource + @ValueSource(strings = {"\t\t", " "}) + public void testBlankSysProp(String propValue) { + System.setProperty("cryptomator.pluginDir", propValue); + var result = ClassLoaderFactory.forPluginDir(); + + mockedClass.verify(() -> ClassLoaderFactory.forPluginDirWithPath(any()), never()); + Assertions.assertNotNull(result); + Assertions.assertEquals(0, result.getURLs().length); + } + } @Test