lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From er...@apache.org
Subject [1/2] lucene-solr git commit: SOLR-8308: Core gets inaccessible after RENAME operation with special characters (cherry picked from commit 70f8742)
Date Tue, 02 Feb 2016 22:55:20 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/branch_5x 61601ee20 -> 4de5f1d99


SOLR-8308: Core gets inaccessible after RENAME operation with special characters
(cherry picked from commit 70f8742)


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/7dcb0772
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/7dcb0772
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/7dcb0772

Branch: refs/heads/branch_5x
Commit: 7dcb07723cdd90066857bacac7d5e31eda22f8ce
Parents: 61601ee
Author: Erick Erickson <erick@apache.org>
Authored: Tue Feb 2 13:18:21 2016 -0800
Committer: Erick Erickson <erick@apache.org>
Committed: Tue Feb 2 13:18:21 2016 -0800

----------------------------------------------------------------------
 .../org/apache/solr/core/CoreContainer.java     | 58 +++++++++++--------
 .../handler/admin/CoreAdminHandlerTest.java     | 61 ++++++++++++++++++--
 2 files changed, 91 insertions(+), 28 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/7dcb0772/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 5f48d91..264bb19 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/7dcb0772/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
 
   }


Mime
View raw message