Add File.java to forbidden APIs

This commit cuts over all of core (not quite all tests) to java.nio.Path
It also adds the file class to the core forbidden APIs to prevent its usage.

This commit also resolves #8254 since we now consistently useing the NIO Path
API. The Changes in this commit allow for more information if IO operations fail
since the NIO API throws exceptions instead of boolean return values. The build-in
methods used in this commit are also more resillient to encodeing errors like
unmappable characters and throw exceptions if those chars are present in a file.

Closes #8254
Closes #8666

Author: s1monw

dev-tools/forbidden/core-signatures.txt

@@ -114,3 +114,17 @@ org.apache.lucene.search.TimeLimitingCollector#getGlobalCounter()
 
 @defaultMessage Don't interrupt threads use FutureUtils#cancel(Future<T>) instead
 java.util.concurrent.Future#cancel(boolean)
+
+@defaultMessage Use java.nio.file.Path / java.nio.file.Files instead of java.io.File API
+java.util.jar.JarFile
+java.util.zip.ZipFile
+java.io.File
+java.io.FileInputStream
+java.io.FileOutputStream
+java.io.PrintStream#<init>(java.lang.String,java.lang.String)
+java.io.PrintWriter#<init>(java.lang.String,java.lang.String)
+java.util.Formatter#<init>(java.lang.String,java.lang.String,java.util.Locale)
+java.io.RandomAccessFile
+java.nio.file.Path#toFile()
+
+

pom.xml

@@ -1218,7 +1218,7 @@
                                 <exclude>org/elasticsearch/common/logging/log4j/ConsoleAppender*</exclude>
                                 <exclude>org/elasticsearch/common/http/client/HttpDownloadHelper*</exclude>
                                 <exclude>org/elasticsearch/common/cli/Terminal*</exclude>
-                                <exclude>org/elasticsearch/plugins/PluginManager.class</exclude>
+                                <exclude>org/elasticsearch/plugins/PluginManager$SysOut.class</exclude>
                                 <exclude>org/elasticsearch/common/http/client/HttpDownloadHelper.class</exclude>
                                 <exclude>org/elasticsearch/bootstrap/Bootstrap.class</exclude>
                                 <exclude>org/elasticsearch/Version.class</exclude>

src/main/java/org/elasticsearch/action/admin/cluster/snapshots/status/TransportSnapshotsStatusAction.java

@@ -42,6 +42,7 @@
 import org.elasticsearch.threadpool.ThreadPool;
 import org.elasticsearch.transport.TransportService;
 
+import java.io.IOException;
 import java.util.Map;
 import java.util.Set;
 
