NPE in PluginsService when starting elasticsearch with a wrong user

When starting elasticsearch with a wrong linux user, it could generate a `NullPointerException` when `PluginsService` tries to list available plugins in `./plugins` dir.

To reproduce:

* create a plugins directory with `rwx` rights for root user only
* launch elasticsearch from another account (elasticsearch for example)

It was supposed to be fixed with #4186, but sadly it's not :-(

Closes #5195.

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

@@ -228,6 +228,28 @@ public class FileSystemUtils {
         }
     }
 
+    /**
+     * Check that a directory exists, is a directory and is readable
+     * by the current user
+     */
+    public static boolean isAccessibleDirectory(File directory, ESLogger logger) {
+        assert directory != null && logger != null;
+
+        if (!directory.exists()) {
+            logger.debug("[{}] directory does not exist.", directory.getAbsolutePath());
+            return false;
+        }
+        if (!directory.isDirectory()) {
+            logger.debug("[{}] should be a directory but is not.", directory.getAbsolutePath());
+            return false;
+        }
+        if (!directory.canRead()) {
+            logger.debug("[{}] directory is not readable.", directory.getAbsolutePath());
+            return false;
+        }
+        return true;
+    }
+
     private FileSystemUtils() {
 
     }

src/main/java/org/elasticsearch/plugins/PluginsService.java

@@ -44,6 +44,8 @@ import java.lang.reflect.Method;
 import java.net.URL;
 import java.util.*;
 
+import static org.elasticsearch.common.io.FileSystemUtils.isAccessibleDirectory;
+
 /**
  *
  */
@@ -316,11 +318,8 @@ public class PluginsService extends AbstractComponent {
     }
 
     private void loadPluginsIntoClassLoader() {
-        File pluginsFile = environment.pluginsFile();
+        File pluginsDirectory = environment.pluginsFile();
-        if (!pluginsFile.exists()) {
+        if (!isAccessibleDirectory(pluginsDirectory, logger)) {
-            return;
-        }
-        if (!pluginsFile.isDirectory()) {
             return;
         }
 
@@ -342,40 +341,37 @@ public class PluginsService extends AbstractComponent {
             return;
         }
 
-        File[] pluginsFiles = pluginsFile.listFiles();
+        for (File plugin : pluginsDirectory.listFiles()) {
-        if (pluginsFile != null) {
+            // We check that subdirs are directories and readable
-            for (File pluginFile : pluginsFiles) {
+            if (!isAccessibleDirectory(plugin, logger)) {
-                if (pluginFile.isDirectory()) {
+                continue;
-                    if (logger.isTraceEnabled()) {
+            }
-                        logger.trace("--- adding plugin [" + pluginFile.getAbsolutePath() + "]");
+
-                    }
+            logger.trace("--- adding plugin [{}]", plugin.getAbsolutePath());
-                    try {
+
-                        // add the root
+            try {
-                        addURL.invoke(classLoader, pluginFile.toURI().toURL());
+                // add the root
-                        // gather files to add
+                addURL.invoke(classLoader, plugin.toURI().toURL());
-                        List<File> libFiles = Lists.newArrayList();
+                // gather files to add
-                        if (pluginFile.listFiles() != null) {
+                List<File> libFiles = Lists.newArrayList();
-                            libFiles.addAll(Arrays.asList(pluginFile.listFiles()));
+                if (plugin.listFiles() != null) {
-                        }
+                    libFiles.addAll(Arrays.asList(plugin.listFiles()));
-                        File libLocation = new File(pluginFile, "lib");
+                }
-                        if (libLocation.exists() && libLocation.isDirectory() && libLocation.listFiles() != null) {
+                File libLocation = new File(plugin, "lib");
-                            libFiles.addAll(Arrays.asList(libLocation.listFiles()));
+                if (libLocation.exists() && libLocation.isDirectory() && libLocation.listFiles() != null) {
-                        }
+                    libFiles.addAll(Arrays.asList(libLocation.listFiles()));
-
+                }
-                        // if there are jars in it, add it as well
+
-                        for (File libFile : libFiles) {
+                // if there are jars in it, add it as well
-                            if (!(libFile.getName().endsWith(".jar") || libFile.getName().endsWith(".zip"))) {
+                for (File libFile : libFiles) {
-                                continue;
+                    if (!(libFile.getName().endsWith(".jar") || libFile.getName().endsWith(".zip"))) {
-                            }
+                        continue;
-                            addURL.invoke(classLoader, libFile.toURI().toURL());
+                    }
-                        }
+                    addURL.invoke(classLoader, libFile.toURI().toURL());
-                    } catch (Throwable e) {
+                }
-                        logger.warn("failed to add plugin [" + pluginFile + "]", e);
+            } catch (Throwable e) {
-                    }
+                logger.warn("failed to add plugin [" + plugin + "]", e);
-                }
             }
-        } else {
-            logger.debug("failed to list plugins from {}. Check your right access.", pluginsFile.getAbsolutePath());
         }
     }
 
@@ -398,7 +394,7 @@ public class PluginsService extends AbstractComponent {
 
                     // Is it a site plugin as well? Does it have also an embedded _site structure
                     File siteFile = new File(new File(environment.pluginsFile(), plugin.name()), "_site");
-                    boolean isSite = siteFile.exists() && siteFile.isDirectory();
+                    boolean isSite = isAccessibleDirectory(siteFile, logger);
                     if (logger.isTraceEnabled()) {
                         logger.trace("found a jvm plugin [{}], [{}]{}",
                                 plugin.name(), plugin.description(), isSite ? ": with _site structure" : "");
@@ -441,7 +437,7 @@ public class PluginsService extends AbstractComponent {
         for (File pluginFile : pluginsFile.listFiles()) {
             if (!loadedJvmPlugins.contains(pluginFile.getName())) {
                 File sitePluginDir = new File(pluginFile, "_site");
-                if (sitePluginDir.exists()) {
+                if (isAccessibleDirectory(sitePluginDir, logger)) {
                     // We have a _site plugin. Let's try to get more information on it
                     String name = pluginFile.getName();
                     String version = PluginInfo.VERSION_NOT_AVAILABLE;
@@ -491,7 +487,7 @@ public class PluginsService extends AbstractComponent {
         }
 
         File sitePluginDir = new File(pluginsFile, name + "/_site");
-        return sitePluginDir.exists();
+        return isAccessibleDirectory(sitePluginDir, logger);
     }
 
     private Plugin loadPlugin(String className, Settings settings) {