Revert "Use a private directory for temporary files"

This reverts commit fdbc53b.

Author: David Roberts

core/src/main/java/org/elasticsearch/bootstrap/Bootstrap.java

@@ -56,15 +56,11 @@
 import java.io.PrintStream;
 import java.io.UnsupportedEncodingException;
 import java.net.URISyntaxException;
-import java.nio.file.Files;
 import java.nio.file.Path;
 import java.security.NoSuchAlgorithmException;
-import java.util.Comparator;
 import java.util.List;
 import java.util.Collections;
-import java.util.Set;
 import java.util.concurrent.CountDownLatch;
-import java.util.stream.Collectors;
 
 /**
  * Internal startup code.
@@ -189,7 +185,6 @@ public void run() {
                         IOUtils.close(node, spawner);
                         LoggerContext context = (LoggerContext) LogManager.getContext(false);
                         Configurator.shutdown(context);
-                        cleanTmpFiles(environment.tmpFile());
                     } catch (IOException ex) {
                         throw new ElasticsearchException("failed to stop node", ex);
                     }
@@ -267,17 +262,9 @@ private void start() throws NodeValidationException {
         keepAliveThread.start();
     }
 
-    /**
-     * This is used for graceful shutdown on Windows when triggered by
-     * CTRL-C or the service control manager.
-     */
     static void stop() throws IOException {
         try {
             IOUtils.close(INSTANCE.node, INSTANCE.spawner);
-            // TODO: should the logger be shut down here like it is for the shutdown hook?
-            if (INSTANCE.node != null) {
-                cleanTmpFiles(INSTANCE.node.getEnvironment().tmpFile());
-            }
         } finally {
             INSTANCE.keepAliveLatch.countDown();
         }
@@ -408,10 +395,4 @@ private static void checkLucene() {
         }
     }
 
-    static void cleanTmpFiles(Path tmpFile) throws IOException {
-        List<Path> toDelete = Files.walk(tmpFile).sorted(Comparator.reverseOrder()).collect(Collectors.toList());
-        for (Path path : toDelete) {
-            Files.delete(path);
-        }
-    }
 }

core/src/main/java/org/elasticsearch/bootstrap/Security.java

@@ -276,7 +276,7 @@ static void addFilePermissions(Permissions policy, Environment environment) thro
         addDirectoryPath(policy, Environment.PATH_HOME_SETTING.getKey(), environment.pluginsFile(), "read,readlink");
         addDirectoryPath(policy, "path.conf'", environment.configFile(), "read,readlink");
         // read-write dirs
