SOLR-14561 Followup - validate params for more core operations (#1629)

Add template to solr.in scripts
Also testes Windows paths
Added RefGuide documentation to some params

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

@@ -117,6 +117,7 @@ public final class SolrPaths {
    * @throws SolrException if path is outside allowed paths
    */
   public static void assertPathAllowed(Path pathToAssert, Set<Path> allowPaths) throws SolrException {
+    if (pathToAssert == null) return;
     if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
           "Path " + pathToAssert + " disallowed. UNC paths not supported. Please use drive letter instead.");

solr/core/src/java/org/apache/solr/handler/ReplicationHandler.java

@@ -324,10 +324,13 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
     }
   }
 
+  @SuppressWarnings("deprecation")
   private void deleteSnapshot(ModifiableSolrParams params, SolrQueryResponse rsp) {
-    String name = params.required().get(NAME);
+    params.required().get(NAME);
 
-    SnapShooter snapShooter = new SnapShooter(core, params.get(CoreAdminParams.BACKUP_LOCATION), params.get(NAME));
+    String location = params.get(CoreAdminParams.BACKUP_LOCATION);
+    core.getCoreContainer().assertPathAllowed(location == null ? null : Path.of(location));
+    SnapShooter snapShooter = new SnapShooter(core, location, params.get(NAME));
     snapShooter.validateDeleteSnapshot();
     snapShooter.deleteSnapAsync(this);
     rsp.add(STATUS, OK_STATUS);
@@ -448,7 +451,6 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
     }
     String name = params.get(NAME);
     String location = params.get(CoreAdminParams.BACKUP_LOCATION);
-
     String repoName = params.get(CoreAdminParams.BACKUP_REPOSITORY);
     CoreContainer cc = core.getCoreContainer();
     BackupRepository repo = null;
@@ -460,12 +462,14 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
       }
     } else {
       repo = new LocalFileSystemRepository();
-    }
-
       //If location is not provided then assume that the restore index is present inside the data directory.
       if (location == null) {
         location = core.getDataDir();
       }
+    }
+    if ("file".equals(repo.createURI("x").getScheme())) {
+      core.getCoreContainer().assertPathAllowed(Paths.get(location));
+    }
 
     URI locationUri = repo.createURI(location);
 
@@ -573,6 +577,9 @@ public class ReplicationHandler extends RequestHandlerBase implements SolrCoreAw
           location = core.getCoreDescriptor().getInstanceDir().resolve(location).normalize().toString();
         }
       }
+      if ("file".equals(repo.createURI("x").getScheme())) {
+        core.getCoreContainer().assertPathAllowed(Paths.get(location));
+      }
 
         // small race here before the commit point is saved
       URI locationUri = repo.createURI(location);

solr/core/src/java/org/apache/solr/handler/SnapShooter.java