@@ -86,7 +87,7 @@ protected SnapshotsStatusResponse newResponse() {
     @Override
     protected void masterOperation(final SnapshotsStatusRequest request,
                                    final ClusterState state,
-                                   final ActionListener<SnapshotsStatusResponse> listener) throws ElasticsearchException {
+                                   final ActionListener<SnapshotsStatusResponse> listener) throws Exception {
         ImmutableList<SnapshotMetaData.Entry> currentSnapshots = snapshotsService.currentSnapshots(request.repository(), request.snapshots());
 
         if (currentSnapshots.isEmpty()) {
@@ -136,7 +137,7 @@ public void onFailure(Throwable e) {
     }
 
     private SnapshotsStatusResponse buildResponse(SnapshotsStatusRequest request, ImmutableList<SnapshotMetaData.Entry> currentSnapshots,
-                                                  TransportNodesSnapshotsStatus.NodesSnapshotStatus nodeSnapshotStatuses) {
+                                                  TransportNodesSnapshotsStatus.NodesSnapshotStatus nodeSnapshotStatuses) throws IOException {
         // First process snapshot that are currently processed
         ImmutableList.Builder<SnapshotStatus> builder = ImmutableList.builder();
         Set<SnapshotId> currentSnapshotIds = newHashSet();

src/main/java/org/elasticsearch/action/support/master/TransportMasterNodeOperationAction.java

@@ -22,6 +22,7 @@
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.ActionResponse;
+import org.elasticsearch.action.ActionRunnable;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.TransportAction;
 import org.elasticsearch.cluster.ClusterChangedEvent;
@@ -32,6 +33,7 @@
 import org.elasticsearch.cluster.node.DiscoveryNodes;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
+import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.discovery.MasterNotDiscoveredException;
 import org.elasticsearch.node.NodeClosedException;
 import org.elasticsearch.threadpool.ThreadPool;
@@ -64,7 +66,7 @@ protected TransportMasterNodeOperationAction(Settings settings, String actionNam
 
     protected abstract Response newResponse();
 
-    protected abstract void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) throws ElasticsearchException;
+    protected abstract void masterOperation(Request request, ClusterState state, ActionListener<Response> listener) throws Exception;
 
     protected boolean localExecute(Request request) {
         return false;
@@ -126,20 +128,12 @@ protected boolean validate(ClusterState newState) {
                 );
 
             } else {
-                try {
-                    threadPool.executor(executor).execute(new Runnable() {
-                        @Override
-                        public void run() {
-                            try {
-                                masterOperation(request, clusterService.state(), listener);
-                            } catch (Throwable e) {
-                                listener.onFailure(e);
-                            }
-                        }
-                    });
-                } catch (Throwable t) {
-                    listener.onFailure(t);
-                }
+                threadPool.executor(executor).execute(new ActionRunnable(listener) {
+                    @Override
+                    protected void doRun() throws Exception {
+                        masterOperation(request, clusterService.state(), listener);
+                    }
+                });
             }
         } else {
             if (nodes.masterNode() == null) {

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

@@ -40,6 +40,8 @@
 
 import java.io.File;
 import java.io.FileOutputStream;
+import java.io.OutputStream;
+import java.nio.file.*;
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
@@ -149,15 +151,12 @@ public static void main(String[] args) {
 
         if (pidFile != null) {
             try {
-                File fPidFile = new File(pidFile);
-                if (fPidFile.getParentFile() != null) {
-                    FileSystemUtils.mkdirs(fPidFile.getParentFile());
-                }
-                FileOutputStream outputStream = new FileOutputStream(fPidFile);
+                Path fPidFile = Paths.get(pidFile);
+                Files.createDirectories(fPidFile.getParent());
+                OutputStream outputStream = Files.newOutputStream(fPidFile, StandardOpenOption.DELETE_ON_CLOSE);
                 outputStream.write(Long.toString(JvmInfo.jvmInfo().pid()).getBytes(Charsets.UTF_8));
-                outputStream.close();
-
-                fPidFile.deleteOnExit();
+                outputStream.flush(); // make those bytes visible...
+                // don't close this stream we will delete on JVM exit
             } catch (Exception e) {
                 String errorMessage = buildErrorMessage("pid", e);
                 System.err.println(errorMessage);

src/main/java/org/elasticsearch/cluster/metadata/MetaDataCreateIndexService.java

@@ -70,10 +70,13 @@
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.threadpool.ThreadPool;
 
-import java.io.File;
-import java.io.FileInputStream;
+import java.io.BufferedReader;
+import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.UnsupportedEncodingException;
+import java.nio.file.DirectoryStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.*;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
@@ -303,17 +306,17 @@ public ClusterState execute(ClusterState currentState) throws Exception {
                     }
 
                     // now add config level mappings
-                    File mappingsDir = new File(environment.configFile(), "mappings");
-                    if (mappingsDir.exists() && mappingsDir.isDirectory()) {
+                    Path mappingsDir = environment.configFile().resolve("mappings");
+                    if (Files.isDirectory(mappingsDir)) {
                         // first index level
-                        File indexMappingsDir = new File(mappingsDir, request.index());
-                        if (indexMappingsDir.exists() && indexMappingsDir.isDirectory()) {
+                        Path indexMappingsDir = mappingsDir.resolve(request.index());
+                        if (Files.isDirectory(indexMappingsDir)) {
                             addMappings(mappings, indexMappingsDir);
                         }
 
                         // second is the _default mapping
-                        File defaultMappingsDir = new File(mappingsDir, "_default");
-                        if (defaultMappingsDir.exists() && defaultMappingsDir.isDirectory()) {
+                        Path defaultMappingsDir = mappingsDir.resolve("_default");
+                        if (Files.isDirectory(defaultMappingsDir)) {
                             addMappings(mappings, defaultMappingsDir);
                         }
                     }
@@ -485,28 +488,30 @@ private Map<String, Object> parseMapping(String mappingSource) throws Exception
         return XContentFactory.xContent(mappingSource).createParser(mappingSource).mapAndClose();
     }
 
-    private void addMappings(Map<String, Map<String, Object>> mappings, File mappingsDir) {
-        File[] mappingsFiles = mappingsDir.listFiles();
-        for (File mappingFile : mappingsFiles) {
-            if (mappingFile.isHidden()) {
-                continue;
-            }
-            int lastDotIndex = mappingFile.getName().lastIndexOf('.');
-            String mappingType = lastDotIndex != -1 ? mappingFile.getName().substring(0, lastDotIndex) : mappingFile.getName();
-            try {
-                String mappingSource = Streams.copyToString(new InputStreamReader(new FileInputStream(mappingFile), Charsets.UTF_8));
-                if (mappings.containsKey(mappingType)) {
-                    XContentHelper.mergeDefaults(mappings.get(mappingType), parseMapping(mappingSource));
-                } else {
-                    mappings.put(mappingType, parseMapping(mappingSource));
+    private void addMappings(Map<String, Map<String, Object>> mappings, Path mappingsDir) throws IOException {
+        try (DirectoryStream<Path> stream = Files.newDirectoryStream(mappingsDir)) {
+            for (Path mappingFile : stream) {
+                final String fileName = mappingFile.getFileName().toString();
+                if (Files.isHidden(mappingFile)) {
+                    continue;
+                }
+                int lastDotIndex = fileName.lastIndexOf('.');
+                String mappingType = lastDotIndex != -1 ? mappingFile.getFileName().toString().substring(0, lastDotIndex) : mappingFile.getFileName().toString();
+                try (BufferedReader reader = Files.newBufferedReader(mappingFile, Charsets.UTF_8)) {
+                    String mappingSource = Streams.copyToString(reader);
+                    if (mappings.containsKey(mappingType)) {
+                        XContentHelper.mergeDefaults(mappings.get(mappingType), parseMapping(mappingSource));
+                    } else {
+                        mappings.put(mappingType, parseMapping(mappingSource));
+                    }
+                } catch (Exception e) {
+                    logger.warn("failed to read / parse mapping [" + mappingType + "] from location [" + mappingFile + "], ignoring...", e);
                 }
-            } catch (Exception e) {
-                logger.warn("failed to read / parse mapping [" + mappingType + "] from location [" + mappingFile + "], ignoring...", e);
             }
         }
     }
 
-    private List<IndexTemplateMetaData> findTemplates(CreateIndexClusterStateUpdateRequest request, ClusterState state, IndexTemplateFilter indexTemplateFilter) {
+    private List<IndexTemplateMetaData> findTemplates(CreateIndexClusterStateUpdateRequest request, ClusterState state, IndexTemplateFilter indexTemplateFilter) throws IOException {
         List<IndexTemplateMetaData> templates = Lists.newArrayList();
         for (ObjectCursor<IndexTemplateMetaData> cursor : state.metaData().templates().values()) {
             IndexTemplateMetaData template = cursor.value;
@@ -516,22 +521,21 @@ private List<IndexTemplateMetaData> findTemplates(CreateIndexClusterStateUpdateR
         }
 
         // see if we have templates defined under config
-        File templatesDir = new File(environment.configFile(), "templates");
-        if (templatesDir.exists() && templatesDir.isDirectory()) {
-            File[] templatesFiles = templatesDir.listFiles();
-            if (templatesFiles != null) {
-                for (File templatesFile : templatesFiles) {
-                    if (templatesFile.isFile()) {
+        final Path templatesDir = environment.configFile().resolve("templates");
+        if (Files.exists(templatesDir) && Files.isDirectory(templatesDir)) {
+            try (DirectoryStream<Path> stream = Files.newDirectoryStream(templatesDir)) {
+                for (Path templatesFile : stream) {
+                    if (Files.isRegularFile(templatesFile)) {
                         XContentParser parser = null;
                         try {
-                            byte[] templatesData = Streams.copyToByteArray(templatesFile);
+                            final byte[] templatesData = Files.readAllBytes(templatesFile);
                             parser = XContentHelper.createParser(templatesData, 0, templatesData.length);
-                            IndexTemplateMetaData template = IndexTemplateMetaData.Builder.fromXContent(parser, templatesFile.getName());
+                            IndexTemplateMetaData template = IndexTemplateMetaData.Builder.fromXContent(parser, templatesFile.getFileName().toString());
                             if (indexTemplateFilter.apply(request, template)) {
                                 templates.add(template);
                             }
                         } catch (Exception e) {
-                            logger.warn("[{}] failed to read template [{}] from config", e, request.index(), templatesFile.getAbsolutePath());
+                            logger.warn("[{}] failed to read template [{}] from config", e, request.index(), templatesFile.toAbsolutePath());
                         } finally {
                             Releasables.closeWhileHandlingException(parser);
                         }