Workaround JDK bug 8034057

This causes a FileSystemException when trying to retrieve FileStore for a path,
and falsely returns false for Files.isWritable

Squashed commit of the following:

commit d2cc0d966f3bc94aa836b316a42b3c5724bc01ef
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Aug 18 15:49:48 2015 -0400

    fixes for the non-bogus comments

commit 6e0a272f5f8ef7358654ded8ff4ffc31831fa5c7
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Aug 18 15:30:43 2015 -0400

    Fix isWritable too

commit 2a8764ca118fc4c950bfc60d0b97de873e0e82ad
Author: Robert Muir <rmuir@apache.org>
Date:   Tue Aug 18 14:49:50 2015 -0400

    try to workaround filestore bug
This commit is contained in:
Robert Muir 2015-08-18 16:28:57 -04:00
parent 509edefb04
commit e07f039659
5 changed files with 97 additions and 9 deletions

View File

@ -76,7 +76,7 @@ public abstract class CheckFileCommand extends CliTool.Command {
if (paths != null && paths.length > 0) { if (paths != null && paths.length > 0) {
for (Path path : paths) { for (Path path : paths) {
try { try {
boolean supportsPosixPermissions = Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class); boolean supportsPosixPermissions = Environment.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
if (supportsPosixPermissions) { if (supportsPosixPermissions) {
PosixFileAttributes attributes = Files.readAttributes(path, PosixFileAttributes.class); PosixFileAttributes attributes = Files.readAttributes(path, PosixFileAttributes.class);
permissions.put(path, attributes.permissions()); permissions.put(path, attributes.permissions());

View File

@ -26,10 +26,12 @@ import org.elasticsearch.common.io.PathUtils;
import java.io.IOException; import java.io.IOException;
import java.nio.file.FileStore; import java.nio.file.FileStore;
import java.nio.file.FileSystemException;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.attribute.FileAttributeView; import java.nio.file.attribute.FileAttributeView;
import java.nio.file.attribute.FileStoreAttributeView; import java.nio.file.attribute.FileStoreAttributeView;
import java.util.Arrays;
/** /**
* Implementation of FileStore that supports * Implementation of FileStore that supports
@ -73,13 +75,16 @@ class ESFileStore extends FileStore {
} }
} }
/** Files.getFileStore(Path) useless here! Don't complain, just try it yourself. */ /**
static FileStore getMatchingFileStore(Path path, FileStore fileStores[]) throws IOException { * Files.getFileStore(Path) useless here! Don't complain, just try it yourself.
FileStore store = Files.getFileStore(path); */
@SuppressForbidden(reason = "works around the bugs")
static FileStore getMatchingFileStore(Path path, FileStore fileStores[]) throws IOException {
if (Constants.WINDOWS) { if (Constants.WINDOWS) {
return store; // be defensive, don't even try to do anything fancy. return getFileStoreWindows(path, fileStores);
} }
FileStore store = Files.getFileStore(path);
try { try {
String mount = getMountPointLinux(store); String mount = getMountPointLinux(store);
@ -110,6 +115,57 @@ class ESFileStore extends FileStore {
// fall back to crappy one we got from Files.getFileStore // fall back to crappy one we got from Files.getFileStore
return store; return store;
} }
/**
* remove this code and just use getFileStore for windows on java 9
* works around https://bugs.openjdk.java.net/browse/JDK-8034057
*/
@SuppressForbidden(reason = "works around https://bugs.openjdk.java.net/browse/JDK-8034057")
static FileStore getFileStoreWindows(Path path, FileStore fileStores[]) throws IOException {
assert Constants.WINDOWS;
try {
return Files.getFileStore(path);
} catch (FileSystemException possibleBug) {
final char driveLetter;
// look for a drive letter to see if its the SUBST bug,
// it might be some other type of path, like a windows share
// if something goes wrong, we just deliver the original exception
try {
String root = path.toRealPath().getRoot().toString();
if (root.length() < 2) {
throw new RuntimeException("root isn't a drive letter: " + root);
}
driveLetter = Character.toLowerCase(root.charAt(0));
if (Character.isAlphabetic(driveLetter) == false || root.charAt(1) != ':') {
throw new RuntimeException("root isn't a drive letter: " + root);
}
} catch (Throwable checkFailed) {
// something went wrong,
possibleBug.addSuppressed(checkFailed);
throw possibleBug;
}
// we have a drive letter: the hack begins!!!!!!!!
try {
// we have no choice but to parse toString of all stores and find the matching drive letter
for (FileStore store : fileStores) {
String toString = store.toString();
int length = toString.length();
if (length > 3 && toString.endsWith(":)") && toString.charAt(length - 4) == '(') {
if (Character.toLowerCase(toString.charAt(length - 3)) == driveLetter) {
return store;
}
}
}
throw new RuntimeException("no filestores matched");
} catch (Throwable weTried) {
IOException newException = new IOException("Unable to retrieve filestore for '" + path + "', tried matching against " + Arrays.toString(fileStores), weTried);
newException.addSuppressed(possibleBug);
throw newException;
}
}
}
@Override @Override
public String name() { public String name() {

View File

@ -19,6 +19,7 @@
package org.elasticsearch.env; package org.elasticsearch.env;
import org.apache.lucene.util.Constants;
import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterName;
import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.io.PathUtils;
@ -302,9 +303,37 @@ public class Environment {
* <li>Only requires the security permissions of {@link Files#getFileStore(Path)}, * <li>Only requires the security permissions of {@link Files#getFileStore(Path)},
* no permissions to the actual mount point are required. * no permissions to the actual mount point are required.
* <li>Exception handling has the same semantics as {@link Files#getFileStore(Path)}. * <li>Exception handling has the same semantics as {@link Files#getFileStore(Path)}.
* <li>Works around https://bugs.openjdk.java.net/browse/JDK-8034057.
* </ul> * </ul>
*/ */
public FileStore getFileStore(Path path) throws IOException { public static FileStore getFileStore(Path path) throws IOException {
return ESFileStore.getMatchingFileStore(path, fileStores); return ESFileStore.getMatchingFileStore(path, fileStores);
} }
/**
* Returns true if the path is writable.
* Acts just like {@link Files#isWritable(Path)}, except won't
* falsely return false for paths on SUBST'd drive letters
* See https://bugs.openjdk.java.net/browse/JDK-8034057
* Note this will set the file modification time (to its already-set value)
* to test access.
*/
@SuppressForbidden(reason = "works around https://bugs.openjdk.java.net/browse/JDK-8034057")
public static boolean isWritable(Path path) throws IOException {
boolean v = Files.isWritable(path);
if (v || Constants.WINDOWS == false) {
return v;
}
// isWritable returned false on windows, the hack begins!!!!!!
// resetting the modification time is the least destructive/simplest
// way to check for both files and directories, and fails early just
// in getting the current value if file doesn't exist, etc
try {
Files.setLastModifiedTime(path, Files.getLastModifiedTime(path));
return true;
} catch (Throwable e) {
return false;
}
}
} }

View File

@ -112,7 +112,7 @@ public class PluginManager {
Files.createDirectory(environment.pluginsFile()); Files.createDirectory(environment.pluginsFile());
} }
if (!Files.isWritable(environment.pluginsFile())) { if (!Environment.isWritable(environment.pluginsFile())) {
throw new IOException("plugin directory " + environment.pluginsFile() + " is read only"); throw new IOException("plugin directory " + environment.pluginsFile() + " is read only");
} }
@ -246,7 +246,7 @@ public class PluginManager {
} catch (IOException e) { } catch (IOException e) {
throw new IOException("Could not move [" + binFile + "] to [" + toLocation + "]", e); throw new IOException("Could not move [" + binFile + "] to [" + toLocation + "]", e);
} }
if (Files.getFileStore(toLocation).supportsFileAttributeView(PosixFileAttributeView.class)) { if (Environment.getFileStore(toLocation).supportsFileAttributeView(PosixFileAttributeView.class)) {
// add read and execute permissions to existing perms, so execution will work. // add read and execute permissions to existing perms, so execution will work.
// read should generally be set already, but set it anyway: don't rely on umask... // read should generally be set already, but set it anyway: don't rely on umask...
final Set<PosixFilePermission> executePerms = new HashSet<>(); final Set<PosixFilePermission> executePerms = new HashSet<>();

View File

@ -56,3 +56,6 @@ java.io.ObjectInputStream
java.io.ObjectInput java.io.ObjectInput
java.nio.file.Files#isHidden(java.nio.file.Path) @ Dependent on the operating system, use FileSystemUtils.isHidden instead java.nio.file.Files#isHidden(java.nio.file.Path) @ Dependent on the operating system, use FileSystemUtils.isHidden instead
java.nio.file.Files#getFileStore(java.nio.file.Path) @ Use Environment.getFileStore() instead, impacted by JDK-8034057
java.nio.file.Files#isWritable(java.nio.file.Path) @ Use Environment.isWritable() instead, impacted by JDK-8034057