@@ -86,6 +86,9 @@ public class SnapShooter {
     this.backupRepo = Objects.requireNonNull(backupRepo);
     this.baseSnapDirPath = location;
     this.snapshotName = snapshotName;
+    if ("file".equals(location.getScheme())) {
+      solrCore.getCoreContainer().assertPathAllowed(Paths.get(location));
+    }
     if (snapshotName != null) {
       directoryName = "snapshot." + snapshotName;
     } else {

solr/core/src/java/org/apache/solr/handler/admin/MergeIndexesOp.java

@@ -18,10 +18,8 @@
 package org.apache.solr.handler.admin;
 
 import java.lang.invoke.MethodHandles;
-import java.util.HashMap;
+import java.nio.file.Paths;
-import java.util.List;
+import java.util.*;
-import java.util.Map;
-import java.util.Set;
 
 import com.google.common.collect.Lists;
 import org.apache.lucene.index.DirectoryReader;
@@ -79,6 +77,8 @@ class MergeIndexesOp implements CoreAdminHandler.CoreAdminOp {
           sourceCores.add(srcCore);
         }
       } else {
+        // Validate each 'indexDir' input as valid
+        Arrays.stream(dirNames).forEach(indexDir -> core.getCoreContainer().assertPathAllowed(Paths.get(indexDir)));
         DirectoryFactory dirFactory = core.getDirectoryFactory();
         for (int i = 0; i < dirNames.length; i++) {
           boolean markAsDone = false;

solr/core/src/test/org/apache/solr/cloud/BasicDistributedZk2Test.java

@@ -404,8 +404,9 @@ public class BasicDistributedZk2Test extends AbstractFullDistribZkTestBase {
       params.set("qt", ReplicationHandler.PATH);
       params.set("command", "backup");
       params.set("name", backupName);
-      Path location = createTempDir();
+      final Path location = FilterPath.unwrap(createTempDir()).toRealPath();
-      location = FilterPath.unwrap(location).toRealPath();
+      // Allow non-standard location outside SOLR_HOME
+      jettys.forEach(j -> j.getCoreContainer().getAllowPaths().add(location));
       params.set("location", location.toString());
 
       QueryRequest request = new QueryRequest(params);

solr/core/src/test/org/apache/solr/cloud/api/collections/AbstractCloudBackupRestoreTestCase.java

@@ -47,6 +47,7 @@ import org.apache.solr.common.cloud.ImplicitDocRouter;
 import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.params.CoreAdminParams;
+import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -72,6 +73,12 @@ public abstract class AbstractCloudBackupRestoreTestCase extends SolrCloudTestCa
   @BeforeClass
   public static void createCluster() throws Exception {
     docsSeed = random().nextLong();
+    System.setProperty("solr.allowPaths", "*");
+  }
+
+  @AfterClass
+  public static void afterClass() throws Exception {
+    System.clearProperty("solr.allowPaths");
   }
 
   /**

solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCloudSnapshots.java

@@ -65,6 +65,7 @@ public class TestSolrCloudSnapshots extends SolrCloudTestCase {
   @BeforeClass
   public static void setupClass() throws Exception {
     useFactory("solr.StandardDirectoryFactory");
+    System.setProperty("solr.allowPaths", "*");
     configureCluster(NUM_NODES)// nodes
         .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
         .configure();
@@ -76,6 +77,7 @@ public class TestSolrCloudSnapshots extends SolrCloudTestCase {
   public static void teardownClass() throws Exception {
     System.clearProperty("test.build.data");
     System.clearProperty("test.cache.data");
+    System.clearProperty("solr.allowPaths");
   }
 
   @Test

solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCoreSnapshots.java

@@ -66,6 +66,7 @@ public class TestSolrCoreSnapshots extends SolrCloudTestCase {
 
   @BeforeClass
   public static void setupClass() throws Exception {
+    System.setProperty("solr.allowPaths", "*");
     useFactory("solr.StandardDirectoryFactory");
     configureCluster(1)// nodes
         .addConfig("conf1", TEST_PATH().resolve("configsets").resolve("cloud-minimal").resolve("conf"))
@@ -77,6 +78,7 @@ public class TestSolrCoreSnapshots extends SolrCloudTestCase {
   public static void teardownClass() throws Exception {
     System.clearProperty("test.build.data");
     System.clearProperty("test.cache.data");
+    System.clearProperty("solr.allowPaths");
   }
 
   @Test

solr/core/src/test/org/apache/solr/handler/TestCoreBackup.java

@@ -16,16 +16,11 @@
  */
 package org.apache.solr.handler;
 
-import java.io.File;
-import java.io.IOException;
-import java.util.Arrays;
-
-import org.apache.lucene.index.IndexCommit;
 import org.apache.lucene.index.DirectoryReader;
+import org.apache.lucene.index.IndexCommit;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.FSDirectory;
 import org.apache.lucene.util.TestUtil;
-
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.core.CoreContainer;
@@ -34,8 +29,12 @@ import org.apache.solr.response.SolrQueryResponse;
 import org.junit.After;
 import org.junit.Before;
 
-public class TestCoreBackup extends SolrTestCaseJ4 {
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
 
+public class TestCoreBackup extends SolrTestCaseJ4 {
   @Before // unique core per test
   public void coreInit() throws Exception {
     initCore("solrconfig.xml", "schema.xml");
@@ -63,6 +62,7 @@ public class TestCoreBackup extends SolrTestCaseJ4 {
     String snapshotName = TestUtil.randomSimpleString(random(), 1, 5);
 
     final CoreContainer cores = h.getCoreContainer();
+    cores.getAllowPaths().add(Paths.get(location));
     try (final CoreAdminHandler admin = new CoreAdminHandler(cores)) {
       SolrQueryResponse resp = new SolrQueryResponse();
       admin.handleRequestBody
@@ -96,6 +96,7 @@ public class TestCoreBackup extends SolrTestCaseJ4 {
     final CoreAdminHandler admin = new CoreAdminHandler(cores);
 
     final File backupDir = createTempDir().toFile();
+    cores.getAllowPaths().add(backupDir.toPath());
 
     { // first a backup before we've ever done *anything*...
       SolrQueryResponse resp = new SolrQueryResponse();
@@ -197,6 +198,7 @@ public class TestCoreBackup extends SolrTestCaseJ4 {
     final CoreAdminHandler admin = new CoreAdminHandler(cores);
     
     final File backupDir = createTempDir().toFile();
+    cores.getAllowPaths().add(backupDir.toPath());
 
 
     { // take an initial 'backup1a' containing our 1 document

solr/core/src/test/org/apache/solr/handler/TestReplicationHandler.java

@@ -1619,6 +1619,8 @@ public class TestReplicationHandler extends SolrTestCaseJ4 {
     final File backupDir = createTempDir().toFile();
     final BackupStatusChecker backupStatus = new BackupStatusChecker(masterClient);
 
+    masterJetty.getCoreContainer().getAllowPaths().add(backupDir.toPath());
+
     { // initial request w/o any committed docs
       final String backupName = "empty_backup1";
       final GenericSolrRequest req = new GenericSolrRequest

solr/core/src/test/org/apache/solr/handler/TestRestoreCore.java

@@ -125,6 +125,7 @@ public class TestRestoreCore extends SolrJettyTestBase {
     //Use the default backup location or an externally provided location.
     if (random().nextBoolean()) {
       location = createTempDir().toFile().getAbsolutePath();
+      masterJetty.getCoreContainer().getAllowPaths().add(Path.of(location)); // Allow core to be created outside SOLR_HOME
       params += "&location=" + URLEncoder.encode(location, "UTF-8");
     }
 
@@ -181,11 +182,21 @@ public class TestRestoreCore extends SolrJettyTestBase {
 
   }
 
+  public void testBackupFailsMissingAllowPaths() throws Exception {
+    final String params = "&location=" + URLEncoder.encode(createTempDir().toFile().getAbsolutePath(), "UTF-8");
+    Throwable t = expectThrows(IOException.class, () -> {
+      TestReplicationHandlerBackup.runBackupCommand(masterJetty, ReplicationHandler.CMD_BACKUP, params);
+    });
+    // The backup command will fail since the tmp dir is outside allowPaths
+    assertTrue(t.getMessage().contains("Server returned HTTP response code: 400"));
+  }
+
   @Test
   public void testFailedRestore() throws Exception {
     int nDocs = BackupRestoreUtils.indexDocs(masterClient, "collection1", docsSeed);
 
     String location = createTempDir().toFile().getAbsolutePath();
+    masterJetty.getCoreContainer().getAllowPaths().add(Path.of(location));
     String snapshotName = TestUtil.randomSimpleString(random(), 1, 5);
     String params = "&name=" + snapshotName + "&location=" + URLEncoder.encode(location, "UTF-8");
     String baseUrl = masterJetty.getBaseUrl().toString();

solr/core/src/test/org/apache/solr/handler/V2ApiIntegrationTest.java

@@ -17,20 +17,10 @@
 
 package org.apache.solr.handler;
 
-
-import java.io.IOException;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 import org.apache.solr.client.solrj.ResponseParser;
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.client.solrj.impl.BaseHttpSolrClient;
+import org.apache.solr.client.solrj.impl.*;
-import org.apache.solr.client.solrj.impl.BinaryResponseParser;
-import org.apache.solr.client.solrj.impl.CloudSolrClient;
-import org.apache.solr.client.solrj.impl.NoOpResponseParser;
-import org.apache.solr.client.solrj.impl.XMLResponseParser;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.V2Request;
 import org.apache.solr.client.solrj.response.DelegationTokenResponse;
@@ -42,6 +32,12 @@ import org.apache.solr.common.util.Utils;
 import org.junit.BeforeClass;
 import org.junit.Test;
 
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 public class V2ApiIntegrationTest extends SolrCloudTestCase {
   private static String COLL_NAME = "collection1";
 
@@ -176,6 +172,7 @@ public class V2ApiIntegrationTest extends SolrCloudTestCase {
     backupParams.put("name", "backup_test");
     backupParams.put("collection", COLL_NAME);
     backupParams.put("location", tempDir);
+    cluster.getJettySolrRunners().forEach(j -> j.getCoreContainer().getAllowPaths().add(Paths.get(tempDir)));
     client.request(new V2Request.Builder("/c")
         .withMethod(SolrRequest.METHOD.POST)
         .withPayload(Utils.toJSONString(backupPayload))

solr/core/src/test/org/apache/solr/handler/admin/CoreMergeIndexesAdminHandlerTest.java

@@ -16,9 +16,7 @@
  */
 package org.apache.solr.handler.admin;
 
-import java.io.File;
+import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
-import java.io.IOException;
-
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.LockFactory;
 import org.apache.solr.SolrTestCaseJ4;
@@ -35,7 +33,8 @@ import org.junit.Test;
 import org.junit.rules.RuleChain;
 import org.junit.rules.TestRule;
 
-import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule;
+import java.io.File;
+import java.io.IOException;
 
 public class CoreMergeIndexesAdminHandlerTest extends SolrTestCaseJ4 {
   
@@ -74,6 +73,7 @@ public class CoreMergeIndexesAdminHandlerTest extends SolrTestCaseJ4 {
     final File workDir = createTempDir().toFile();
 
     final CoreContainer cores = h.getCoreContainer();
+    cores.getAllowPaths().add(workDir.toPath());
 
     try (final CoreAdminHandler admin = new CoreAdminHandler(cores);
          SolrCore core = cores.getCore("collection1")) {

solr/solr-ref-guide/src/index-replication.adoc

@@ -328,7 +328,30 @@ This command is useful for making periodic backups. There are several supported
 +
 * `numberToKeep:`: This can be used with the backup command unless the `maxNumberOfBackups` initialization parameter has been specified on the handler – in which case `maxNumberOfBackups` is always used and attempts to use the `numberToKeep` request parameter will cause an error.
 * `name`: (optional) Backup name. The snapshot will be created in a directory called `snapshot.<name>` within the data directory of the core. By default the name is generated using date in `yyyyMMddHHmmssSSS` format. If `location` parameter is passed, that would be used instead of the data directory
-* `location`: Backup location.
+* `repository`: The name of the backup repository to use. When not specified, it defaults to local file system.
+* `location`: Backup location. Value depends on the repository in use. For file system repository, location defaults to core's dataDir, and if specified, it needs to be within `SOLR_HOME`, `SOLR_DATA_HOME` or the paths specified by solr.xml `allowPaths`.
+
+`restore`::
+Restore a backup from a backup repository.
++
+[source,bash]
+http://_master_host:port_/solr/_core_name_/replication?command=restore
++
+This command is used to restore a backup. There are several supported request parameters:
++
+* `name`: (optional) Backup name. The name of the backed up index snapshot to be restored. If the name is not provided, it looks for backups with snapshot.<timestamp> format in the location directory. It picks the latest timestamp backup in that case.
+* `repository`: The name of the backup repository where the backup resides. When not specified, it defaults to local file system.
+* `location`: Backup location. Value depends on the repository in use. For file system repository, location defaults to core's dataDir, and if specified, it needs to be within `SOLR_HOME`, `SOLR_DATA_HOME` or the paths specified by solr.xml `allowPaths`.
+
+`restorestatus`::
+Check the status of a running restore operation.
++
+[source,bash]
+http://_master_host:port_/solr/_core_name_/replication?command=restorestatus
++
+This command is used to check the status of a restore operation. This command takes no parameters.
++
+The status value can be "In Progress" , "success" or "failed". If it failed then an "exception" will also be sent in the response.
 
 `deletebackup`::
 Delete any backup created using the `backup` command.

solr/solrj/src/test/org/apache/solr/client/solrj/MergeIndexesExampleTestBase.java

@@ -77,7 +77,8 @@ public abstract class MergeIndexesExampleTestBase extends SolrTestCaseJ4 {
     if (log.isInfoEnabled()) {
       log.info("CORES={} : {}", cores, cores.getLoadedCoreNames());
     }
-
+    cores.getAllowPaths().add(dataDir1.toPath());
+    cores.getAllowPaths().add(dataDir2.toPath());
   }
 
   @Override

solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java

@@ -91,6 +91,7 @@ public class MiniSolrCloudCluster {
   public static final String DEFAULT_CLOUD_SOLR_XML = "<solr>\n" +
       "\n" +
       "  <str name=\"shareSchema\">${shareSchema:false}</str>\n" +
+      "  <str name=\"allowPaths\">${solr.allowPaths:}</str>\n" +
       "  <str name=\"configSetBaseDir\">${configSetBaseDir:configsets}</str>\n" +
       "  <str name=\"coreRootDirectory\">${coreRootDirectory:.}</str>\n" +
       "  <str name=\"collectionsHandler\">${collectionsHandler:solr.CollectionsHandler}</str>\n" +