This is an automated email from the ASF dual-hosted git repository.
epugh 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 fee2cbbb845 SOLR-17739: ConfigSet API should return error when
forbidden file types are in the upload (#3351)
fee2cbbb845 is described below
commit fee2cbbb845cca8a0e235b7726eec25c9bb79d72
Author: Abhishek Umarjikar <[email protected]>
AuthorDate: Wed May 21 19:57:57 2025 +0530
SOLR-17739: ConfigSet API should return error when forbidden file types are
in the upload (#3351)
---------
Co-authored-by: Eric Pugh <[email protected]>
---
solr/CHANGES.txt | 2 ++
.../java/org/apache/solr/cloud/ZkConfigSetService.java | 6 +++++-
.../apache/solr/core/FileSystemConfigSetService.java | 7 +++++--
.../test/org/apache/solr/cloud/TestConfigSetsAPI.java | 3 ++-
.../apache/solr/common/cloud/ZkMaintenanceUtils.java | 11 ++++++-----
.../solr/common/cloud/TestZkConfigSetService.java | 18 ++++++++++++++++--
6 files changed, 36 insertions(+), 11 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d3d7f82d142..539543cb257 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -47,6 +47,8 @@ Improvements
* SOLR-17043: "LB" and "Cloud" clients no longer use brittle URL
pattern-matching when routing requests. (Jude Muriithi via David Smiley, Jason
Gerlowski)
+* SOLR-17739: Uploading a config sets with forbidden file types will produces
a Exception. Previously it was just ignoring those files and logging a WARN.
(Abhishek Umarjikar via Eric Pugh)
+
Optimizations
---------------------
* SOLR-17568: The CLI bin/solr export tool now contacts the appropriate nodes
directly for data instead of proxying through one.
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 44638776bf3..9766c6df42d 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
@@ -179,7 +179,11 @@ public class ZkConfigSetService extends ConfigSetService {
String filePath = CONFIGS_ZKNODE + "/" + configName + "/" + fileName;
try {
if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(fileName)) {
- log.warn("Not including uploading file to config, as it is a forbidden
type: {}", fileName);
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The file type provided for upload, '"
+ + fileName
+ + "', is forbidden for use in uploading configsets.");
} else if (FileTypeMagicUtil.isFileForbiddenInConfigset(data)) {
String mimeType = FileTypeMagicUtil.INSTANCE.guessMimeType(data);
throw new SolrException(
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 c904035983a..1b7cc819fbe 100644
--- a/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/FileSystemConfigSetService.java
@@ -150,8 +150,11 @@ public class FileSystemConfigSetService extends
ConfigSetService {
String configName, String fileName, byte[] data, boolean
overwriteOnExists)
throws IOException {
if (ZkMaintenanceUtils.isFileForbiddenInConfigSets(fileName)) {
- log.warn("Not including uploading file to config, as it is a forbidden
type: {}", fileName);
- return;
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST,
+ "The file type provided for upload, '"
+ + fileName
+ + "', is forbidden for use in uploading configsets.");
}
if (FileTypeMagicUtil.isFileForbiddenInConfigset(data)) {
String mimeType = FileTypeMagicUtil.INSTANCE.guessMimeType(data);
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 cc0644364bf..7be45b2daac 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
@@ -594,7 +594,8 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
.build()) {
String configPath = "/configs/" + configsetName + configsetSuffix;
assertEquals(
- 0, uploadConfigSet(configsetName, configsetSuffix, null, true,
false, v2, true, false));
+ 400, uploadConfigSet(configsetName, configsetSuffix, null, true,
false, v2, true, false));
+
for (String fileEnding :
ZkMaintenanceUtils.DEFAULT_FORBIDDEN_FILE_TYPES) {
String f = configPath + "/test." + fileEnding;
assertFalse(
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 6e18e0d3e54..f4eee0e78da 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
@@ -33,6 +33,7 @@ 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.StrUtils;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
@@ -344,11 +345,11 @@ public class ZkMaintenanceUtils {
return FileVisitResult.CONTINUE;
}
if (isFileForbiddenInConfigSets(filename)) {
- log.info(
- "uploadToZK skipping '{}' due to forbidden file types '{}'",
- filename,
- USE_FORBIDDEN_FILE_TYPES);
- return FileVisitResult.CONTINUE;
+ 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);
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 ed23cb863a1..7d083d330a3 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
@@ -31,6 +31,7 @@ import org.apache.solr.SolrTestCaseJ4;
import org.apache.solr.cloud.AbstractZkTestCase;
import org.apache.solr.cloud.ZkConfigSetService;
import org.apache.solr.cloud.ZkTestServer;
+import org.apache.solr.common.SolrException;
import org.apache.solr.core.ConfigSetService;
import org.apache.solr.core.CoreContainer;
import org.apache.solr.util.LogLevel;
@@ -102,6 +103,19 @@ public class TestZkConfigSetService extends SolrTestCaseJ4
{
assertEquals("testconfig", configs.get(0));
assertTrue(configSetService.checkConfigExists("testconfig"));
+ Path tempConfigForbidden = createTempDir("config");
+ Files.createFile(tempConfigForbidden.resolve("Test.java"));
+ Files.createFile(tempConfigForbidden.resolve("file1"));
+ Files.createDirectory(tempConfigForbidden.resolve("dir"));
+
+ Exception ex =
+ assertThrows(
+ SolrException.class,
+ () -> {
+ configSetService.uploadConfig("_testconf",
tempConfigForbidden);
+ });
+ assertTrue(ex.getMessage().contains("is forbidden for use in uploading
configsets."));
+
// check downloading
Path downloadPath = createTempDir("download");
configSetService.downloadConfig("testconfig", downloadPath);
@@ -120,7 +134,7 @@ public class TestZkConfigSetService extends SolrTestCaseJ4 {
Files.write(tempConfig.resolve("file1"), overwritten);
configSetService.uploadConfig("testconfig", tempConfig);
- assertEquals(1, configSetService.listConfigs().size());
+ assertEquals(2, configSetService.listConfigs().size());
Path download2 = createTempDir("download2");
configSetService.downloadConfig("testconfig", download2);
byte[] checkdata2 = Files.readAllBytes(download2.resolve("file1"));
@@ -128,7 +142,7 @@ public class TestZkConfigSetService extends SolrTestCaseJ4 {
// uploading same files to a new name creates a new config
configSetService.uploadConfig("config2", tempConfig);
- assertEquals(2, configSetService.listConfigs().size());
+ assertEquals(3, configSetService.listConfigs().size());
// Test copying a config works in both flavors
configSetService.copyConfig("config2", "config2copy");
