diff --git a/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java b/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java index e8538daec56b0..74fa7e0c1d5ac 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/ESPolicy.java @@ -31,6 +31,7 @@ import java.security.Permissions; import java.security.Policy; import java.security.ProtectionDomain; +import java.util.Collections; import java.util.Map; import java.util.function.Predicate; @@ -50,7 +51,7 @@ final class ESPolicy extends Policy { ESPolicy(PermissionCollection dynamic, Map plugins, boolean filterBadDefaults) { this.template = Security.readPolicy(getClass().getResource(POLICY_RESOURCE), JarHell.parseClassPath()); - this.untrusted = Security.readPolicy(getClass().getResource(UNTRUSTED_RESOURCE), new URL[0]); + this.untrusted = Security.readPolicy(getClass().getResource(UNTRUSTED_RESOURCE), Collections.emptySet()); if (filterBadDefaults) { this.system = new SystemPolicy(Policy.getPolicy()); } else { diff --git a/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java b/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java index 22ba936d9030f..c5346bf243d90 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/JarHell.java @@ -36,9 +36,11 @@ import java.nio.file.SimpleFileVisitor; import java.nio.file.attribute.BasicFileAttributes; import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Locale; import java.util.Map; import java.util.Set; @@ -93,7 +95,7 @@ public static void checkJarHell() throws IOException, URISyntaxException { * @return array of URLs * @throws IllegalStateException if the classpath contains empty elements */ - public static URL[] parseClassPath() { + public static Set parseClassPath() { return parseClassPath(System.getProperty("java.class.path")); } @@ -104,13 +106,12 @@ public static URL[] parseClassPath() { * @throws IllegalStateException if the classpath contains empty elements */ @SuppressForbidden(reason = "resolves against CWD because that is how classpaths work") - static URL[] parseClassPath(String classPath) { + static Set parseClassPath(String classPath) { String pathSeparator = System.getProperty("path.separator"); String fileSeparator = System.getProperty("file.separator"); String elements[] = classPath.split(pathSeparator); - URL urlElements[] = new URL[elements.length]; - for (int i = 0; i < elements.length; i++) { - String element = elements[i]; + Set urlElements = new LinkedHashSet<>(); // order is already lost, but some filesystems have it + for (String element : elements) { // Technically empty classpath element behaves like CWD. // So below is the "correct" code, however in practice with ES, this is usually just a misconfiguration, // from old shell scripts left behind or something: @@ -136,13 +137,17 @@ static URL[] parseClassPath(String classPath) { } // now just parse as ordinary file try { - urlElements[i] = PathUtils.get(element).toUri().toURL(); + URL url = PathUtils.get(element).toUri().toURL(); + if (urlElements.add(url) == false) { + throw new IllegalStateException("jar hell!" + System.lineSeparator() + + "duplicate jar on classpath: " + classPath); + } } catch (MalformedURLException e) { // should not happen, as we use the filesystem API throw new RuntimeException(e); } } - return urlElements; + return Collections.unmodifiableSet(urlElements); } /** @@ -150,7 +155,7 @@ static URL[] parseClassPath(String classPath) { * @throws IllegalStateException if jar hell was found */ @SuppressForbidden(reason = "needs JarFile for speed, just reading entries") - public static void checkJarHell(URL urls[]) throws URISyntaxException, IOException { + public static void checkJarHell(Set urls) throws URISyntaxException, IOException { Logger logger = Loggers.getLogger(JarHell.class); // we don't try to be sneaky and use deprecated/internal/not portable stuff // like sun.boot.class.path, and with jigsaw we don't yet have a way to get @@ -168,8 +173,8 @@ public static void checkJarHell(URL urls[]) throws URISyntaxException, IOExcepti } if (path.toString().endsWith(".jar")) { if (!seenJars.add(path)) { - logger.debug("excluding duplicate classpath element: {}", path); - continue; + throw new IllegalStateException("jar hell!" + System.lineSeparator() + + "duplicate jar on classpath: " + path); } logger.debug("examining jar: {}", path); try (JarFile file = new JarFile(path.toString())) { @@ -198,8 +203,8 @@ public static void checkJarHell(URL urls[]) throws URISyntaxException, IOExcepti public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { String entry = root.relativize(file).toString(); if (entry.endsWith(".class")) { - // normalize with the os separator - entry = entry.replace(sep, ".").substring(0, entry.length() - 6); + // normalize with the os separator, remove '.class' + entry = entry.replace(sep, ".").substring(0, entry.length() - ".class".length()); checkClass(clazzes, entry, path); } return super.visitFile(file, attrs); diff --git a/core/src/main/java/org/elasticsearch/bootstrap/Security.java b/core/src/main/java/org/elasticsearch/bootstrap/Security.java index 3b59f235b1cf4..de16bbe76aa42 100644 --- a/core/src/main/java/org/elasticsearch/bootstrap/Security.java +++ b/core/src/main/java/org/elasticsearch/bootstrap/Security.java @@ -48,8 +48,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; /** * Initializes SecurityManager with necessary permissions. @@ -127,19 +129,23 @@ static void configure(Environment environment, boolean filterBadDefaults) throws @SuppressForbidden(reason = "proper use of URL") static Map getPluginPermissions(Environment environment) throws IOException, NoSuchAlgorithmException { Map map = new HashMap<>(); - // collect up lists of plugins and modules - List pluginsAndModules = new ArrayList<>(); + // collect up set of plugins and modules by listing directories. + Set pluginsAndModules = new LinkedHashSet<>(); // order is already lost, but some filesystems have it if (Files.exists(environment.pluginsFile())) { try (DirectoryStream stream = Files.newDirectoryStream(environment.pluginsFile())) { for (Path plugin : stream) { - pluginsAndModules.add(plugin); + if (pluginsAndModules.add(plugin) == false) { + throw new IllegalStateException("duplicate plugin: " + plugin); + } } } } if (Files.exists(environment.modulesFile())) { try (DirectoryStream stream = Files.newDirectoryStream(environment.modulesFile())) { - for (Path plugin : stream) { - pluginsAndModules.add(plugin); + for (Path module : stream) { + if (pluginsAndModules.add(module) == false) { + throw new IllegalStateException("duplicate module: " + module); + } } } } @@ -149,15 +155,18 @@ static Map getPluginPermissions(Environment environment) throws I if (Files.exists(policyFile)) { // first get a list of URLs for the plugins' jars: // we resolve symlinks so map is keyed on the normalize codebase name - List codebases = new ArrayList<>(); + Set codebases = new LinkedHashSet<>(); // order is already lost, but some filesystems have it try (DirectoryStream jarStream = Files.newDirectoryStream(plugin, "*.jar")) { for (Path jar : jarStream) { - codebases.add(jar.toRealPath().toUri().toURL()); + URL url = jar.toRealPath().toUri().toURL(); + if (codebases.add(url) == false) { + throw new IllegalStateException("duplicate module/plugin: " + url); + } } } // parse the plugin's policy file into a set of permissions - Policy policy = readPolicy(policyFile.toUri().toURL(), codebases.toArray(new URL[codebases.size()])); + Policy policy = readPolicy(policyFile.toUri().toURL(), codebases); // consult this policy for each of the plugin's jars: for (URL url : codebases) { @@ -175,24 +184,33 @@ static Map getPluginPermissions(Environment environment) throws I /** * Reads and returns the specified {@code policyFile}. *

- * Resources (e.g. jar files and directories) listed in {@code codebases} location - * will be provided to the policy file via a system property of the short name: - * e.g. ${codebase.joda-convert-1.2.jar} would map to full URL. + * Jar files listed in {@code codebases} location will be provided to the policy file via + * a system property of the short name: e.g. ${codebase.joda-convert-1.2.jar} + * would map to full URL. */ @SuppressForbidden(reason = "accesses fully qualified URLs to configure security") - static Policy readPolicy(URL policyFile, URL codebases[]) { + static Policy readPolicy(URL policyFile, Set codebases) { try { try { // set codebase properties for (URL url : codebases) { String shortName = PathUtils.get(url.toURI()).getFileName().toString(); - System.setProperty("codebase." + shortName, url.toString()); + if (shortName.endsWith(".jar") == false) { + continue; // tests :( + } + String previous = System.setProperty("codebase." + shortName, url.toString()); + if (previous != null) { + throw new IllegalStateException("codebase property already set: " + shortName + "->" + previous); + } } return Policy.getInstance("JavaPolicy", new URIParameter(policyFile.toURI())); } finally { // clear codebase properties for (URL url : codebases) { String shortName = PathUtils.get(url.toURI()).getFileName().toString(); + if (shortName.endsWith(".jar") == false) { + continue; // tests :( + } System.clearProperty("codebase." + shortName); } } diff --git a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java index be78dd927ff32..a5905e191689a 100644 --- a/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java +++ b/core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java @@ -455,8 +455,7 @@ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch, E /** check a candidate plugin for jar hell before installing it */ void jarHellCheck(Path candidate, Path pluginsDir) throws Exception { // create list of current jars in classpath - final List jars = new ArrayList<>(); - jars.addAll(Arrays.asList(JarHell.parseClassPath())); + final Set jars = new HashSet<>(JarHell.parseClassPath()); // read existing bundles. this does some checks on the installation too. PluginsService.getPluginBundles(pluginsDir); @@ -464,13 +463,15 @@ void jarHellCheck(Path candidate, Path pluginsDir) throws Exception { // add plugin jars to the list Path pluginJars[] = FileSystemUtils.files(candidate, "*.jar"); for (Path jar : pluginJars) { - jars.add(jar.toUri().toURL()); + if (jars.add(jar.toUri().toURL()) == false) { + throw new IllegalStateException("jar hell! duplicate plugin jar: " + jar); + } } // TODO: no jars should be an error // TODO: verify the classname exists in one of the jars! // check combined (current classpath + new jars to-be-added) - JarHell.checkJarHell(jars.toArray(new URL[jars.size()])); + JarHell.checkJarHell(jars); } /** diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java index 7d9887370588e..9295c6c38d872 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginsService.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginsService.java @@ -58,8 +58,10 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -107,16 +109,16 @@ public PluginsService(Settings settings, Path modulesDirectory, Path pluginsDire pluginsList.add(pluginInfo); } + Set seenBundles = new LinkedHashSet<>(); List modulesList = new ArrayList<>(); // load modules if (modulesDirectory != null) { try { - List bundles = getModuleBundles(modulesDirectory); - List> loaded = loadBundles(bundles); - pluginsLoaded.addAll(loaded); - for (Tuple module : loaded) { - modulesList.add(module.v1()); + Set modules = getModuleBundles(modulesDirectory); + for (Bundle bundle : modules) { + modulesList.add(bundle.plugin); } + seenBundles.addAll(modules); } catch (IOException ex) { throw new IllegalStateException("Unable to initialize modules", ex); } @@ -125,17 +127,19 @@ public PluginsService(Settings settings, Path modulesDirectory, Path pluginsDire // now, find all the ones that are in plugins/ if (pluginsDirectory != null) { try { - List bundles = getPluginBundles(pluginsDirectory); - List> loaded = loadBundles(bundles); - pluginsLoaded.addAll(loaded); - for (Tuple plugin : loaded) { - pluginsList.add(plugin.v1()); + Set plugins = getPluginBundles(pluginsDirectory); + for (Bundle bundle : plugins) { + pluginsList.add(bundle.plugin); } + seenBundles.addAll(plugins); } catch (IOException ex) { throw new IllegalStateException("Unable to initialize plugins", ex); } } + List> loaded = loadBundles(seenBundles); + pluginsLoaded.addAll(loaded); + this.info = new PluginsAndModules(pluginsList, modulesList); this.plugins = Collections.unmodifiableList(pluginsLoaded); @@ -234,48 +238,70 @@ public PluginsAndModules info() { // a "bundle" is a group of plugins in a single classloader // really should be 1-1, but we are not so fortunate static class Bundle { - List plugins = new ArrayList<>(); - List urls = new ArrayList<>(); + final PluginInfo plugin; + final Set urls; + + Bundle(PluginInfo plugin, Set urls) { + this.plugin = Objects.requireNonNull(plugin); + this.urls = Objects.requireNonNull(urls); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Bundle bundle = (Bundle) o; + return Objects.equals(plugin, bundle.plugin); + } + + @Override + public int hashCode() { + return Objects.hash(plugin); + } } // similar in impl to getPluginBundles, but DO NOT try to make them share code. // we don't need to inherit all the leniency, and things are different enough. - static List getModuleBundles(Path modulesDirectory) throws IOException { + static Set getModuleBundles(Path modulesDirectory) throws IOException { // damn leniency if (Files.notExists(modulesDirectory)) { - return Collections.emptyList(); + return Collections.emptySet(); } - List bundles = new ArrayList<>(); + Set bundles = new LinkedHashSet<>(); try (DirectoryStream stream = Files.newDirectoryStream(modulesDirectory)) { for (Path module : stream) { if (FileSystemUtils.isHidden(module)) { continue; // skip over .DS_Store etc } PluginInfo info = PluginInfo.readFromProperties(module); - Bundle bundle = new Bundle(); - bundle.plugins.add(info); + Set urls = new LinkedHashSet<>(); // gather urls for jar files try (DirectoryStream jarStream = Files.newDirectoryStream(module, "*.jar")) { for (Path jar : jarStream) { // normalize with toRealPath to get symlinks out of our hair - bundle.urls.add(jar.toRealPath().toUri().toURL()); + URL url = jar.toRealPath().toUri().toURL(); + if (urls.add(url) == false) { + throw new IllegalStateException("duplicate codebase: " + url); + } } } - bundles.add(bundle); + if (bundles.add(new Bundle(info, urls)) == false) { + throw new IllegalStateException("duplicate module: " + info); + } } } return bundles; } - static List getPluginBundles(Path pluginsDirectory) throws IOException { + static Set getPluginBundles(Path pluginsDirectory) throws IOException { Logger logger = Loggers.getLogger(PluginsService.class); // TODO: remove this leniency, but tests bogusly rely on it if (!isAccessibleDirectory(pluginsDirectory, logger)) { - return Collections.emptyList(); + return Collections.emptySet(); } - List bundles = new ArrayList<>(); + Set bundles = new LinkedHashSet<>(); try (DirectoryStream stream = Files.newDirectoryStream(pluginsDirectory)) { for (Path plugin : stream) { @@ -292,47 +318,58 @@ static List getPluginBundles(Path pluginsDirectory) throws IOException { + plugin.getFileName() + "]. Was the plugin built before 2.0?", e); } - List urls = new ArrayList<>(); + Set urls = new LinkedHashSet<>(); try (DirectoryStream jarStream = Files.newDirectoryStream(plugin, "*.jar")) { for (Path jar : jarStream) { // normalize with toRealPath to get symlinks out of our hair - urls.add(jar.toRealPath().toUri().toURL()); + URL url = jar.toRealPath().toUri().toURL(); + if (urls.add(url) == false) { + throw new IllegalStateException("duplicate codebase: " + url); + } } } - final Bundle bundle = new Bundle(); - bundles.add(bundle); - bundle.plugins.add(info); - bundle.urls.addAll(urls); + if (bundles.add(new Bundle(info, urls)) == false) { + throw new IllegalStateException("duplicate plugin: " + info); + } } } return bundles; } - private List> loadBundles(List bundles) { + private List> loadBundles(Set bundles) { List> plugins = new ArrayList<>(); for (Bundle bundle : bundles) { // jar-hell check the bundle against the parent classloader // pluginmanager does it, but we do it again, in case lusers mess with jar files manually try { - final List jars = new ArrayList<>(); - jars.addAll(Arrays.asList(JarHell.parseClassPath())); - jars.addAll(bundle.urls); - JarHell.checkJarHell(jars.toArray(new URL[0])); + Set classpath = JarHell.parseClassPath(); + // check we don't have conflicting codebases + Set intersection = new HashSet<>(classpath); + intersection.retainAll(bundle.urls); + if (intersection.isEmpty() == false) { + throw new IllegalStateException("jar hell! duplicate codebases between" + + " plugin and core: " + intersection); + } + // check we don't have conflicting classes + Set union = new HashSet<>(classpath); + union.addAll(bundle.urls); + JarHell.checkJarHell(union); } catch (Exception e) { - throw new IllegalStateException("failed to load bundle " + bundle.urls + " due to jar hell", e); + throw new IllegalStateException("failed to load plugin " + bundle.plugin + + " due to jar hell", e); } - // create a child to load the plugins in this bundle - ClassLoader loader = URLClassLoader.newInstance(bundle.urls.toArray(new URL[0]), getClass().getClassLoader()); - for (PluginInfo pluginInfo : bundle.plugins) { - // reload lucene SPI with any new services from the plugin - reloadLuceneSPI(loader); - final Class pluginClass = loadPluginClass(pluginInfo.getClassname(), loader); - final Plugin plugin = loadPlugin(pluginClass, settings); - plugins.add(new Tuple<>(pluginInfo, plugin)); - } + // create a child to load the plugin in this bundle + ClassLoader loader = URLClassLoader.newInstance(bundle.urls.toArray(new URL[0]), + getClass().getClassLoader()); + // reload lucene SPI with any new services from the plugin + reloadLuceneSPI(loader); + final Class pluginClass = + loadPluginClass(bundle.plugin.getClassname(), loader); + final Plugin plugin = loadPlugin(pluginClass, settings); + plugins.add(new Tuple<>(bundle.plugin, plugin)); } return Collections.unmodifiableList(plugins); diff --git a/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java b/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java index d38d346d6c14f..7003ef3d81efe 100644 --- a/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java +++ b/core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java @@ -30,7 +30,9 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Set; import java.util.jar.Attributes; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; @@ -62,7 +64,8 @@ URL makeFile(Path dir, String name) throws IOException { public void testDifferentJars() throws Exception { Path dir = createTempDir(); - URL[] jars = {makeJar(dir, "foo.jar", null, "DuplicateClass.class"), makeJar(dir, "bar.jar", null, "DuplicateClass.class")}; + Set jars = asSet(makeJar(dir, "foo.jar", null, "DuplicateClass.class"), + makeJar(dir, "bar.jar", null, "DuplicateClass.class")); try { JarHell.checkJarHell(jars); fail("did not get expected exception"); @@ -74,17 +77,11 @@ public void testDifferentJars() throws Exception { } } - public void testDuplicateClasspathLeniency() throws Exception { - Path dir = createTempDir(); - URL jar = makeJar(dir, "foo.jar", null, "Foo.class"); - URL[] jars = {jar, jar}; - JarHell.checkJarHell(jars); - } - public void testDirsOnClasspath() throws Exception { Path dir1 = createTempDir(); Path dir2 = createTempDir(); - URL[] dirs = {makeFile(dir1, "DuplicateClass.class"), makeFile(dir2, "DuplicateClass.class")}; + Set dirs = asSet(makeFile(dir1, "DuplicateClass.class"), + makeFile(dir2, "DuplicateClass.class")); try { JarHell.checkJarHell(dirs); fail("did not get expected exception"); @@ -99,7 +96,8 @@ public void testDirsOnClasspath() throws Exception { public void testDirAndJar() throws Exception { Path dir1 = createTempDir(); Path dir2 = createTempDir(); - URL[] dirs = {makeJar(dir1, "foo.jar", null, "DuplicateClass.class"), makeFile(dir2, "DuplicateClass.class")}; + Set dirs = asSet(makeJar(dir1, "foo.jar", null, "DuplicateClass.class"), + makeFile(dir2, "DuplicateClass.class")); try { JarHell.checkJarHell(dirs); fail("did not get expected exception"); @@ -113,8 +111,8 @@ public void testDirAndJar() throws Exception { public void testWithinSingleJar() throws Exception { // the java api for zip file does not allow creating duplicate entries (good!) so - // this bogus jar had to be constructed with ant - URL[] jars = {JarHellTests.class.getResource("duplicate-classes.jar")}; + // this bogus jar had to be with https://github.com/jasontedor/duplicate-classes + Set jars = Collections.singleton(JarHellTests.class.getResource("duplicate-classes.jar")); try { JarHell.checkJarHell(jars); fail("did not get expected exception"); @@ -127,7 +125,7 @@ public void testWithinSingleJar() throws Exception { } public void testXmlBeansLeniency() throws Exception { - URL[] jars = {JarHellTests.class.getResource("duplicate-xmlbeans-classes.jar")}; + Set jars = Collections.singleton(JarHellTests.class.getResource("duplicate-xmlbeans-classes.jar")); JarHell.checkJarHell(jars); } @@ -145,7 +143,7 @@ public void testRequiredJDKVersionTooOld() throws Exception { Attributes attributes = manifest.getMainAttributes(); attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0.0"); attributes.put(new Attributes.Name("X-Compile-Target-JDK"), targetVersion.toString()); - URL[] jars = {makeJar(dir, "foo.jar", manifest, "Foo.class")}; + Set jars = Collections.singleton(makeJar(dir, "foo.jar", manifest, "Foo.class")); try { JarHell.checkJarHell(jars); fail("did not get expected exception"); @@ -161,7 +159,7 @@ public void testBadJDKVersionInJar() throws Exception { Attributes attributes = manifest.getMainAttributes(); attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0.0"); attributes.put(new Attributes.Name("X-Compile-Target-JDK"), "bogus"); - URL[] jars = {makeJar(dir, "foo.jar", manifest, "Foo.class")}; + Set jars = Collections.singleton(makeJar(dir, "foo.jar", manifest, "Foo.class")); try { JarHell.checkJarHell(jars); fail("did not get expected exception"); @@ -176,8 +174,7 @@ public void testRequiredJDKVersionIsOK() throws Exception { Attributes attributes = manifest.getMainAttributes(); attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0.0"); attributes.put(new Attributes.Name("X-Compile-Target-JDK"), "1.7"); - URL[] jars = {makeJar(dir, "foo.jar", manifest, "Foo.class")}; - + Set jars = Collections.singleton(makeJar(dir, "foo.jar", manifest, "Foo.class")); JarHell.checkJarHell(jars); } @@ -188,7 +185,7 @@ public void testGoodESVersionInJar() throws Exception { Attributes attributes = manifest.getMainAttributes(); attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0.0"); attributes.put(new Attributes.Name("X-Compile-Elasticsearch-Version"), Version.CURRENT.toString()); - URL[] jars = {makeJar(dir, "foo.jar", manifest, "Foo.class")}; + Set jars = Collections.singleton(makeJar(dir, "foo.jar", manifest, "Foo.class")); JarHell.checkJarHell(jars); } @@ -199,7 +196,7 @@ public void testBadESVersionInJar() throws Exception { Attributes attributes = manifest.getMainAttributes(); attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0.0"); attributes.put(new Attributes.Name("X-Compile-Elasticsearch-Version"), "1.0-bogus"); - URL[] jars = {makeJar(dir, "foo.jar", manifest, "Foo.class")}; + Set jars = Collections.singleton(makeJar(dir, "foo.jar", manifest, "Foo.class")); try { JarHell.checkJarHell(jars); fail("did not get expected exception"); @@ -242,8 +239,8 @@ public void testParseClassPathUnix() throws Exception { Path element1 = createTempDir(); Path element2 = createTempDir(); - URL expected[] = { element1.toUri().toURL(), element2.toUri().toURL() }; - assertArrayEquals(expected, JarHell.parseClassPath(element1.toString() + ":" + element2.toString())); + Set expected = asSet(element1.toUri().toURL(), element2.toUri().toURL()); + assertEquals(expected, JarHell.parseClassPath(element1.toString() + ":" + element2.toString())); } /** @@ -271,8 +268,8 @@ public void testParseClassPathWindows() throws Exception { Path element1 = createTempDir(); Path element2 = createTempDir(); - URL expected[] = { element1.toUri().toURL(), element2.toUri().toURL() }; - assertArrayEquals(expected, JarHell.parseClassPath(element1.toString() + ";" + element2.toString())); + Set expected = asSet(element1.toUri().toURL(), element2.toUri().toURL()); + assertEquals(expected, JarHell.parseClassPath(element1.toString() + ";" + element2.toString())); } /** @@ -298,13 +295,13 @@ public void testCrazyEclipseClassPathWindows() throws Exception { assumeTrue("test is designed for windows-like systems only", ";".equals(System.getProperty("path.separator"))); assumeTrue("test is designed for windows-like systems only", "\\".equals(System.getProperty("file.separator"))); - URL expected[] = { + Set expected = asSet( PathUtils.get("c:\\element1").toUri().toURL(), PathUtils.get("c:\\element2").toUri().toURL(), PathUtils.get("c:\\element3").toUri().toURL(), - PathUtils.get("c:\\element 4").toUri().toURL(), - }; - URL actual[] = JarHell.parseClassPath("c:\\element1;c:\\element2;/c:/element3;/c:/element 4"); - assertArrayEquals(expected, actual); + PathUtils.get("c:\\element 4").toUri().toURL() + ); + Set actual = JarHell.parseClassPath("c:\\element1;c:\\element2;/c:/element3;/c:/element 4"); + assertEquals(expected, actual); } } diff --git a/distribution/src/main/resources/bin/elasticsearch.in.bat b/distribution/src/main/resources/bin/elasticsearch.in.bat index 98b6a16316c2d..a25008338729c 100644 --- a/distribution/src/main/resources/bin/elasticsearch.in.bat +++ b/distribution/src/main/resources/bin/elasticsearch.in.bat @@ -15,7 +15,7 @@ for %%I in ("%SCRIPT_DIR%..") do set ES_HOME=%%~dpfI REM check in case a user was using this mechanism if "%ES_CLASSPATH%" == "" ( -set ES_CLASSPATH=!ES_HOME!/lib/elasticsearch-${project.version}.jar;!ES_HOME!/lib/* +set ES_CLASSPATH=!ES_HOME!/lib/* ) else ( ECHO Error: Don't modify the classpath with ES_CLASSPATH, Best is to add 1>&2 ECHO additional elements via the plugin mechanism, or if code must really be 1>&2 diff --git a/distribution/src/main/resources/bin/elasticsearch.in.sh b/distribution/src/main/resources/bin/elasticsearch.in.sh index 58b26a2d6ebc7..2d22439225697 100644 --- a/distribution/src/main/resources/bin/elasticsearch.in.sh +++ b/distribution/src/main/resources/bin/elasticsearch.in.sh @@ -10,4 +10,4 @@ EOF exit 1 fi -ES_CLASSPATH="$ES_HOME/lib/elasticsearch-${project.version}.jar:$ES_HOME/lib/*" +ES_CLASSPATH="$ES_HOME/lib/*" diff --git a/plugins/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java b/plugins/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java index c7ffe4f287f0c..3c0f3b0433c0c 100644 --- a/plugins/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java +++ b/plugins/ingest-attachment/src/main/java/org/elasticsearch/ingest/attachment/TikaImpl.java @@ -47,7 +47,9 @@ import java.security.PrivilegedExceptionAction; import java.security.ProtectionDomain; import java.security.SecurityPermission; +import java.util.Arrays; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.PropertyPermission; import java.util.Set; @@ -128,7 +130,12 @@ static PermissionCollection getRestrictedPermissions() { addReadPermissions(perms, JarHell.parseClassPath()); // plugin jars if (TikaImpl.class.getClassLoader() instanceof URLClassLoader) { - addReadPermissions(perms, ((URLClassLoader)TikaImpl.class.getClassLoader()).getURLs()); + URL[] urls = ((URLClassLoader)TikaImpl.class.getClassLoader()).getURLs(); + Set set = new LinkedHashSet<>(Arrays.asList(urls)); + if (set.size() != urls.length) { + throw new AssertionError("duplicate jars: " + Arrays.toString(urls)); + } + addReadPermissions(perms, set); } // jvm's java.io.tmpdir (needs read/write) perms.add(new FilePermission(System.getProperty("java.io.tmpdir") + System.getProperty("file.separator") + "-", @@ -145,7 +152,7 @@ static PermissionCollection getRestrictedPermissions() { // add resources to (what is typically) a jar, but might not be (e.g. in tests/IDE) @SuppressForbidden(reason = "adds access to jar resources") - static void addReadPermissions(Permissions perms, URL resources[]) { + static void addReadPermissions(Permissions perms, Set resources) { try { for (URL url : resources) { Path path = PathUtils.get(url.toURI()); diff --git a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java index fe833470ad255..4ab45d8c1a62b 100644 --- a/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java +++ b/test/framework/src/main/java/org/elasticsearch/bootstrap/BootstrapForTesting.java @@ -182,7 +182,7 @@ static Map getPluginPermissions() throws Exception { } // compute classpath minus obvious places, all other jars will get the permission. - Set codebases = new HashSet<>(Arrays.asList(parseClassPathWithSymlinks())); + Set codebases = new HashSet<>(parseClassPathWithSymlinks()); Set excluded = new HashSet<>(Arrays.asList( // es core Bootstrap.class.getProtectionDomain().getCodeSource().getLocation(), @@ -200,7 +200,7 @@ static Map getPluginPermissions() throws Exception { // parse each policy file, with codebase substitution from the classpath final List policies = new ArrayList<>(); for (URL policyFile : pluginPolicies) { - policies.add(Security.readPolicy(policyFile, codebases.toArray(new URL[codebases.size()]))); + policies.add(Security.readPolicy(policyFile, codebases)); } // consult each policy file for those codebases @@ -227,10 +227,14 @@ public boolean implies(ProtectionDomain domain, Permission permission) { * this is for matching the toRealPath() in the code where we have a proper plugin structure */ @SuppressForbidden(reason = "does evil stuff with paths and urls because devs and jenkins do evil stuff with paths and urls") - static URL[] parseClassPathWithSymlinks() throws Exception { - URL raw[] = JarHell.parseClassPath(); - for (int i = 0; i < raw.length; i++) { - raw[i] = PathUtils.get(raw[i].toURI()).toRealPath().toUri().toURL(); + static Set parseClassPathWithSymlinks() throws Exception { + Set raw = JarHell.parseClassPath(); + Set cooked = new HashSet<>(raw.size()); + for (URL url : raw) { + boolean added = cooked.add(PathUtils.get(url.toURI()).toRealPath().toUri().toURL()); + if (added == false) { + throw new IllegalStateException("Duplicate in classpath after resolving symlinks: " + url); + } } return raw; }