This is an automated email from the ASF dual-hosted git repository.

janhoy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new ad5ad2c  Revert "SOLR-15950 Create SOLR_HOME/filestore lazily on first 
use, do not create userfiles folder (#593)"
ad5ad2c is described below

commit ad5ad2cc7c87e0f559f129c829bd56dbe7892cb1
Author: Jan Høydahl <[email protected]>
AuthorDate: Sat Feb 19 00:08:24 2022 +0100

    Revert "SOLR-15950 Create SOLR_HOME/filestore lazily on first use, do not 
create userfiles folder (#593)"
    
    This reverts commit 6b0e1bc9
---
 solr/CHANGES.txt                                   |  2 --
 .../java/org/apache/solr/core/CoreContainer.java   | 18 ++++++++++++-
 .../src/java/org/apache/solr/core/SolrPaths.java   | 30 ++++++----------------
 .../apache/solr/filestore/DistribPackageStore.java | 27 +++++--------------
 .../modules/query-guide/pages/loading.adoc         |  2 +-
 .../query-guide/pages/stream-source-reference.adoc |  2 +-
 .../pages/major-changes-in-solr-9.adoc             |  2 --
 .../solrj/io/stream/StreamExpressionTest.java      |  1 +
 8 files changed, 34 insertions(+), 50 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 780575c..87f727b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -581,8 +581,6 @@ and each individual module's jar will be included in its 
directory's lib/ folder
 
 * SOLR-15936: Reduce unnecessary startup logging, such as SSL warnings when 
SSL not in use (janhoy)
 
-* SOLR-15950: Create SOLR_HOME/filestore lazily on first use. Do not 
automatically create SOLR_HOME/userfiles (janhoy)
-
 * SOLR-15777: Forbid useDocValuesAsStored for ICUCollationField (warn for 
luceneMatchVersion < 9.0.0). (Michael Gibney)
 
 * SOLR-12901: Highlighting: hl.method=unified is the new default. (David 
Smiley)
diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java 
b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
index 152ce1f..99abee8 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -118,6 +118,7 @@ import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.security.spec.InvalidKeySpecException;
@@ -828,6 +829,9 @@ public class CoreContainer {
       metricManager.loadClusterReporters(metricReporters, this);
     }
 
+    // Do this before cores get created
+    createUserFilesDirectory();
+
     // setup executor to load cores in parallel
     ExecutorService coreLoadExecutor = MetricUtils.instrumentedExecutorService(
         ExecutorUtil.newMDCAwareFixedThreadPool(
@@ -944,6 +948,17 @@ public class CoreContainer {
     status |= LOAD_COMPLETE | INITIAL_CORE_LOAD_COMPLETE;
   }
 
+  private void createUserFilesDirectory() {
+    if (isZooKeeperAware()) {
+      Path userFilesPath = getUserFilesPath(); // TODO make configurable on 
cfg?
+      try {
+        Files.createDirectories(userFilesPath); // does nothing if already 
exists
+      } catch (Exception e) {
+        log.warn("Unable to create [{}].  Features requiring this directory 
may fail.", userFilesPath, e);
+      }
+    }
+  }
+
   public void securityNodeChanged() {
     log.info("Security node changed, reloading security.json");
     reloadSecurityProperties();
@@ -2069,7 +2084,8 @@ public class CoreContainer {
   /**
    * A path where Solr users can retrieve arbitrary files from.  Absolute.
    * <p>
-   * Files located in this directory can be manipulated using select Solr 
features (e.g. streaming expressions).
+   * This directory is generally created by each node on startup.  Files 
located in this directory can then be
+   * manipulated using select Solr features (e.g. streaming expressions).
    */
   public Path getUserFilesPath() {
     return solrHome.resolve("userfiles");
diff --git a/solr/core/src/java/org/apache/solr/core/SolrPaths.java 
b/solr/core/src/java/org/apache/solr/core/SolrPaths.java
index 00ec58b..37fe247 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrPaths.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrPaths.java
@@ -71,34 +71,20 @@ public final class SolrPaths {
   public static void assertPathAllowed(Path pathToAssert, Set<Path> 
allowPaths) throws SolrException {
     if (ALL_PATHS.equals(allowPaths)) return; // Catch-all allows all paths 
(*/_ALL_)
     if (pathToAssert == null) return;
-    assertNotUnc(pathToAssert);
-    // Conversion Path -> String -> Path is to be able to compare against 
org.apache.lucene.mockfile.FilterPath instances
-    final Path path = Path.of(pathToAssert.toString()).normalize();
-    assertNoPathTraversal(path);
-    if (!path.isAbsolute()) return; // All relative paths are accepted
-    if (allowPaths.stream().noneMatch(p -> 
path.startsWith(Path.of(p.toString())))) {
+    if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-          "Path " + path + " must be relative to SOLR_HOME, SOLR_DATA_HOME 
coreRootDirectory. Set system property 'solr.allowPaths' to add other allowed 
paths.");
+          "Path " + pathToAssert + " disallowed. UNC paths not supported. 
Please use drive letter instead.");
     }
-  }
-
-  /**
-   * Asserts that a path does not contain directory traversal
-   */
-  public static void assertNoPathTraversal(Path pathToAssert) {
-    if (pathToAssert.toString().contains("..")) {
+    // Conversion Path -> String -> Path is to be able to compare against 
org.apache.lucene.mockfile.FilterPath instances
+    final Path path = Path.of(pathToAssert.toString()).normalize();
+    if (path.startsWith("..")) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
           "Path " + pathToAssert + " disallowed due to path traversal..");
     }
-  }
-
-  /**
-   * Asserts that a path is not a Windows UNC path
-   */
-  public static void assertNotUnc(Path pathToAssert) {
-    if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) {
+    if (!path.isAbsolute()) return; // All relative paths are accepted
+    if (allowPaths.stream().noneMatch(p -> 
path.startsWith(Path.of(p.toString())))) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-          "Path " + pathToAssert + " disallowed. UNC paths not supported.");
+          "Path " + path + " must be relative to SOLR_HOME, SOLR_DATA_HOME 
coreRootDirectory. Set system property 'solr.allowPaths' to add other allowed 
paths.");
     }
   }
 
diff --git 
a/solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java 
b/solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
index 775c009..0df0538 100644
--- a/solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
+++ b/solr/core/src/java/org/apache/solr/filestore/DistribPackageStore.java
@@ -51,7 +51,6 @@ import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.common.annotation.SolrThreadUnsafe;
 import org.apache.solr.core.CoreContainer;
-import org.apache.solr.core.SolrPaths;
 import org.apache.solr.filestore.PackageStoreAPI.MetaData;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
@@ -92,16 +91,11 @@ public class DistribPackageStore implements PackageStore {
     if (File.separatorChar == '\\') {
       path = path.replace('/', File.separatorChar);
     }
-    SolrPaths.assertNotUnc(Path.of(path));
-    while (path.startsWith(File.separator)) { // Trim all leading slashes
-      path = path.substring(1);
+    if (!path.isEmpty() && path.charAt(0) != File.separatorChar) {
+      path = File.separator + path;
     }
-    var finalPath = getPackageStoreDirPath(solrHome).resolve(path);
-    // Guard against path traversal by asserting final path is sub path of 
filestore
-    if 
(!finalPath.normalize().startsWith(getPackageStoreDirPath(solrHome).normalize()))
 {
-      throw new SolrException(BAD_REQUEST, "Illegal path " + path);
-    }
-    return finalPath;
+    // Use concat because path might start with a slash and be incorrectly 
interpreted as absolute
+    return solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY + path);
   }
 
   class FileInfo {
@@ -577,17 +571,8 @@ public class DistribPackageStore implements PackageStore {
     }
   }
 
-  public static synchronized Path getPackageStoreDirPath(Path solrHome) {
-    var path = solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY);
-    if (!Files.exists(path)) {
-      try {
-        Files.createDirectories(path);
-        log.info("Created filestore folder {}", path);
-      } catch (IOException e) {
-        throw new SolrException(SERVER_ERROR, "Faild creating 'filestore' 
folder in SOLR_HOME", e);
-      }
-    }
-    return path;
+  public static Path getPackageStoreDirPath(Path solrHome) {
+    return solrHome.resolve(PackageStoreAPI.PACKAGESTORE_DIRECTORY);
   }
 
   private static String _getMetapath(String path) {
diff --git a/solr/solr-ref-guide/modules/query-guide/pages/loading.adoc 
b/solr/solr-ref-guide/modules/query-guide/pages/loading.adoc
index 150f981..571bff9 100644
--- a/solr/solr-ref-guide/modules/query-guide/pages/loading.adoc
+++ b/solr/solr-ref-guide/modules/query-guide/pages/loading.adoc
@@ -22,7 +22,7 @@ These functions are designed to cut down the time spent on 
data preparation and
 == Reading Files
 
 The `cat` function can be used to read files under the *userfiles* directory in
-`$SOLR_HOME`. This directory must be created by the user.
+`$SOLR_HOME`.
 The `cat` function takes two parameters.
 
 The first parameter is a comma-delimited list of paths.
diff --git 
a/solr/solr-ref-guide/modules/query-guide/pages/stream-source-reference.adoc 
b/solr/solr-ref-guide/modules/query-guide/pages/stream-source-reference.adoc
index abe3b97..60f421f 100644
--- a/solr/solr-ref-guide/modules/query-guide/pages/stream-source-reference.adoc
+++ b/solr/solr-ref-guide/modules/query-guide/pages/stream-source-reference.adoc
@@ -325,7 +325,7 @@ Each emitted tuple contains two fields: `file` and `line`.
 
 * `filePaths`: (Mandatory) a comma separated list of filepaths to read lines 
from.
 If the specified path is a directory, it will be crawled recursively and all 
contained files will be read.
-To prevent malicious users from reading arbitrary files from Solr nodes, 
`filePaths` must be a relative path measured from a chroot of 
`$SOLR_HOME/userfiles` on the node running the streaming expression. This 
directory must be created by the user.
+To prevent malicious users from reading arbitrary files from Solr nodes, 
`filePaths` must be a relative path measured from a chroot of 
`$SOLR_HOME/userfiles` on the node running the streaming expression.
 * `maxLines`: (defaults to -1) The maximum number of lines to read (and tuples 
to emit).
 If a negative value is specified, all lines in the specified files will be 
emitted as tuples.
 Files are read in the order that they appear in the comma-separated 
`filePaths` argument.
diff --git 
a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc 
b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
index 2bc9c22..b03493b 100644
--- 
a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
+++ 
b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
@@ -128,8 +128,6 @@ changes, however the module needs to be installed - see the 
section xref:deploym
 * SOLR-13989: Hadoop authentication support has been moved to the hadoop-auth 
module. Existing Solr configurations do not need any Hadoop authentication 
related
 changes, however the module needs to be installed - see the section 
xref:deployment-guide:hadoop-authentication-plugin.adoc[].
 
-* SOLR-15950: The folder $SOLR_HOME/userfiles, used by the "cat" streaming 
expression, is no longer created automatically on startup. The user must create 
this folder.
-
 * SOLR-15097: JWTAuthPlugin has been moved to a module. Users need to add the 
module to classpath. The plugin has also
   changed package name to `org.apache.solr.security.jwt`, but can still be 
loaded as shortform `class="solr.JWTAuthPlugin"`.
 
diff --git 
a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
 
b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
index 38c0902..4043708 100644
--- 
a/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
+++ 
b/solr/solrj/src/test/org/apache/solr/client/solrj/io/stream/StreamExpressionTest.java
@@ -3917,6 +3917,7 @@ public class StreamExpressionTest extends 
SolrCloudTestCase {
 
   private static Path findUserFilesDataDir() {
     for (JettySolrRunner jetty : cluster.getJettySolrRunners()) {
+      final String baseDir = cluster.getBaseDir().toAbsolutePath().toString();
       for (CoreDescriptor coreDescriptor : 
jetty.getCoreContainer().getCoreDescriptors()) {
         if (coreDescriptor.getCollectionName().equals(FILESTREAM_COLLECTION)) {
           return jetty.getCoreContainer().getUserFilesPath();

Reply via email to