Skip to content

Commit

Permalink
Fail hard on empty classpath elements.
Browse files Browse the repository at this point in the history
This can happen easily, if somehow old 1.x shellscripts survive and try to launch 2.x code.
I have the feeling this happens maybe because of packaging upgrades or something.
Either way: we can just fail hard and clear in this situation, rather than the current situation
where CWD might be /, and we might traverse the entire filesystem until we hit an error...

Relates to #13864
  • Loading branch information
rmuir committed Sep 30, 2015
1 parent 5915309 commit 415d897
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 5 deletions.
28 changes: 23 additions & 5 deletions core/src/main/java/org/elasticsearch/bootstrap/JarHell.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,35 @@ public static void checkJarHell() throws Exception {
}

/**
* Parses the classpath into a set of URLs
* Parses the classpath into an array of URLs
* @return array of URLs
* @throws IllegalStateException if the classpath contains empty elements
*/
@SuppressForbidden(reason = "resolves against CWD because that is how classpaths work")
public static URL[] parseClassPath() {
String elements[] = System.getProperty("java.class.path").split(System.getProperty("path.separator"));
return parseClassPath(System.getProperty("java.class.path"));
}

/**
* Parses the classpath into a set of URLs. For testing.
* @param classPath classpath to parse (typically the system property {@code java.class.path})
* @return array of URLs
* @throws IllegalStateException if the classpath contains empty elements
*/
@SuppressForbidden(reason = "resolves against CWD because that is how classpaths work")
static URL[] parseClassPath(String classPath) {
String elements[] = classPath.split(System.getProperty("path.separator"));
URL urlElements[] = new URL[elements.length];
for (int i = 0; i < elements.length; i++) {
String element = elements[i];
// empty classpath element behaves like CWD.
// 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:
// if (element.isEmpty()) {
// element = System.getProperty("user.dir");
// }
// Instead we just throw an exception, and keep it clean.
if (element.isEmpty()) {
element = System.getProperty("user.dir");
throw new IllegalStateException("Classpath should not contain empty elements! (outdated shell script from a previous version?) classpath='" + classPath + "'");
}
try {
urlElements[i] = PathUtils.get(element).toUri().toURL();
Expand Down
60 changes: 60 additions & 0 deletions core/src/test/java/org/elasticsearch/bootstrap/JarHellTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -275,4 +275,64 @@ public void testInvalidVersions() {
}
}
}

// classpath testing is system specific, so we just write separate tests for *nix and windows cases

/**
* Parse a simple classpath with two elements on unix
*/
public void testParseClassPathUnix() throws Exception {
assumeTrue("test is designed for unix-like systems only", ":".equals(System.getProperty("path.separator")));
assumeTrue("test is designed for unix-like systems only", "/".equals(System.getProperty("file.separator")));

Path element1 = createTempDir();
Path element2 = createTempDir();

URL expected[] = { element1.toUri().toURL(), element2.toUri().toURL() };
assertArrayEquals(expected, JarHell.parseClassPath(element1.toString() + ":" + element2.toString()));
}

/**
* Make sure an old unix classpath with an empty element (implicitly CWD: i'm looking at you 1.x ES scripts) fails
*/
public void testEmptyClassPathUnix() throws Exception {
assumeTrue("test is designed for unix-like systems only", ":".equals(System.getProperty("path.separator")));
assumeTrue("test is designed for unix-like systems only", "/".equals(System.getProperty("file.separator")));

try {
JarHell.parseClassPath(":/element1:/element2");
fail("should have hit exception");
} catch (IllegalStateException expected) {
assertTrue(expected.getMessage().contains("should not contain empty elements"));
}
}

/**
* Parse a simple classpath with two elements on windows
*/
public void testParseClassPathWindows() 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")));

Path element1 = createTempDir();
Path element2 = createTempDir();

URL expected[] = { element1.toUri().toURL(), element2.toUri().toURL() };
assertArrayEquals(expected, JarHell.parseClassPath(element1.toString() + ";" + element2.toString()));
}

/**
* Make sure an old windows classpath with an empty element (implicitly CWD: i'm looking at you 1.x ES scripts) fails
*/
public void testEmptyClassPathWindows() 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")));

try {
JarHell.parseClassPath(";c:\\element1;c:\\element2");
fail("should have hit exception");
} catch (IllegalStateException expected) {
assertTrue(expected.getMessage().contains("should not contain empty elements"));
}
}
}

0 comments on commit 415d897

Please sign in to comment.