Merge pull request #11069 from rmuir/ban_pathutils

Ban PathUtils.get (for now, until we fix the two remaining issues)

dev-tools/forbidden/all-signatures.txt

@@ -41,3 +41,7 @@ org.apache.lucene.search.PrefixFilter
 
 java.nio.file.Paths @ Use PathUtils.get instead.
 java.nio.file.FileSystems#getDefault() @ use PathUtils.getDefault instead.
+
+@defaultMessage Specify a location for the temp file/directory instead.
+java.nio.file.Files#createTempDirectory(java.lang.String,java.nio.file.attribute.FileAttribute[])
+java.nio.file.Files#createTempFile(java.lang.String,java.lang.String,java.nio.file.attribute.FileAttribute[])

dev-tools/forbidden/core-signatures.txt

@@ -127,3 +127,7 @@ org.apache.lucene.search.TimeLimitingCollector#getGlobalCounter()
 
 @defaultMessage Don't interrupt threads use FutureUtils#cancel(Future<T>) instead
 java.util.concurrent.Future#cancel(boolean)
+
+@defaultMessage Don't try reading from paths that are not configured in Environment, resolve from Environment instead
+org.elasticsearch.common.io.PathUtils#get(java.lang.String, java.lang.String[])
+org.elasticsearch.common.io.PathUtils#get(java.net.URI)

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

@@ -27,7 +27,6 @@ import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.collect.Tuple;
 import org.elasticsearch.common.inject.CreationException;
 import org.elasticsearch.common.inject.spi.Message;
-import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.jna.Kernel32Library;
 import org.elasticsearch.common.jna.Natives;
 import org.elasticsearch.common.lease.Releasables;

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

@@ -19,7 +19,7 @@
 
 package org.elasticsearch.bootstrap;
 
-import org.elasticsearch.common.io.PathUtils;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.env.Environment;
 
 import java.io.*;
