From 36423442a9a2ec0ab765ba4929a3ff6e633452a1 Mon Sep 17 00:00:00 2001 From: evani balasubramanyam Date: Sun, 8 Aug 2021 03:49:03 +0530 Subject: [PATCH 1/3] Segregated exception checks in PluginsService checkBundleJarHell, also added a unit test to test io error msg --- .../elasticsearch/plugins/PluginsService.java | 7 ++++++- .../plugins/PluginsServiceTests.java | 20 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index 86479d2499376..1368fd4bbee49 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -36,6 +36,7 @@ import java.lang.reflect.Constructor; import java.net.URL; import java.net.URLClassLoader; +import java.net.URISyntaxException; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; @@ -567,7 +568,11 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map union = new HashSet<>(classpath); union.addAll(bundle.urls); JarHell.checkJarHell(union, logger::debug); - } catch (Exception e) { + } catch (final IOException ioe) { + throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to io error, caused when checking for Jar Hell", ioe); + } catch (final URISyntaxException urie) { + throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to malformed uri, caused when checking for Jar Hell", urie); + } catch (final Exception e) { throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to jar hell", e); } } diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 6a21be7db67ce..84be790c3ea7b 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -500,6 +500,26 @@ public void testJarHellDuplicateClassWithCore() throws Exception { assertThat(e.getCause().getMessage(), containsString("Level")); } + public void testJarHellWhenExtendedPluginJarNotFound() throws Exception { + Path pluginDir = createTempDir(); + Path pluginJar = pluginDir.resolve("dummy.jar"); + + Path otherDir = createTempDir(); + Path extendedPlugin = otherDir.resolve("extendedDep-not-present.jar"); + + PluginInfo info = new PluginInfo("dummy", "desc", "1.0", Version.CURRENT, "1.8", + "Dummy", Arrays.asList("extendedPlugin"), false, PluginType.ISOLATED, "", false); + + PluginsService.Bundle bundle = new PluginsService.Bundle(info, pluginDir); + Map> transitiveUrls = new HashMap<>(); + transitiveUrls.put("extendedPlugin", Collections.singleton(extendedPlugin.toUri().toURL())); + + IllegalStateException e = expectThrows(IllegalStateException.class, () -> + PluginsService.checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveUrls)); + + assertEquals("failed to load plugin dummy due to io error, caused when checking for Jar Hell", e.getMessage()); + } + public void testJarHellDuplicateClassWithDep() throws Exception { Path pluginDir = createTempDir(); Path pluginJar = pluginDir.resolve("plugin.jar"); From f90a8e8581dc154a59f747e08412d56cc9b8b4c0 Mon Sep 17 00:00:00 2001 From: evani balasubramanyam Date: Sun, 8 Aug 2021 20:53:48 +0530 Subject: [PATCH 2/3] modified checkBundleJarHell, adding specialized check only for IllegalStateException --- .../java/org/elasticsearch/plugins/PluginsService.java | 9 +++------ .../org/elasticsearch/plugins/PluginsServiceTests.java | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java index 1368fd4bbee49..c1a37ffcf6b11 100644 --- a/server/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/server/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -36,7 +36,6 @@ import java.lang.reflect.Constructor; import java.net.URL; import java.net.URLClassLoader; -import java.net.URISyntaxException; import java.nio.file.DirectoryStream; import java.nio.file.Files; import java.nio.file.Path; @@ -568,12 +567,10 @@ static void checkBundleJarHell(Set classpath, Bundle bundle, Map union = new HashSet<>(classpath); union.addAll(bundle.urls); JarHell.checkJarHell(union, logger::debug); - } catch (final IOException ioe) { - throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to io error, caused when checking for Jar Hell", ioe); - } catch (final URISyntaxException urie) { - throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to malformed uri, caused when checking for Jar Hell", urie); + } catch (final IllegalStateException ise) { + throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to jar hell", ise); } catch (final Exception e) { - throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " due to jar hell", e); + throw new IllegalStateException("failed to load plugin " + bundle.plugin.getName() + " while checking for jar hell", e); } } diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 84be790c3ea7b..4d03970e85646 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -517,7 +517,7 @@ public void testJarHellWhenExtendedPluginJarNotFound() throws Exception { IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsService.checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveUrls)); - assertEquals("failed to load plugin dummy due to io error, caused when checking for Jar Hell", e.getMessage()); + assertEquals("failed to load plugin dummy while checking for jar hell", e.getMessage()); } public void testJarHellDuplicateClassWithDep() throws Exception { From c1dfe69d280bdc09e1c6e31aef2a85bddb53306c Mon Sep 17 00:00:00 2001 From: evani balasubramanyam Date: Mon, 9 Aug 2021 10:45:41 +0530 Subject: [PATCH 3/3] removing unwanted tabs from testJarHellWhenExtendedPluginJarNotFound test --- .../plugins/PluginsServiceTests.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java index 4d03970e85646..a10902fa92739 100644 --- a/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java +++ b/server/src/test/java/org/elasticsearch/plugins/PluginsServiceTests.java @@ -501,23 +501,23 @@ public void testJarHellDuplicateClassWithCore() throws Exception { } public void testJarHellWhenExtendedPluginJarNotFound() throws Exception { - Path pluginDir = createTempDir(); - Path pluginJar = pluginDir.resolve("dummy.jar"); + Path pluginDir = createTempDir(); + Path pluginJar = pluginDir.resolve("dummy.jar"); - Path otherDir = createTempDir(); - Path extendedPlugin = otherDir.resolve("extendedDep-not-present.jar"); + Path otherDir = createTempDir(); + Path extendedPlugin = otherDir.resolve("extendedDep-not-present.jar"); - PluginInfo info = new PluginInfo("dummy", "desc", "1.0", Version.CURRENT, "1.8", + PluginInfo info = new PluginInfo("dummy", "desc", "1.0", Version.CURRENT, "1.8", "Dummy", Arrays.asList("extendedPlugin"), false, PluginType.ISOLATED, "", false); - PluginsService.Bundle bundle = new PluginsService.Bundle(info, pluginDir); - Map> transitiveUrls = new HashMap<>(); - transitiveUrls.put("extendedPlugin", Collections.singleton(extendedPlugin.toUri().toURL())); + PluginsService.Bundle bundle = new PluginsService.Bundle(info, pluginDir); + Map> transitiveUrls = new HashMap<>(); + transitiveUrls.put("extendedPlugin", Collections.singleton(extendedPlugin.toUri().toURL())); - IllegalStateException e = expectThrows(IllegalStateException.class, () -> + IllegalStateException e = expectThrows(IllegalStateException.class, () -> PluginsService.checkBundleJarHell(JarHell.parseClassPath(), bundle, transitiveUrls)); - assertEquals("failed to load plugin dummy while checking for jar hell", e.getMessage()); + assertEquals("failed to load plugin dummy while checking for jar hell", e.getMessage()); } public void testJarHellDuplicateClassWithDep() throws Exception {