This is an automated email from the ASF dual-hosted git repository.
dsmiley 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 d2f8faf5705 Refactor: move isFileForbiddenInConfigSets (#4193)
d2f8faf5705 is described below
commit d2f8faf5705a955f46d40af98630520ff732e10c
Author: David Smiley <[email protected]>
AuthorDate: Thu Mar 19 22:45:59 2026 -0400
Refactor: move isFileForbiddenInConfigSets (#4193)
CLI configSet upload no longer blocks certain file types. The user of the
CLI tool must already have permissions to touch ZooKeeper. Enforcement of such
remains at Solr's ConfigSet HTTP endpoint.
Co-authored-by: copilot-swe-agent[bot]
<[email protected]>
Co-authored-by: dsmiley <[email protected]>
---
changelog/unreleased/move-configset-methods.yml | 7 ++++
.../org/apache/solr/cloud/ZkConfigSetService.java | 26 +++++++++++--
.../org/apache/solr/core/ConfigSetService.java | 28 ++++++++++++++
.../solr/core/FileSystemConfigSetService.java | 5 +--
.../org/apache/solr/core/backup/BackupManager.java | 5 +--
.../solr/handler/configsets/UploadConfigSet.java | 3 +-
.../org/apache/solr/cloud/TestConfigSetsAPI.java | 7 ++--
.../solr/common/cloud/ZkMaintenanceUtils.java | 43 ++--------------------
.../solr/common/cloud/TestZkConfigSetService.java | 4 +-
9 files changed, 71 insertions(+), 57 deletions(-)
diff --git a/changelog/unreleased/move-configset-methods.yml
b/changelog/unreleased/move-configset-methods.yml
new file mode 100644
index 00000000000..428ed169cf3
--- /dev/null
+++ b/changelog/unreleased/move-configset-methods.yml
@@ -0,0 +1,7 @@
+title: CLI configSet upload no longer blocks certain file types. The user of
the CLI tool must already have permissions to touch ZooKeeper. Enforcement of
such remains at Solr's ConfigSet HTTP endpoint.
+type: other
+authors:
+ - name: David Smiley
+links:
+ - name: PR#4193
+ url: https://github.com/apache/solr/pull/4193
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
index 99b4fb2d675..a0b9880c4ca 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
@@ -18,7 +18,11 @@ package org.apache.solr.cloud;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
@@ -168,6 +172,22 @@ public class ZkConfigSetService extends ConfigSetService {
@Override
public void uploadConfig(String configName, Path dir) throws IOException {
+ Files.walkFileTree(
+ dir,
+ new SimpleFileVisitor<>() {
+ @Override
+ public FileVisitResult visitFile(Path file, BasicFileAttributes
attrs) {
+ String filename = file.getFileName().toString();
+ if (ConfigSetService.isFileForbiddenInConfigSets(filename)) {
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The file type provided for upload, '"
+ + filename
+ + "', is forbidden for use in uploading configsets.");
+ }
+ return FileVisitResult.CONTINUE;
+ }
+ });
zkClient.uploadToZK(
dir, CONFIGS_ZKNODE + "/" + configName,
ConfigSetService.UPLOAD_FILENAME_EXCLUDE_PATTERN);
}
@@ -178,7 +198,7 @@ public class ZkConfigSetService extends ConfigSetService {
throws IOException {
String filePath = CONFIGS_ZKNODE + "/" + configName + "/" + fileName;
try {
- if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(fileName)) {
+ if (ConfigSetService.isFileForbiddenInConfigSets(fileName)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"The file type provided for upload, '"
@@ -324,11 +344,11 @@ public class ZkConfigSetService extends ConfigSetService {
private void copyData(String fromZkFilePath, String toZkFilePath)
throws KeeperException, InterruptedException {
- if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(fromZkFilePath)) {
+ if (ConfigSetService.isFileForbiddenInConfigSets(fromZkFilePath)) {
log.warn(
"Skipping copy of file in ZK, as the source file is a forbidden
type: {}",
fromZkFilePath);
- } else if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(toZkFilePath)) {
+ } else if (ConfigSetService.isFileForbiddenInConfigSets(toZkFilePath)) {
log.warn(
"Skipping download of file from ZK, as the target file is a
forbidden type: {}",
toZkFilePath);
diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
index 40145779a0d..409809a7d98 100644
--- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
@@ -26,6 +26,7 @@ import java.nio.file.Path;
import java.util.List;
import java.util.Locale;
import java.util.Map;
+import java.util.Set;
import java.util.regex.Pattern;
import org.apache.solr.cloud.ZkConfigSetService;
import org.apache.solr.cloud.ZkController;
@@ -33,6 +34,7 @@ import org.apache.solr.common.ConfigNode;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.StrUtils;
import org.apache.solr.handler.admin.ConfigSetsHandler;
import org.apache.solr.schema.IndexSchema;
import org.apache.solr.schema.IndexSchemaFactory;
@@ -46,6 +48,32 @@ public abstract class ConfigSetService {
public static final String UPLOAD_FILENAME_EXCLUDE_REGEX = "^\\..*$";
public static final Pattern UPLOAD_FILENAME_EXCLUDE_PATTERN =
Pattern.compile(UPLOAD_FILENAME_EXCLUDE_REGEX);
+
+ public static final String FORBIDDEN_FILE_TYPES_PROP =
"solr.configset.forbidden.file.types";
+ public static final Set<String> DEFAULT_FORBIDDEN_FILE_TYPES =
+ Set.of("class", "java", "jar", "tgz", "zip", "tar", "gz");
+
+ private static final Set<String> USE_FORBIDDEN_FILE_TYPES;
+
+ static {
+ String userForbiddenFileTypes =
EnvUtils.getProperty(FORBIDDEN_FILE_TYPES_PROP);
+ USE_FORBIDDEN_FILE_TYPES =
+ StrUtils.isNullOrEmpty(userForbiddenFileTypes)
+ ? DEFAULT_FORBIDDEN_FILE_TYPES
+ : Set.of(userForbiddenFileTypes.split(","));
+ }
+
+ /**
+ * Determine if a file path is forbidden for use in configsets based on its
file extension.
+ *
+ * @param filePath the file path or name to check
+ * @return true if the file extension is among the forbidden types
+ */
+ public static boolean isFileForbiddenInConfigSets(String filePath) {
+ int lastDot = filePath.lastIndexOf('.');
+ return lastDot >= 0 &&
USE_FORBIDDEN_FILE_TYPES.contains(filePath.substring(lastDot + 1));
+ }
+
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
public static ConfigSetService createConfigSetService(CoreContainer
coreContainer) {
diff --git
a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
index 811e6d91fee..496e9fa86a4 100644
--- a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
@@ -35,7 +35,6 @@ import java.util.Objects;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import org.apache.solr.common.SolrException;
-import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.util.Utils;
import org.apache.solr.util.FileTypeMagicUtil;
import org.apache.solr.util.FileUtils;
@@ -155,7 +154,7 @@ public class FileSystemConfigSetService extends
ConfigSetService {
public void uploadFileToConfig(
String configName, String fileName, byte[] data, boolean
overwriteOnExists)
throws IOException {
- if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(fileName)) {
+ if (ConfigSetService.isFileForbiddenInConfigSets(fileName)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"The file type provided for upload, '"
@@ -234,7 +233,7 @@ public class FileSystemConfigSetService extends
ConfigSetService {
@Override
public FileVisitResult visitFile(Path file, BasicFileAttributes
attrs)
throws IOException {
- if
(ZkMaintenanceUtils.isFileForbiddenInConfigSets(file.getFileName().toString()))
{
+ if
(ConfigSetService.isFileForbiddenInConfigSets(file.getFileName().toString())) {
log.warn(
"Not including uploading file to config, as it is a
forbidden type: {}",
file.getFileName());
diff --git a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
index 2525bc36ad8..f47df829f74 100644
--- a/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
+++ b/solr/core/src/java/org/apache/solr/core/backup/BackupManager.java
@@ -37,7 +37,6 @@ import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.cloud.ZkStateReader;
import org.apache.solr.common.util.Utils;
import org.apache.solr.core.ConfigSetService;
@@ -344,7 +343,7 @@ public class BackupManager {
// checking for '/' is correct for a directory since
ConfigSetService#getAllConfigFiles
// always separates file paths with '/'
if (!filePath.endsWith("/")) {
- if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(filePath)) {
+ if (ConfigSetService.isFileForbiddenInConfigSets(filePath)) {
log.warn(
"Not including zookeeper file in backup, as it is a forbidden
type: {}", filePath);
} else {
@@ -385,7 +384,7 @@ public class BackupManager {
switch (t) {
case FILE:
{
- if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(filePath)) {
+ if (ConfigSetService.isFileForbiddenInConfigSets(filePath)) {
log.warn(
"Not including zookeeper file in restore, as it is a
forbidden type: {}", file);
} else {
diff --git
a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java
b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java
index d87c38154f5..617d5c8af22 100644
--- a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java
+++ b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSet.java
@@ -30,7 +30,6 @@ import java.util.zip.ZipInputStream;
import org.apache.solr.client.api.endpoint.ConfigsetsApi;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.common.SolrException;
-import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.core.ConfigSetService;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.jersey.PermissionName;
@@ -129,7 +128,7 @@ public class UploadConfigSet extends ConfigSetAPIBase
implements ConfigsetsApi.U
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
"The file path provided for upload, '" + singleFilePath + "', is not
valid.");
- } else if
(ZkMaintenanceUtils.isFileForbiddenInConfigSets(fixedSingleFilePath)
+ } else if
(ConfigSetService.isFileForbiddenInConfigSets(fixedSingleFilePath)
|| FileTypeMagicUtil.isFileForbiddenInConfigset(data)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST,
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
index a09737174ae..5dc13d0ca3a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
@@ -84,7 +84,6 @@ import
org.apache.solr.client.solrj.response.schema.SchemaResponse;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.cloud.ZkMaintenanceUtils;
import org.apache.solr.common.params.CollectionParams.CollectionAction;
import org.apache.solr.common.params.ConfigSetParams;
import org.apache.solr.common.params.ConfigSetParams.ConfigSetAction;
@@ -583,7 +582,7 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
assertEquals(
400, uploadConfigSet(configsetName, configsetSuffix, null, true,
false, v2, true, false));
- for (String fileEnding :
ZkMaintenanceUtils.DEFAULT_FORBIDDEN_FILE_TYPES) {
+ for (String fileEnding : ConfigSetService.DEFAULT_FORBIDDEN_FILE_TYPES) {
String f = configPath + "/test." + fileEnding;
assertFalse(
"Expecting file " + f + " to not exist, because it has a forbidden
file type",
@@ -804,7 +803,7 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
.withTimeout(AbstractZkTestCase.TIMEOUT, TimeUnit.MILLISECONDS)
.withConnTimeOut(45000, TimeUnit.MILLISECONDS)
.build()) {
- for (String fileType : ZkMaintenanceUtils.DEFAULT_FORBIDDEN_FILE_TYPES) {
+ for (String fileType : ConfigSetService.DEFAULT_FORBIDDEN_FILE_TYPES) {
ignoreException("is forbidden for use in configSets");
assertEquals(
"Can't upload a configset file with a forbidden type: " + fileType,
@@ -1385,7 +1384,7 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
try (ZipOutputStream zout = new ZipOutputStream(out)) {
if (Files.isRegularFile(fileOrDirectory)) {
// Create entries with given file, one for each forbidden endding
- for (String fileType :
ZkMaintenanceUtils.DEFAULT_FORBIDDEN_FILE_TYPES) {
+ for (String fileType : ConfigSetService.DEFAULT_FORBIDDEN_FILE_TYPES) {
zout.putNextEntry(new ZipEntry("test." + fileType));
try (InputStream in = Files.newInputStream(fileOrDirectory)) {
diff --git
a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
index 0d98a65da7c..6438e84bbfc 100644
---
a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
+++
b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkMaintenanceUtils.java
@@ -29,12 +29,9 @@ import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Locale;
-import java.util.Set;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import org.apache.solr.client.solrj.SolrServerException;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.util.EnvUtils;
import org.apache.solr.common.util.StrUtils;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
@@ -345,13 +342,6 @@ public class ZkMaintenanceUtils {
filenameExclusions);
return FileVisitResult.CONTINUE;
}
- if (isFileForbiddenInConfigSets(filename)) {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST,
- "The file type provided for upload, '"
- + filename
- + "', is forbidden for use in uploading configsets.");
- }
// TODO: Cannot check MAGIC header for file since FileTypeGuesser
is in core
String zkNode = createZkNodeName(zkPath, rootPath, file);
try {
@@ -443,13 +433,9 @@ public class ZkMaintenanceUtils {
if (children.size() == 0) {
// If we didn't copy data down, then we also didn't create the file.
But we still need a
// marker on the local disk so create an empty file.
- if (isFileForbiddenInConfigSets(zkPath)) {
- log.warn("Skipping download of file from ZK, as it is a forbidden
type: {}", zkPath);
- } else {
- // TODO: Cannot check MAGIC header for file since FileTypeGuesser is
in core
- if (copyDataDown(zkClient, zkPath, file) == 0) {
- Files.createFile(file);
- }
+ // TODO: Cannot check MAGIC header for file since FileTypeGuesser is
in core
+ if (copyDataDown(zkClient, zkPath, file) == 0) {
+ Files.createFile(file);
}
} else {
Files.createDirectories(file); // Make parent dir.
@@ -581,29 +567,6 @@ public class ZkMaintenanceUtils {
return ret;
}
- public static final String FORBIDDEN_FILE_TYPES_PROP =
"solr.configset.forbidden.file.types";
- public static final Set<String> DEFAULT_FORBIDDEN_FILE_TYPES =
- Set.of("class", "java", "jar", "tgz", "zip", "tar", "gz");
- private static volatile Set<String> USE_FORBIDDEN_FILE_TYPES = null;
-
- public static boolean isFileForbiddenInConfigSets(String filePath) {
- // Try to set the forbidden file types just once, since it is set by
SysProp/EnvVar
- if (USE_FORBIDDEN_FILE_TYPES == null) {
- synchronized (DEFAULT_FORBIDDEN_FILE_TYPES) {
- if (USE_FORBIDDEN_FILE_TYPES == null) {
- String userForbiddenFileTypes =
EnvUtils.getProperty(FORBIDDEN_FILE_TYPES_PROP);
- if (StrUtils.isNullOrEmpty(userForbiddenFileTypes)) {
- USE_FORBIDDEN_FILE_TYPES = DEFAULT_FORBIDDEN_FILE_TYPES;
- } else {
- USE_FORBIDDEN_FILE_TYPES =
Set.of(userForbiddenFileTypes.split(","));
- }
- }
- }
- }
- int lastDot = filePath.lastIndexOf('.');
- return lastDot >= 0 &&
USE_FORBIDDEN_FILE_TYPES.contains(filePath.substring(lastDot + 1));
- }
-
/**
* Create a persistent znode with no data if it does not already exist
*
diff --git
a/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestZkConfigSetService.java
b/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestZkConfigSetService.java
index 9928d543210..d7ed83b4d61 100644
---
a/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestZkConfigSetService.java
+++
b/solr/solrj-zookeeper/src/test/org/apache/solr/common/cloud/TestZkConfigSetService.java
@@ -131,7 +131,7 @@ public class TestZkConfigSetService extends SolrTestCaseJ4 {
Files.write(tempConfig.resolve("file1"), overwritten);
configSetService.uploadConfig("testconfig", tempConfig);
- assertEquals(2, configSetService.listConfigs().size());
+ assertEquals(1, configSetService.listConfigs().size());
Path download2 = createTempDir("download2");
configSetService.downloadConfig("testconfig", download2);
byte[] checkdata2 = Files.readAllBytes(download2.resolve("file1"));
@@ -139,7 +139,7 @@ public class TestZkConfigSetService extends SolrTestCaseJ4 {
// uploading same files to a new name creates a new config
configSetService.uploadConfig("config2", tempConfig);
- assertEquals(3, configSetService.listConfigs().size());
+ assertEquals(2, configSetService.listConfigs().size());
// Test copying a config works in both flavors
configSetService.copyConfig("config2", "config2copy");
