diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 447761fdd9c..ca3989ec413 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 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 4a1ec2125fd..95c852039b4 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); 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 2149267a142..ebcb310b7af 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 }