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();