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/02 20:34:19 UTC
lucene-solr git commit: SOLR-8308: Core gets inaccessible after
RENAME operation with special characters
Repository: lucene-solr
Updated Branches:
refs/heads/master 70ad8316f -> 70f87420a
SOLR-8308: Core gets inaccessible after RENAME operation with special characters
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/70f87420
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/70f87420
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/70f87420
Branch: refs/heads/master
Commit: 70f87420ab106989c9501870f2f851d5f5f85ea5
Parents: 70ad831
Author: Erick Erickson <er...@apache.org>
Authored: Tue Feb 2 11:23:48 2016 -0800
Committer: Erick Erickson <er...@apache.org>
Committed: Tue Feb 2 11:23:48 2016 -0800
----------------------------------------------------------------------
solr/CHANGES.txt | 5 ++
.../org/apache/solr/core/CoreContainer.java | 58 +++++++++++--------
.../handler/admin/CoreAdminHandlerTest.java | 61 ++++++++++++++++++--
3 files changed, 96 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/70f87420/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 447761f..ca3989e 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -144,6 +144,9 @@ New Features
* SOLR-8285: Ensure the /export handler works with NULL field values (Joel Bernstein)
+* SOLR-8308: Core gets inaccessible after RENAME operation with special characters
+ (Erik Hatcher, Erick Erickson)
+
Bug Fixes
----------------------
* SOLR-8386: Add field option in the new admin UI schema page loads up even when no schemaFactory has been
@@ -590,6 +593,8 @@ Other Changes
* SOLR-8600: add & use ReRankQParserPlugin parameter [default] constants,
changed ReRankQuery.toString to use StringBuilder. (Christine Poerschke)
+* SOLR-8308: Core gets inaccessible after RENAME operation with special characters.
+
================== 5.4.1 ==================
Bug Fixes
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/70f87420/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 4a1ec21..95c8520 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -32,6 +32,7 @@ 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;
@@ -113,12 +114,12 @@ public class CoreContainer {
protected Properties containerProperties;
private ConfigSetService coreConfigService;
-
+
protected ZkContainer zkSys = new ZkContainer();
protected ShardHandlerFactory shardHandlerFactory;
-
+
private UpdateShardHandler updateShardHandler;
-
+
private ExecutorService coreContainerWorkExecutor = ExecutorUtil.newMDCAwareCachedThreadPool(
new DefaultSolrThreadFactory("coreContainerWorkExecutor") );
@@ -131,7 +132,7 @@ public class CoreContainer {
protected final String solrHome;
protected final CoresLocator coresLocator;
-
+
private String hostName;
private final JarRepository jarRepository = new JarRepository(this);
@@ -207,7 +208,7 @@ public class CoreContainer {
public CoreContainer(NodeConfig config, Properties properties) {
this(config, properties, new CorePropertiesLocator(config.getCoreRootDirectory()));
}
-
+
public CoreContainer(NodeConfig config, Properties properties, boolean asyncSolrCoreLoad) {
this(config, properties, new CorePropertiesLocator(config.getCoreRootDirectory()), asyncSolrCoreLoad);
}
@@ -215,7 +216,7 @@ public class CoreContainer {
public CoreContainer(NodeConfig config, Properties properties, CoresLocator locator) {
this(config, properties, locator, false);
}
-
+
public CoreContainer(NodeConfig config, Properties properties, CoresLocator locator, boolean asyncSolrCoreLoad) {
this.loader = config.getSolrResourceLoader();
this.solrHome = loader.getInstancePath().toString();
@@ -335,7 +336,7 @@ public class CoreContainer {
/**
* This method allows subclasses to construct a CoreContainer
* without any default init behavior.
- *
+ *
* @param testConstructor pass (Object)null.
* @lucene.experimental
*/
@@ -350,7 +351,7 @@ public class CoreContainer {
public static CoreContainer createAndLoad(Path solrHome) {
return createAndLoad(solrHome, solrHome.resolve(SolrXmlConfig.SOLR_XML_FILE));
}
-
+
/**
* Create a new CoreContainer and load its cores
* @param solrHome the solr home directory
@@ -368,7 +369,7 @@ public class CoreContainer {
}
return cc;
}
-
+
public Properties getContainerProperties() {
return containerProperties;
}
@@ -463,7 +464,7 @@ public class CoreContainer {
if (zkSys.getZkController() != null) {
zkSys.getZkController().throwErrorIfReplicaReplaced(cd);
}
-
+
core = create(cd, false);
} finally {
if (asyncSolrCoreLoad) {
@@ -510,7 +511,7 @@ public class CoreContainer {
ExecutorUtil.shutdownAndAwaitTermination(coreLoadExecutor);
}
}
-
+
if (isZooKeeperAware()) {
zkSys.getZkController().checkOverseerDesignate();
}
@@ -536,7 +537,7 @@ public class CoreContainer {
}
private volatile boolean isShutDown = false;
-
+
public boolean isShutDown() {
return isShutDown;
}
@@ -547,11 +548,11 @@ public class CoreContainer {
public void shutdown() {
log.info("Shutting down CoreContainer instance="
+ System.identityHashCode(this));
-
+
isShutDown = true;
-
+
ExecutorUtil.shutdownAndAwaitTermination(coreContainerWorkExecutor);
-
+
if (isZooKeeperAware()) {
cancelCoreRecoveries();
zkSys.publishCoresAsDown(solrCores.getCores());
@@ -640,7 +641,7 @@ public class CoreContainer {
}
}
}
-
+
@Override
protected void finalize() throws Throwable {
try {
@@ -655,17 +656,26 @@ public class CoreContainer {
public CoresLocator getCoresLocator() {
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." );
}
- if( name == null ||
- name.indexOf( '/' ) >= 0 ||
- name.indexOf( '\\' ) >= 0 ){
- throw new RuntimeException( "Invalid core name: "+name );
- }
- // We can register a core when creating them via the admin UI, so we need to insure that the dynamic descriptors
+
+ // We can register a core when creating them via the admin UI, so we need to ensure that the dynamic descriptors
// are up to date
CoreDescriptor cd = core.getCoreDescriptor();
if ((cd.isTransient() || ! cd.isLoadOnStartup())
@@ -808,6 +818,7 @@ public class CoreContainer {
SolrCore core = null;
try {
MDCLoggingContext.setCore(core);
+ validateCoreName(dcore.getName());
if (zkSys.getZkController() != null) {
zkSys.getZkController().preRegister(dcore);
}
@@ -1010,6 +1021,7 @@ public class CoreContainer {
}
public void rename(String name, String toName) {
+ 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/70f87420/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 2149267..ebcb310 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
@@ -128,10 +128,25 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
assertTrue("instDir doesn't exist: " + instDir, Files.exists(instDir));
final File instPropFile = new File(workDir, "instProp");
FileUtils.copyDirectory(instDir.toFile(), instPropFile);
-
- // create a new core (using CoreAdminHandler) w/ properties
-
+
SolrQueryResponse resp = new SolrQueryResponse();
+ // Sneaking in a test for using a bad core name
+ try {
+ admin.handleRequestBody
+ (req(CoreAdminParams.ACTION,
+ CoreAdminParams.CoreAdminAction.CREATE.toString(),
+ CoreAdminParams.INSTANCE_DIR, instPropFile.getAbsolutePath(),
+ CoreAdminParams.NAME, "ugly$core=name"),
+ resp);
+
+ } catch (SolrException se) {
+ assertTrue("Expected error message for bad core name.", se.toString().contains("Invalid core name"));
+ }
+ CoreDescriptor cd = cores.getCoreDescriptor("ugly$core=name");
+ assertNull("Should NOT have added this core!", cd);
+
+ // create a new core (using CoreAdminHandler) w/ properties
+
admin.handleRequestBody
(req(CoreAdminParams.ACTION,
CoreAdminParams.CoreAdminAction.CREATE.toString(),
@@ -142,7 +157,7 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
resp);
assertNull("Exception on create", resp.getException());
- CoreDescriptor cd = cores.getCoreDescriptor("props");
+ cd = cores.getCoreDescriptor("props");
assertNotNull("Core not added!", cd);
assertEquals(cd.getCoreProperty("hoss", null), "man");
assertEquals(cd.getCoreProperty("foo", null), "baz");
@@ -188,7 +203,43 @@ public class CoreAdminHandlerTest extends SolrTestCaseJ4 {
assertEquals("bogus_dir_core status isn't empty",
0, ((NamedList)status.get("bogus_dir_core")).size());
-
+
+ //Try renaming the core, we should fail
+ // First assert that the props core exists
+ cd = cores.getCoreDescriptor("props");
+ assertNotNull("Core disappeared!", cd);
+
+ // now rename it something else just for kicks since we don't actually test this that I could find.
+ admin.handleRequestBody
+ (req(CoreAdminParams.ACTION,
+ CoreAdminParams.CoreAdminAction.RENAME.toString(),
+ CoreAdminParams.CORE, "props",
+ CoreAdminParams.OTHER, "rename_me"),
+ resp);
+
+ cd = cores.getCoreDescriptor("rename_me");
+ assertNotNull("Core should have been renamed!", cd);
+
+ // Rename it something bogus and see if you get an exception, the old core is still there and the bogus one isn't
+ try {
+ admin.handleRequestBody
+ (req(CoreAdminParams.ACTION,
+ CoreAdminParams.CoreAdminAction.RENAME.toString(),
+ 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"));
+ }
+
+ cd = cores.getCoreDescriptor("bad$name");
+ assertNull("Core should NOT exist!", cd);
+
+ cd = cores.getCoreDescriptor("rename_me");
+ assertNotNull("Core should have been renamed!", cd);
+
+
+
// :TODO: because of SOLR-3665 we can't ask for status from all cores
}