@@ -56,7 +56,7 @@ public class Security {
         // TODO: improve test infra so we can reduce permissions where read/write
         // is not really needed...
         Permissions policy = new Permissions();
-        addPath(policy, PathUtils.get(System.getProperty("java.io.tmpdir")), "read,readlink,write,delete");
+        addPath(policy, environment.tmpFile(), "read,readlink,write,delete");
         addPath(policy, environment.homeFile(), "read,readlink,write,delete");
         addPath(policy, environment.configFile(), "read,readlink,write,delete");
         addPath(policy, environment.logsFile(), "read,readlink,write,delete");
@@ -83,6 +83,7 @@ public class Security {
     }
 
     /** Simple checks that everything is ok */
+    @SuppressForbidden(reason = "accesses jvm default tempdir as a self-test")
     public static void selfTest() {
         // check we can manipulate temporary files
         try {

src/main/java/org/elasticsearch/common/io/PathUtils.java

@@ -35,6 +35,7 @@ import java.nio.file.Paths;
  * be changed during tests.
  */
 @SuppressForbidden(reason = "accesses the default filesystem by design")
+// TODO: can we move this to the .env package and make it package-private?
 public final class PathUtils {
     /** no instantiation */
     private PathUtils() {}

src/main/java/org/elasticsearch/env/ESFileStore.java

@@ -21,6 +21,7 @@ package org.elasticsearch.env;
 
 import org.apache.lucene.util.Constants;
 import org.apache.lucene.util.IOUtils;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
 
 import java.io.IOException;
@@ -42,6 +43,9 @@ class ESFileStore extends FileStore {
     /** Cached result of Lucene's {@code IOUtils.spins} on path. */
     final Boolean spins;
     
+    @SuppressForbidden(reason = "tries to determine if disk is spinning")
+    // TODO: move PathUtils to be package-private here instead of 
+    // public+forbidden api!
     ESFileStore(FileStore in) {
         this.in = in;
         Boolean spins;

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

@@ -20,6 +20,7 @@
 package org.elasticsearch.env;
 
 import org.elasticsearch.cluster.ClusterName;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 
@@ -36,6 +37,9 @@ import static org.elasticsearch.common.Strings.cleanPath;
 /**
  * The environment of where things exists.
  */
+@SuppressForbidden(reason = "configures paths for the system")
+// TODO: move PathUtils to be package-private here instead of 
+// public+forbidden api!
 public class Environment {
 
     private final Settings settings;
@@ -54,6 +58,9 @@ public class Environment {
 
     /** Path to the PID file (can be null if no PID file is configured) **/
     private final Path pidFile;
+    
+    /** Path to the temporary file directory used by the JDK */
+    private final Path tmpFile = PathUtils.get(System.getProperty("java.io.tmpdir"));
 
     /** List of filestores on the system */
     private static final FileStore[] fileStores;
@@ -166,6 +173,11 @@ public class Environment {
     public Path pidFile() {
         return pidFile;
     }
+    
+    /** Path to the default temp directory used by the JDK */
+    public Path tmpFile() {
+        return tmpFile;
+    }
 
     /**
      * Looks up the filestore associated with a Path.

src/main/java/org/elasticsearch/env/NodeEnvironment.java

@@ -21,10 +21,12 @@ package org.elasticsearch.env;
 
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
+
 import org.apache.lucene.store.*;
 import org.apache.lucene.util.IOUtils;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.node.DiscoveryNode;
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.component.AbstractComponent;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.io.FileSystemUtils;
@@ -137,8 +139,7 @@ public class NodeEnvironment extends AbstractComponent implements Closeable {
         int maxLocalStorageNodes = settings.getAsInt("node.max_local_storage_nodes", 50);
         for (int possibleLockId = 0; possibleLockId < maxLocalStorageNodes; possibleLockId++) {
             for (int dirIndex = 0; dirIndex < environment.dataWithClusterFiles().length; dirIndex++) {
-                // TODO: wtf with resolve(get())
+                Path dir = environment.dataWithClusterFiles()[dirIndex].resolve(NODES_FOLDER).resolve(Integer.toString(possibleLockId));
-                Path dir = environment.dataWithClusterFiles()[dirIndex].resolve(PathUtils.get(NODES_FOLDER, Integer.toString(possibleLockId)));
                 Files.createDirectories(dir);
                 
                 try (Directory luceneDir = FSDirectory.open(dir, NativeFSLockFactory.INSTANCE)) {
@@ -689,6 +690,7 @@ public class NodeEnvironment extends AbstractComponent implements Closeable {
      *
      * @param indexSettings settings for the index
      */
+    @SuppressForbidden(reason = "Lee is working on it: https://github.com/elastic/elasticsearch/pull/11065")
     private Path resolveCustomLocation(@IndexSettings Settings indexSettings) {
         assert indexSettings != ImmutableSettings.EMPTY;
         String customDataDir = indexSettings.get(IndexMetaData.SETTING_DATA_PATH);
@@ -696,7 +698,7 @@ public class NodeEnvironment extends AbstractComponent implements Closeable {
             // This assert is because this should be caught by MetaDataCreateIndexService
             assert customPathsEnabled;
             if (addNodeId) {
-                return PathUtils.get(customDataDir, Integer.toString(this.localNodeId));
+                return PathUtils.get(customDataDir).resolve(Integer.toString(this.localNodeId));
             } else {
                 return PathUtils.get(customDataDir);
             }

src/main/java/org/elasticsearch/http/HttpServer.java

@@ -21,11 +21,8 @@ package org.elasticsearch.http;
 
 import com.google.common.collect.ImmutableMap;
 
-import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.common.component.AbstractLifecycleComponent;
 import org.elasticsearch.common.inject.Inject;
-import org.elasticsearch.common.io.FileSystemUtils;
-import org.elasticsearch.common.io.PathUtils;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.node.service.NodeService;
@@ -167,16 +164,23 @@ public class HttpServer extends AbstractLifecycleComponent<HttpServer> {
             sitePath = path.substring(i1 + 1);
         }
 
+        // we default to index.html, or what the plugin provides (as a unix-style path)
+        // this is a relative path under _site configured by the plugin.
         if (sitePath.length() == 0) {
-            sitePath = "/index.html";
+            sitePath = "index.html";
+        } else {
+            // remove extraneous leading slashes, its not an absolute path.
+            while (sitePath.length() > 0 && sitePath.charAt(0) == '/') {
+                sitePath = sitePath.substring(1);
+            }
         }
         final Path siteFile = environment.pluginsFile().resolve(pluginName).resolve("_site");
 
         final String separator = siteFile.getFileSystem().getSeparator();
         // Convert file separators.
         sitePath = sitePath.replace("/", separator);
-        // this is a plugin provided site, serve it as static files from the plugin location
+        
-        Path file = FileSystemUtils.append(siteFile, PathUtils.get(sitePath), 0);
+        Path file = siteFile.resolve(sitePath);
 
         // return not found instead of forbidden to prevent malicious requests to find out if files exist or dont exist
         if (!Files.exists(file) || Files.isHidden(file) || !file.toAbsolutePath().normalize().startsWith(siteFile.toAbsolutePath())) {

src/main/java/org/elasticsearch/repositories/fs/FsRepository.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.repositories.fs;
 
+import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.blobstore.BlobPath;
 import org.elasticsearch.common.blobstore.BlobStore;
 import org.elasticsearch.common.blobstore.fs.FsBlobStore;
@@ -67,7 +68,7 @@ public class FsRepository extends BlobStoreRepository {
      * @param indexShardRepository index shard repository
      * @throws IOException
      */
-    @Inject
+    @Inject @SuppressForbidden(reason = "needs fixing: https://github.com/elastic/elasticsearch/issues/11068")
     public FsRepository(RepositoryName name, RepositorySettings repositorySettings, IndexShardRepository indexShardRepository) throws IOException {
         super(name.getName(), repositorySettings, indexShardRepository);
         Path locationFile;

src/test/java/org/elasticsearch/bootstrap/SecurityTests.java

@@ -39,12 +39,12 @@ public class SecurityTests extends ElasticsearchTestCase {
         settingsBuilder.put("path.home", esHome.toString());
         Settings settings = settingsBuilder.build();
 
-        Environment environment = new Environment(settings);
         Path fakeTmpDir = createTempDir();
         String realTmpDir = System.getProperty("java.io.tmpdir");
         Permissions permissions;
         try {
             System.setProperty("java.io.tmpdir", fakeTmpDir.toString());
+            Environment environment = new Environment(settings);
             permissions = Security.createPermissions(environment);
         } finally {
             System.setProperty("java.io.tmpdir", realTmpDir);
@@ -73,12 +73,13 @@ public class SecurityTests extends ElasticsearchTestCase {
         settingsBuilder.put("pidfile", path.resolve("test.pid").toString());
         Settings settings = settingsBuilder.build();
 
-        Environment environment = new Environment(settings);
         Path fakeTmpDir = createTempDir();
         String realTmpDir = System.getProperty("java.io.tmpdir");
         Permissions permissions;
+        Environment environment;
         try {
             System.setProperty("java.io.tmpdir", fakeTmpDir.toString());
+            environment = new Environment(settings);
             permissions = Security.createPermissions(environment);
         } finally {
             System.setProperty("java.io.tmpdir", realTmpDir);