Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
title: Add validation for configset names at API layer in line with what Admin UI enforces.
type: fixed
authors:
- name: Eric Pugh
links:
- name: SOLR-18170
url: https://issues.apache.org/jira/browse/SOLR-18170
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.solr.client.api.endpoint.ConfigsetsApi;
import org.apache.solr.client.api.model.CloneConfigsetRequestBody;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.client.solrj.util.SolrIdentifierValidator;
import org.apache.solr.cloud.ConfigSetCmds;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.ConfigSetParams;
Expand All @@ -50,6 +51,7 @@ public CloneConfigSet(
public SolrJerseyResponse cloneExistingConfigSet(CloneConfigsetRequestBody requestBody)
throws Exception {
final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
SolrIdentifierValidator.validateConfigSetName(requestBody.name);
if (configSetService.checkConfigExists(requestBody.name)) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "ConfigSet already exists: " + requestBody.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
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.client.solrj.util.SolrIdentifierValidator;
import org.apache.solr.common.SolrException;
import org.apache.solr.core.ConfigSetService;
import org.apache.solr.core.CoreContainer;
Expand Down Expand Up @@ -58,6 +59,7 @@ public SolrJerseyResponse uploadConfigSet(
throws IOException {
final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
ensureConfigSetUploadEnabled();
SolrIdentifierValidator.validateConfigSetName(configSetName);

boolean overwritesExisting = configSetService.checkConfigExists(configSetName);
// Get upload parameters
Expand Down Expand Up @@ -110,6 +112,7 @@ public SolrJerseyResponse uploadConfigSetFile(
throws IOException {
final var response = instantiateJerseyResponse(SolrJerseyResponse.class);
ensureConfigSetUploadEnabled();
SolrIdentifierValidator.validateConfigSetName(configSetName);

boolean overwritesExisting = configSetService.checkConfigExists(configSetName);

Expand Down
24 changes: 24 additions & 0 deletions solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,14 @@ public void testCreateErrors() throws Exception {
Create baseConfigNoExists = new Create();
baseConfigNoExists.setConfigSetName("newConfigSet").setBaseConfigSetName("baseConfigSet");
verifyException(solrClient, baseConfigNoExists, "Base ConfigSet does not exist");

// Invalid configset names
for (String invalidName :
new String[] {"configset!", "configset\"", "-configset", "configset name"}) {
Create invalidNameCreate = new Create();
invalidNameCreate.setConfigSetName(invalidName).setBaseConfigSetName("_default");
verifyException(solrClient, invalidNameCreate, "Invalid configset");
}
}
}

Expand Down Expand Up @@ -383,6 +391,22 @@ public void testUploadErrors() throws Exception {
"Expected file doesnt exist in zk. It's possibly overwritten",
zkClient.exists("/configs/myconf/anotherDummyFile"));

// Checking error when configuration name contains invalid characters
for (String invalidName : new String[] {"configset!", "-configset"}) {
map =
postDataAndGetResponse(
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In another PR, this will move to strongly typed SolrJ object.

cluster.getSolrClient(),
cluster.getJettySolrRunners().get(0).getBaseUrl().toString()
+ "/admin/configs?action=UPLOAD&name="
+ invalidName,
emptyData,
null,
false);
assertNotNull(map);
statusCode = (long) getObjectByPath(map, Arrays.asList("responseHeader", "status"));
assertEquals("Expected 400 for invalid configset name: " + invalidName, 400l, statusCode);
}

zkClient.close();
solrClient.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public enum IdentifierType {
SHARD,
COLLECTION,
CORE,
ALIAS
ALIAS,
CONFIGSET
}

public static String validateName(IdentifierType type, String name) {
Expand All @@ -59,6 +60,10 @@ public static String validateCoreName(String coreName) {
return validateName(IdentifierType.CORE, coreName);
}

public static String validateConfigSetName(String configSetName) {
return validateName(IdentifierType.CONFIGSET, configSetName);
}

private static boolean validateIdentifier(String identifier) {
if (identifier != null && identifierPattern.matcher(identifier).matches()) {
return true;
Expand Down
Loading