SOLR-18058: Tweak "allowPath" checks to avoid NPEs

Author: gerlowskija

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

@@ -38,7 +38,6 @@
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.text.SimpleDateFormat;
 import java.util.Arrays;
 import java.util.Collections;
@@ -447,6 +446,7 @@ public CoreContainer(NodeConfig config, CoresLocator locator, boolean asyncSolrC
     SolrPaths.AllowPathBuilder allowPathBuilder = new SolrPaths.AllowPathBuilder();
     allowPathBuilder.addPath(cfg.getSolrHome());
     allowPathBuilder.addPath(cfg.getCoreRootDirectory());
+    allowPathBuilder.addPath(cfg.getConfigSetBaseDirectory());
     if (cfg.getSolrDataHome() != null) {
       allowPathBuilder.addPath(cfg.getSolrDataHome());
     }
@@ -1593,6 +1593,10 @@ public SolrCore create(
         log.warn(msg);
         throw new SolrException(ErrorCode.CONFLICT, msg);
       }
+
+      // Validate 'instancePath' prior to instantiating CoreDescriptor, as CD construction attempts
+      // to read properties from 'instancePath'
+      assertPathAllowed(instancePath);
       CoreDescriptor cd =
           new CoreDescriptor(
               coreName, instancePath, parameters, getContainerProperties(), getZkController());
@@ -1607,8 +1611,7 @@ public SolrCore create(
       }
 
       // Validate paths are relative to known locations to avoid path traversal
-      assertPathAllowed(cd.getInstanceDir());
-      assertPathAllowed(Paths.get(cd.getDataDir()));
+      assertPathAllowed(Path.of(cd.getDataDir()));
 
       boolean preExistingZkEntry = false;
       try {
@@ -1679,6 +1682,8 @@ public SolrCore create(
     }
   }
 
+  public static final String ALLOW_PATHS_SYSPROP = "solr.allowPaths";
+
   /**
    * Checks that the given path is relative to SOLR_HOME, SOLR_DATA_HOME, coreRootDirectory or one
    * of the paths specified in solr.xml's allowPaths element. Delegates to {@link

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

@@ -56,15 +56,21 @@ public class FileSystemConfigSetService extends ConfigSetService {
   public static final String METADATA_FILE = ".metadata.json";
 
   private final Path configSetBase;
+  // TODO currently it's not really possible to check paths against allowPaths without a
+  // CoreContainer reference, see SOLR-18059
+  private final CoreContainer cc;
 
   public FileSystemConfigSetService(CoreContainer cc) {
     super(cc.getResourceLoader(), cc.getConfig().hasSchemaCache());
+
+    this.cc = cc;
     this.configSetBase = cc.getConfig().getConfigSetBaseDirectory();
   }
 
   /** Testing purpose */
   protected FileSystemConfigSetService(Path configSetBase) {
     super(null, false);
+    this.cc = null;
     this.configSetBase = configSetBase;
   }
 
@@ -317,6 +323,13 @@ protected Path locateInstanceDir(CoreDescriptor cd) {
     String configSet = cd.getConfigSet();
     if (configSet == null) return cd.getInstanceDir();
     Path configSetDirectory = configSetBase.resolve(configSet);
+
+    // CoreContainer only null in testing scenarios - bit of a hack, but will go away with
+    // SOLR-18059
+    if (cc != null) {
+      cc.assertPathAllowed(configSetDirectory);
+    }
+
     if (!Files.isDirectory(configSetDirectory))
       throw new SolrException(
           SolrException.ErrorCode.SERVER_ERROR,

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

@@ -126,6 +126,10 @@ public AllowPathBuilder addPath(String path) {
      * (not supported as a {@link Path} on Windows), see {@link #addPath(String)}.
      */
     public AllowPathBuilder addPath(Path path) {
+      if (path == null) {
+        return this;
+      }
+
       if (paths != ALL_PATHS) {
         if (path.equals(ALL_PATH)) {
           paths = ALL_PATHS;

solr/core/src/test/org/apache/solr/response/TestPrometheusResponseWriter.java

@@ -16,6 +16,8 @@
  */
 package org.apache.solr.response;
 
+import static org.apache.solr.core.CoreContainer.ALLOW_PATHS_SYSPROP;
+
 import com.codahale.metrics.Counter;
 import com.codahale.metrics.Gauge;
 import com.codahale.metrics.Meter;
@@ -33,6 +35,7 @@
 import org.apache.solr.client.solrj.impl.NoOpResponseParser;
 import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.EnvUtils;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.metrics.SolrMetricManager;
 import org.apache.solr.util.ExternalPaths;
@@ -53,6 +56,7 @@ public class TestPrometheusResponseWriter extends SolrTestCaseJ4 {
   public static void beforeClass() throws Exception {
     SharedMetricRegistries.clear();
 
+    EnvUtils.setProperty(ALLOW_PATHS_SYSPROP, ExternalPaths.SERVER_HOME);
     solrClientTestRule.startSolr(LuceneTestCase.createTempDir());
     solrClientTestRule.newCollection().withConfigSet(ExternalPaths.DEFAULT_CONFIGSET).create();
     var cc = solrClientTestRule.getCoreContainer();

solr/core/src/test/org/apache/solr/search/TestThinCache.java

@@ -16,6 +16,8 @@
  */
 package org.apache.solr.search;
 
+import static org.apache.solr.core.CoreContainer.ALLOW_PATHS_SYSPROP;
+
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
@@ -39,6 +41,9 @@ public class TestThinCache extends SolrTestCaseJ4 {
   @ClassRule public static EmbeddedSolrServerTestRule solrRule = new EmbeddedSolrServerTestRule();
   public static final String SOLR_NODE_LEVEL_CACHE_XML =
       "<solr>\n"
+          + "  <str name=\"allowPaths\">${"
+          + ALLOW_PATHS_SYSPROP
+          + ":}</str>"
           + "  <caches>\n"
           + "    <cache name='myNodeLevelCache'\n"
           + "      size='10'\n"
@@ -55,11 +60,12 @@ public class TestThinCache extends SolrTestCaseJ4 {
   @BeforeClass
   public static void setupSolrHome() throws Exception {
     Path home = createTempDir("home");
+    Path configSet = createTempDir("configSet");
+    System.setProperty(ALLOW_PATHS_SYSPROP, configSet.toAbsolutePath().toString());
     Files.writeString(home.resolve("solr.xml"), SOLR_NODE_LEVEL_CACHE_XML);
 
     solrRule.startSolr(home);
 
-    Path configSet = createTempDir("configSet");
     copyMinConf(configSet.toFile());
     // insert a special filterCache configuration
     Path solrConfig = configSet.resolve("conf/solrconfig.xml");

solr/core/src/test/org/apache/solr/servlet/HideStackTraceTest.java

@@ -16,6 +16,8 @@
  */
 package org.apache.solr.servlet;
 
+import static org.apache.solr.core.CoreContainer.ALLOW_PATHS_SYSPROP;
+
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -26,6 +28,7 @@
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.SolrTestCaseJ4.SuppressSSL;
 import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.common.util.EnvUtils;
 import org.apache.solr.handler.component.ResponseBuilder;
 import org.apache.solr.handler.component.SearchComponent;
 import org.apache.solr.util.SolrJettyTestRule;
@@ -47,6 +50,8 @@ public static void setupSolrHome() throws Exception {
 
     Path configSet = createTempDir("configSet");
     copyMinConf(configSet.toFile());
+    EnvUtils.setProperty(ALLOW_PATHS_SYSPROP, configSet.toAbsolutePath().toString());
+
     // insert a special filterCache configuration
     Path solrConfig = configSet.resolve("conf/solrconfig.xml");
     Files.writeString(

solr/prometheus-exporter/src/test/org/apache/solr/prometheus/scraper/SolrStandaloneScraperBasicAuthTest.java

@@ -71,10 +71,13 @@ public class SolrStandaloneScraperBasicAuthTest extends SolrTestCaseJ4 {
   public static void setupSolrHome() throws Exception {
     Path solrHome = LuceneTestCase.createTempDir();
     Files.write(solrHome.resolve("security.json"), securityJson.getBytes(StandardCharsets.UTF_8));
-    solrRule.startSolr(solrHome);
 
     Path configSet = LuceneTestCase.createTempDir();
     SolrStandaloneScraperTest.createConf(configSet);
+    System.setProperty("solr.allowPaths", configSet.toAbsolutePath().toString());
+
+    solrRule.startSolr(solrHome);
+
     solrRule
         .newCollection()
         .withConfigSet(configSet.toString())

solr/prometheus-exporter/src/test/org/apache/solr/prometheus/scraper/SolrStandaloneScraperTest.java

@@ -54,10 +54,12 @@ public class SolrStandaloneScraperTest extends SolrTestCaseJ4 {
 
   @BeforeClass
   public static void setupBeforeClass() throws Exception {
-    solrRule.startSolr(LuceneTestCase.createTempDir());
-
     Path configSet = LuceneTestCase.createTempDir();
     createConf(configSet);
+    System.setProperty("solr.allowPaths", configSet.toAbsolutePath().toString());
+
+    solrRule.startSolr(LuceneTestCase.createTempDir());
+
     solrRule.newCollection().withConfigSet(configSet.toString()).create();
 
     PrometheusExporterSettings settings = PrometheusExporterSettings.builder().build();

solr/solrj/src/test/org/apache/solr/client/solrj/impl/Http2SolrClientProxyTest.java

@@ -16,6 +16,8 @@
  */
 package org.apache.solr.client.solrj.impl;
 
+import static org.apache.solr.core.CoreContainer.ALLOW_PATHS_SYSPROP;
+
 import com.carrotsearch.randomizedtesting.RandomizedTest;
 import java.util.Arrays;
 import java.util.Objects;
@@ -26,6 +28,7 @@
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.cloud.SocketProxy;
 import org.apache.solr.common.SolrInputDocument;
+import org.apache.solr.common.util.EnvUtils;
 import org.apache.solr.embedded.JettyConfig;
 import org.apache.solr.util.ExternalPaths;
 import org.apache.solr.util.SolrJettyTestRule;
@@ -44,6 +47,8 @@ public class Http2SolrClientProxyTest extends SolrTestCaseJ4 {
   public static void beforeTest() throws Exception {
     RandomizedTest.assumeFalse(sslConfig.isSSLMode());
 
+    EnvUtils.setProperty(
+        ALLOW_PATHS_SYSPROP, ExternalPaths.SERVER_HOME); // Needed for configset location
     solrClientTestRule.enableProxy();
     solrClientTestRule.startSolr(createTempDir(), new Properties(), JettyConfig.builder().build());
     // Actually only need extremely minimal configSet but just use the default

solr/test-framework/src/java/org/apache/solr/util/EmbeddedSolrServerTestRule.java

@@ -19,10 +19,12 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Properties;
+import java.util.Set;
 import org.apache.lucene.tests.util.LuceneTestCase;
 import org.apache.solr.client.solrj.embedded.EmbeddedSolrServer;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.NodeConfig;
+import org.apache.solr.core.SolrPaths;
 import org.apache.solr.core.SolrXmlConfig;
 import org.apache.solr.update.UpdateShardHandlerConfig;
 
@@ -87,6 +89,7 @@ public NodeConfig.NodeConfigBuilder newNodeConfigBuilder(Path solrHome) {
 
     return new NodeConfig.NodeConfigBuilder("testNode", solrHome)
         .setUpdateShardHandlerConfig(UpdateShardHandlerConfig.TEST_DEFAULT)
+        .setAllowPaths(Set.of(SolrPaths.ALL_PATH))
         .setCoreRootDirectory(LuceneTestCase.createTempDir("cores").toString());
   }