diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 816a0318dbb..38f90df50a1 100644 --- a/solr/CHANGES.txt +++ b/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 ================= diff --git a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java index e2a59429ac6..3742e7fc7ca 100644 --- a/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java +++ b/solr/core/src/java/org/apache/solr/core/CorePropertiesLocator.java @@ -120,15 +120,20 @@ public class CorePropertiesLocator implements CoresLocator { public List discover(CoreContainer cc) { logger.info("Looking for core definitions underneath {}", rootDirectory.getAbsolutePath()); List 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 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); diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java b/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java index c9e096fe458..37f11e30e90 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreContainer.java +++ b/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); diff --git a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java b/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java index 60d2456fbcc..d4fb8e49912 100644 --- a/solr/core/src/test/org/apache/solr/core/TestCoreDiscovery.java +++ b/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 = " " + "2 " +