From e07f0396598d322a53efb1c8a7d2abd46f2d05ea Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Tue, 18 Aug 2015 16:28:57 -0400 Subject: [PATCH] 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 Date: Tue Aug 18 15:49:48 2015 -0400 fixes for the non-bogus comments commit 6e0a272f5f8ef7358654ded8ff4ffc31831fa5c7 Author: Robert Muir Date: Tue Aug 18 15:30:43 2015 -0400 Fix isWritable too commit 2a8764ca118fc4c950bfc60d0b97de873e0e82ad Author: Robert Muir Date: Tue Aug 18 14:49:50 2015 -0400 try to workaround filestore bug --- .../common/cli/CheckFileCommand.java | 2 +- .../org/elasticsearch/env/ESFileStore.java | 66 +++++++++++++++++-- .../org/elasticsearch/env/Environment.java | 31 ++++++++- .../elasticsearch/plugins/PluginManager.java | 4 +- .../resources/forbidden/all-signatures.txt | 3 + 5 files changed, 97 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java b/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java index 3273daf75f9..2c8aa709edb 100644 --- a/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java +++ b/core/src/main/java/org/elasticsearch/common/cli/CheckFileCommand.java @@ -76,7 +76,7 @@ public abstract class CheckFileCommand extends CliTool.Command { if (paths != null && paths.length > 0) { for (Path path : paths) { try { - boolean supportsPosixPermissions = Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class); + boolean supportsPosixPermissions = Environment.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class); if (supportsPosixPermissions) { PosixFileAttributes attributes = Files.readAttributes(path, PosixFileAttributes.class); permissions.put(path, attributes.permissions()); diff --git a/core/src/main/java/org/elasticsearch/env/ESFileStore.java b/core/src/main/java/org/elasticsearch/env/ESFileStore.java index 8963d7868b1..d74432c591a 100644 --- a/core/src/main/java/org/elasticsearch/env/ESFileStore.java +++ b/core/src/main/java/org/elasticsearch/env/ESFileStore.java @@ -26,10 +26,12 @@ import org.elasticsearch.common.io.PathUtils; import java.io.IOException; import java.nio.file.FileStore; +import java.nio.file.FileSystemException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileAttributeView; import java.nio.file.attribute.FileStoreAttributeView; +import java.util.Arrays; /** * 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 { - FileStore store = Files.getFileStore(path); - + /** + * Files.getFileStore(Path) useless here! Don't complain, just try it yourself. + */ + @SuppressForbidden(reason = "works around the bugs") + static FileStore getMatchingFileStore(Path path, FileStore fileStores[]) throws IOException { 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 { String mount = getMountPointLinux(store); @@ -110,6 +115,57 @@ class ESFileStore extends FileStore { // fall back to crappy one we got from Files.getFileStore 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 public String name() { diff --git a/core/src/main/java/org/elasticsearch/env/Environment.java b/core/src/main/java/org/elasticsearch/env/Environment.java index f50405eb091..a82dab995e5 100644 --- a/core/src/main/java/org/elasticsearch/env/Environment.java +++ b/core/src/main/java/org/elasticsearch/env/Environment.java @@ -19,6 +19,7 @@ package org.elasticsearch.env; +import org.apache.lucene.util.Constants; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.common.SuppressForbidden; import org.elasticsearch.common.io.PathUtils; @@ -302,9 +303,37 @@ public class Environment { *
  • Only requires the security permissions of {@link Files#getFileStore(Path)}, * no permissions to the actual mount point are required. *
  • Exception handling has the same semantics as {@link Files#getFileStore(Path)}. + *
  • Works around https://bugs.openjdk.java.net/browse/JDK-8034057. * */ - public FileStore getFileStore(Path path) throws IOException { + public static FileStore getFileStore(Path path) throws IOException { 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; + } + } } diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java index fbd5b74ee1d..234d719f734 100644 --- a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java +++ b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java @@ -112,7 +112,7 @@ public class PluginManager { Files.createDirectory(environment.pluginsFile()); } - if (!Files.isWritable(environment.pluginsFile())) { + if (!Environment.isWritable(environment.pluginsFile())) { throw new IOException("plugin directory " + environment.pluginsFile() + " is read only"); } @@ -246,7 +246,7 @@ public class PluginManager { } catch (IOException 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. // read should generally be set already, but set it anyway: don't rely on umask... final Set executePerms = new HashSet<>(); diff --git a/dev-tools/src/main/resources/forbidden/all-signatures.txt b/dev-tools/src/main/resources/forbidden/all-signatures.txt index f697b323569..e61d58d4328 100644 --- a/dev-tools/src/main/resources/forbidden/all-signatures.txt +++ b/dev-tools/src/main/resources/forbidden/all-signatures.txt @@ -56,3 +56,6 @@ java.io.ObjectInputStream 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#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