You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2016/02/08 00:03:17 UTC
lucene-solr git commit: SOLR-8642: SOLR allows creation of
collections with invalid names
Repository: lucene-solr
Updated Branches:
refs/heads/master dac06176e -> f77feb718
SOLR-8642: SOLR allows creation of collections with invalid names
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/f77feb71
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/f77feb71
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/f77feb71
Branch: refs/heads/master
Commit: f77feb718a4c93516ea65f4418514202206f2703
Parents: dac0617
Author: Erick Erickson <er...@apache.org>
Authored: Sun Feb 7 14:52:35 2016 -0800
Committer: Erick Erickson <er...@apache.org>
Committed: Sun Feb 7 14:52:40 2016 -0800
----------------------------------------------------------------------
solr/CHANGES.txt | 2 +
.../org/apache/solr/core/CoreContainer.java | 19 ++------
.../solr/handler/admin/CollectionsHandler.java | 7 ++-
.../solr/util/SolrIdentifierValidator.java | 47 ++++++++++++++++++++
.../apache/solr/cloud/TestCollectionAPI.java | 44 ++++++++++++++++++
.../handler/admin/CoreAdminHandlerTest.java | 6 +--
6 files changed, 105 insertions(+), 20 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f77feb71/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 93a8447..8ad69f7 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -353,6 +353,8 @@ New Features
system property. NOTE: this is an expert option and can result in more often needing to do full index replication
for recovery, the sweet spot for using this is very high volume, leader-only indexing. (Tim Potter, Erick Erickson)
+* SOLR-8642: SOLR allows creation of collections with invalid names
+ (Jason Gerlowski via Erick Erickson)
Bug Fixes
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f77feb71/solr/core/src/java/org/apache/solr/core/CoreContainer.java
----------------------------------------------------------------------
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 bca7e6c..b007dbf 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -31,7 +31,6 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Future;
-import java.util.regex.Pattern;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
@@ -64,6 +63,7 @@ import org.apache.solr.security.PKIAuthenticationPlugin;
import org.apache.solr.security.SecurityPluginHolder;
import org.apache.solr.update.UpdateShardHandler;
import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.apache.solr.util.SolrIdentifierValidator;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -656,19 +656,6 @@ public class CoreContainer {
return coresLocator;
}
- // Insure that the core name won't cause problems later on.
- final static Pattern corePattern = Pattern.compile("^[\\._A-Za-z0-9]*$");
-
-
- public void validateCoreName(String name) {
- if (name == null || !corePattern.matcher(name).matches()) {
- throw new IllegalArgumentException("Invalid core name: '" + name +
- "' Names must consist entirely of periods, underscores and alphanumerics");
- }
- }
-
-
-
protected SolrCore registerCore(String name, SolrCore core, boolean registerInZk) {
if( core == null ) {
throw new RuntimeException( "Can not register a null core." );
@@ -817,7 +804,7 @@ public class CoreContainer {
SolrCore core = null;
try {
MDCLoggingContext.setCore(core);
- validateCoreName(dcore.getName());
+ SolrIdentifierValidator.validateCoreName(dcore.getName());
if (zkSys.getZkController() != null) {
zkSys.getZkController().preRegister(dcore);
}
@@ -1020,7 +1007,7 @@ public class CoreContainer {
}
public void rename(String name, String toName) {
- validateCoreName(toName);
+ SolrIdentifierValidator.validateCoreName(toName);
try (SolrCore core = getCore(name)) {
if (core != null) {
registerCore(toName, core, true);
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f77feb71/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index ac14a22..c04386d 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -131,6 +131,7 @@ import org.apache.solr.handler.RequestHandlerBase;
import org.apache.solr.handler.component.ShardHandler;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.util.SolrIdentifierValidator;
import org.apache.zookeeper.CreateMode;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
@@ -356,7 +357,9 @@ public class CollectionsHandler extends RequestHandlerBase {
addMapObject(props, RULE);
addMapObject(props, SNITCH);
verifyRuleParams(h.coreContainer, props);
- if (SYSTEM_COLL.equals(props.get(NAME))) {
+ final String collectionName = (String) props.get(NAME);
+ SolrIdentifierValidator.validateCollectionName(collectionName);
+ if (SYSTEM_COLL.equals(collectionName)) {
//We must always create a .system collection with only a single shard
props.put(NUM_SLICES, 1);
props.remove(SHARDS_PROP);
@@ -426,6 +429,8 @@ public class CollectionsHandler extends RequestHandlerBase {
@Override
Map<String, Object> call(SolrQueryRequest req, SolrQueryResponse rsp, CollectionsHandler handler)
throws Exception {
+ final String aliasName = req.getParams().get(NAME);
+ SolrIdentifierValidator.validateCollectionName(aliasName);
return req.getParams().required().getAll(null, NAME, "collections");
}
},
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f77feb71/solr/core/src/java/org/apache/solr/util/SolrIdentifierValidator.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/util/SolrIdentifierValidator.java b/solr/core/src/java/org/apache/solr/util/SolrIdentifierValidator.java
new file mode 100644
index 0000000..dd6133d
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/util/SolrIdentifierValidator.java
@@ -0,0 +1,47 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.util;
+
+import java.lang.invoke.MethodHandles;
+import java.util.regex.Pattern;
+
+import org.apache.solr.common.SolrException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Ensures that provided identifiers align with Solr's recommendations/requirements for choosing
+ * collection, core, etc identifiers.
+ *
+ * Identifiers are allowed to contain underscores, periods, and alphanumeric characters.
+ */
+public class SolrIdentifierValidator {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ final static Pattern identifierPattern = Pattern.compile("^[\\._A-Za-z0-9]*$");
+
+ public static void validateCollectionName(String collectionName) throws SolrException {
+ validateCoreName(collectionName);
+ }
+
+ public static void validateCoreName(String name) throws SolrException {
+ if (name == null || !identifierPattern.matcher(name).matches()) {
+ log.info("Validation failed on the invalid identifier [{}]. Throwing SolrException to indicate a BAD REQUEST.", name);
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "Invalid name: '" + name + "' Identifiers must consist entirely of periods, underscores and alphanumerics");
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f77feb71/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
index 18aa33b..b19c1c1 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestCollectionAPI.java
@@ -28,6 +28,7 @@ import org.apache.solr.client.solrj.SolrQuery;
import org.apache.solr.client.solrj.SolrRequest;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.request.QueryRequest;
import org.apache.solr.client.solrj.response.QueryResponse;
@@ -78,6 +79,8 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
replicaPropTest();
clusterStatusZNodeVersion();
testClusterStateMigration();
+ testCollectionCreationNameValidation();
+ testAliasCreationNameValidation();
}
private void clusterStatusWithCollectionAndShard() throws IOException, SolrServerException {
@@ -627,6 +630,47 @@ public class TestCollectionAPI extends ReplicaPropertiesBase {
assertEquals(10, response.getResults().getNumFound());
}
}
+
+ private void testCollectionCreationNameValidation() throws Exception {
+ try (CloudSolrClient client = createCloudClient(null)) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("action", CollectionParams.CollectionAction.CREATE.toString());
+ params.set("name", "invalid@name#with$weird%characters");
+ SolrRequest request = new QueryRequest(params);
+ request.setPath("/admin/collections");
+
+ try {
+ client.request(request);
+ fail();
+ } catch (RemoteSolrException e) {
+ final String errorMessage = e.getMessage();
+ assertTrue(errorMessage.contains("Invalid name"));
+ assertTrue(errorMessage.contains("invalid@name#with$weird%characters"));
+ assertTrue(errorMessage.contains("Identifiers must consist entirely of"));
+ }
+ }
+ }
+
+ private void testAliasCreationNameValidation() throws Exception{
+ try (CloudSolrClient client = createCloudClient(null)) {
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("action", CollectionParams.CollectionAction.CREATEALIAS.toString());
+ params.set("name", "invalid@name#with$weird%characters");
+ params.set("collections", COLLECTION_NAME);
+ SolrRequest request = new QueryRequest(params);
+ request.setPath("/admin/collections");
+
+ try {
+ client.request(request);
+ fail();
+ } catch (RemoteSolrException e) {
+ final String errorMessage = e.getMessage();
+ assertTrue(errorMessage.contains("Invalid name"));
+ assertTrue(errorMessage.contains("invalid@name#with$weird%characters"));
+ assertTrue(errorMessage.contains("Identifiers must consist entirely of"));
+ }
+ }
+ }
// Expects the map will have keys, but blank values.
private Map<String, String> getProps(CloudSolrClient client, String collectionName, String replicaName, String... props)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/f77feb71/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
index bbdd1a3..2159325 100644
--- a/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/handler/admin/CoreAdminHandlerTest.java
@@ -139,7 +139,7 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
resp);
} catch (SolrException se) {
- assertTrue("Expected error message for bad core name.", se.toString().contains("Invalid core name"));
+ assertTrue("Expected error message for bad core name.", se.toString().contains("Invalid name"));
}
CoreDescriptor cd = cores.getCoreDescriptor("ugly$core=name");
assertNull("Should NOT have added this core!", cd);
@@ -227,8 +227,8 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
CoreAdminParams.CORE, "rename_me",
CoreAdminParams.OTHER, "bad$name"),
resp);
- } catch (IllegalArgumentException iae) { // why the heck does create return a SolrException (admittedly wrapping an IAE)
- assertTrue("Expected error message for bad core name.", iae.getMessage().contains("Invalid core name"));
+ } catch (SolrException e) { // why the heck does create return a SolrException (admittedly wrapping an IAE)
+ assertTrue("Expected error message for bad core name.", e.getMessage().contains("Invalid name"));
}
cd = cores.getCoreDescriptor("bad$name");