-        addDirectoryPath(policy, "java.io.tmpdir", environment.systemTmpFile(), "read,readlink,write,delete");
+        addDirectoryPath(policy, "java.io.tmpdir", environment.tmpFile(), "read,readlink,write,delete");
         addDirectoryPath(policy, Environment.PATH_LOGS_SETTING.getKey(), environment.logsFile(), "read,readlink,write,delete");
         if (environment.sharedDataFile() != null) {
             addDirectoryPath(policy, Environment.PATH_SHARED_DATA_SETTING.getKey(), environment.sharedDataFile(),

core/src/main/java/org/elasticsearch/env/Environment.java

@@ -25,7 +25,6 @@
 import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Setting.Property;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.set.Sets;
 
 import java.io.IOException;
 import java.net.MalformedURLException;
@@ -34,12 +33,9 @@
 import java.nio.file.FileStore;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
-import java.util.Set;
 import java.util.function.Function;
 
 /**
@@ -87,38 +83,7 @@ public class Environment {
     private final Path pidFile;
 
     /** Path to the temporary file directory used by the JDK */
-    private static final Path systemTmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
-
-    /**
-     * Path to the temporary file directory used by Elasticsearch.
-     * This will be a sub-directory that is created under the JDK temporary directory.
-     */
-    private static final Path tmpFile;
-
-    /**
-     * Creates a secure temporary directory whose contents can only be seen by the user
-     * that Elasticsearch is running as.  This is static so that only one such directory
-     * is created, even when many instances of {@link Environment} are created.
-     */
-    static {
-        Path createdTmpFile;
-        try {
-            try {
-                // On POSIX file systems everyone can see the contents of the /tmp directory, so create a
-                // sub-directory under it that only the user running Elasticsearch can list the contents of.
-                Set<PosixFilePermission> attrs = Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE,
-                    PosixFilePermission.OWNER_EXECUTE);
-                createdTmpFile = Files.createTempDirectory(systemTmpFile, "elasticsearch", PosixFilePermissions.asFileAttribute(attrs));
-            } catch (UnsupportedOperationException e) {
-                // Assume this isn't a POSIX file system.  On Windows each user's %TEMP% directory is visible only to
-                // them and administrators, so the exact permissions of this sub-directory are less important.
-                createdTmpFile = Files.createTempDirectory(systemTmpFile, "elasticsearch");
-            }
-        } catch (IOException e) {
-            throw new IllegalStateException("Unable to create secure temp directory", e);
-        }
-        tmpFile = createdTmpFile;
-    }
+    private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
 
     public Environment(Settings settings) {
         this(settings, null);
@@ -324,14 +289,6 @@ public Path pidFile() {
     }
 
     /** Path to the default temp directory used by the JDK */
-    public Path systemTmpFile() {
-        return systemTmpFile;
-    }
-
-    /**
-     * Path to the temporary file directory used by Elasticsearch.
-     * This will be a sub-directory that is created under the JDK temporary directory.
-     */
     public Path tmpFile() {
         return tmpFile;
     }
@@ -360,7 +317,4 @@ public static void assertEquivalent(Environment actual, Environment expected) {
     private static void assertEquals(Object actual, Object expected, String name) {
         assert Objects.deepEquals(actual, expected) : "actual " + name + " [" + actual + "] is different than [ " + expected + "]";
     }
-
-    // does nothing, just easy way to make sure the class is loaded.
-    public static void ensureClassLoaded() {}
 }

core/src/test/java/org/elasticsearch/bootstrap/BootstrapTests.java

@@ -24,7 +24,6 @@
 import org.elasticsearch.common.settings.SecureSettings;
 import org.elasticsearch.common.settings.SecureString;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.util.set.Sets;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
 import org.junit.After;
@@ -34,15 +33,12 @@
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.attribute.PosixFilePermission;
-import java.nio.file.attribute.PosixFilePermissions;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Set;
 
 public class BootstrapTests extends ESTestCase {
-    private Environment env;
-    private List<FileSystem> fileSystems = new ArrayList<>();
+    Environment env;
+    List<FileSystem> fileSystems = new ArrayList<>();
 
     @After
     public void closeMockFileSystems() throws IOException {
@@ -70,24 +66,4 @@ public void testLoadSecureSettings() throws Exception {
             assertTrue(Files.exists(configPath.resolve("elasticsearch.keystore")));
         }
     }
-
-    public void testCleanTmpFiles() throws IOException {
-        Path testBaseDir = createTempDir();
-        Path createdTmpFile;
-        try {
-            Set<PosixFilePermission> attrs = Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE,
-                PosixFilePermission.OWNER_EXECUTE);
-            createdTmpFile = Files.createTempDirectory(testBaseDir, "BootstrapTests", PosixFilePermissions.asFileAttribute(attrs));
-        } catch (UnsupportedOperationException e) {
-            // Assume this isn't a POSIX file system
-            createdTmpFile = Files.createTempDirectory(testBaseDir, "BootstrapTests");
-        }
-        assertTrue(Files.isDirectory(createdTmpFile));
-        Files.createTempFile(createdTmpFile, "some", "junk");
-        Files.createTempFile(createdTmpFile, "more", "junk");
-        Path nested = Files.createTempDirectory(createdTmpFile, "nested");
-        Files.createTempFile(nested, "nested", "junk");
-        Bootstrap.cleanTmpFiles(createdTmpFile);
-        assertFalse(Files.exists(createdTmpFile));
-    }
 }

qa/evil-tests/src/test/java/org/elasticsearch/env/EnvironmentEvilTests.java

@@ -28,10 +28,10 @@
 import org.elasticsearch.common.io.FileSystemUtils;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.network.IfConfig;
-import org.elasticsearch.env.Environment;
 import org.elasticsearch.plugins.PluginInfo;
 import org.junit.Assert;
 
+import java.io.FilePermission;
 import java.io.InputStream;
 import java.net.SocketPermission;
 import java.net.URL;
@@ -72,9 +72,6 @@ public class BootstrapForTesting {
                                                                "please set ${java.io.tmpdir} in pom.xml"));
         try {
             Security.ensureDirectoryExists(javaTmpDir);
-            // This ensures that the secure temp directory under java.io.tmpdir
-            // exists before we install any mock file systems
-            Environment.ensureClassLoaded();
         } catch (Exception e) {
             throw new RuntimeException("unable to create test temp directory", e);
         }