SOLR-5322: core discovery can fail w/NPE and no explanation if a non-readable directory exists

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1622745 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -199,6 +199,9 @@ Other Changes
 * SOLR-6073: Remove helper methods from CollectionsRequest (SolrJ) for CollectionsAPI
   calls and move to a builder design for the same. (Varun Thacker, Anshum Gupta)
 
+* SOLR-5322: core discovery can fail w/NPE and no explanation if a non-readable directory exists
+  (Said Chavkin, Erick Erickson)
+
 
 ==================  4.10.0 =================
 

solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java

@@ -120,15 +120,20 @@ public class CorePropertiesLocator implements CoresLocator {
   public List<CoreDescriptor> discover(CoreContainer cc) {
     logger.info("Looking for core definitions underneath {}", rootDirectory.getAbsolutePath());
     List<CoreDescriptor> cds = Lists.newArrayList();
+    if (rootDirectory.canRead() == false) {
+      throw new RuntimeException("Solr home '" + rootDirectory.getAbsolutePath() + "' doesn't have read permissions");
+    }
     discoverUnder(rootDirectory, cds, cc);
     logger.info("Found {} core definitions", cds.size());
     return cds;
   }
 
   private void discoverUnder(File root, List<CoreDescriptor> cds, CoreContainer cc) {
-    if (!root.exists())
-      return;
     for (File child : root.listFiles()) {
+      if (child.canRead() == false) {
+        logger.warn("Cannot read directory or file during core discovery '" +  child.getAbsolutePath() + "' during core discovery. Skipping");
+        continue;
+      }
       File propertiesFile = new File(child, PROPERTIES_FILENAME);
       if (propertiesFile.exists()) {
         CoreDescriptor cd = buildCoreDescriptor(propertiesFile, cc);

solr/core/src/test/org/apache/solr/core/TestCoreContainer.java

@@ -350,7 +350,9 @@ public class TestCoreContainer extends SolrTestCaseJ4 {
   @Test
   public void testCustomHandlers() throws Exception {
 
-    SolrResourceLoader loader = new SolrResourceLoader("solr/collection1");
+    solrHomeDirectory = createTempDir("_customHandlers");
+    SolrResourceLoader loader = new SolrResourceLoader(solrHomeDirectory.getAbsolutePath());
+
     ConfigSolr config = ConfigSolr.fromString(loader, CUSTOM_HANDLERS_SOLR_XML);
 
     CoreContainer cc = new CoreContainer(loader, config);

solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java

@@ -21,6 +21,7 @@ import java.io.File;
 import java.io.FileOutputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.nio.file.Files;
 import java.util.Properties;
 
 import org.apache.commons.io.FileUtils;
@@ -227,6 +228,122 @@ public class TestCoreDiscovery extends SolrTestCaseJ4 {
       cc.shutdown();
     }
   }
+
+  @Test
+  public void testCoreDirCantRead() throws Exception {
+    File coreDir = solrHomeDirectory;
+    setMeUp(coreDir.getAbsolutePath());
+    addCoreWithProps(makeCorePropFile("core1", false, true),
+        new File(coreDir, "core1" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
+
+    // Insure that another core is opened successfully
+    addCoreWithProps(makeCorePropFile("core2", false, false, "dataDir=core2"),
+        new File(coreDir, "core2" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
+
+    File toSet = new File(coreDir, "core1");
+    toSet.setReadable(false, false);
+    CoreContainer cc = init();
+    try (SolrCore core1 = cc.getCore("core1");
+         SolrCore core2 = cc.getCore("core2")) {
+      assertNull(core1);
+      assertNotNull(core2);
+    } finally {
+      cc.shutdown();
+    }
+    // So things can be cleaned up by the framework!
+    toSet.setReadable(true, false);
+  }
+
+  @Test
+  public void testNonCoreDirCantRead() throws Exception {
+    File coreDir = solrHomeDirectory;
+    setMeUp(coreDir.getAbsolutePath());
+    addCoreWithProps(makeCorePropFile("core1", false, true),
+        new File(coreDir, "core1" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
+
+    addCoreWithProps(makeCorePropFile("core2", false, false, "dataDir=core2"),
+        new File(coreDir, "core2" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
+
+    File toSet = new File(solrHomeDirectory, "cantReadDir");
+    assertTrue("Should have been able to make directory '" + toSet.getAbsolutePath() + "' ", toSet.mkdirs());
+    toSet.setReadable(false, false);
+    CoreContainer cc = init();
+    try (SolrCore core1 = cc.getCore("core1");
+         SolrCore core2 = cc.getCore("core2")) {
+      assertNotNull(core1); // Should be able to open the perfectly valid core1 despite a non-readable directory
+      assertNotNull(core2);
+    } finally {
+      cc.shutdown();
+    }
+    // So things can be cleaned up by the framework!
+    toSet.setReadable(true, false);
+
+  }
+
+  @Test
+  public void testFileCantRead() throws Exception {
+    File coreDir = solrHomeDirectory;
+    setMeUp(coreDir.getAbsolutePath());
+    addCoreWithProps(makeCorePropFile("core1", false, true),
+        new File(coreDir, "core1" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
+
+    File toSet = new File(solrHomeDirectory, "cantReadFile");
+    assertTrue("Should have been able to make file '" + toSet.getAbsolutePath() + "' ", toSet.createNewFile());
+    toSet.setReadable(false, false);
+    CoreContainer cc = init();
+    try (SolrCore core1 = cc.getCore("core1")) {
+      assertNotNull(core1); // Should still be able to create core despite r/o file.
+    } finally {
+      cc.shutdown();
+    }
+    // So things can be cleaned up by the framework!
+    toSet.setReadable(true, false);
+  }
+
+  @Test
+  public void testSolrHomeDoesntExist() throws Exception {
+    File homeDir = solrHomeDirectory;
+    Files.delete(homeDir.toPath());
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (SolrException ex) {
+      assertTrue("Core init doesn't report if solr home directory doesn't exist " + ex.getMessage(),
+          0 <= ex.getMessage().indexOf("solr.xml does not exist"));
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+  }
+
+
+  @Test
+  public void testSolrHomeNotReadable() throws Exception {
+    File homeDir = solrHomeDirectory;
+    setMeUp(homeDir.getAbsolutePath());
+    addCoreWithProps(makeCorePropFile("core1", false, true),
+        new File(homeDir, "core1" + File.separator + CorePropertiesLocator.PROPERTIES_FILENAME));
+
+    homeDir.setReadable(false, false);
+
+    CoreContainer cc = null;
+    try {
+      cc = init();
+    } catch (Exception ex) {
+      String eoe = ex.getMessage();
+
+      assertTrue("Should have had a runtime exception here",
+          0 < ex.getMessage().indexOf("doesn't have read permissions"));
+    } finally {
+      if (cc != null) {
+        cc.shutdown();
+      }
+    }
+    // So things can be cleaned up by the framework!
+    homeDir.setReadable(true, false);
+
+  }
   // For testing whether finding a solr.xml overrides looking at solr.properties
   private final static String SOLR_XML = "<solr> " +
       "<int name=\"transientCacheSize\">2</int> " +