You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by ti...@apache.org on 2020/09/20 09:08:11 UTC

[curator] branch master updated: CURATOR-583: Fix ArrayIndexOutOfBoundsException when passing empty list parameter to reconfigure API

This is an automated email from the ASF dual-hosted git repository.

tison pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/curator.git


The following commit(s) were added to refs/heads/master by this push:
     new 800fdf4  CURATOR-583: Fix ArrayIndexOutOfBoundsException when passing empty list parameter to reconfigure API
800fdf4 is described below

commit 800fdf4c5cbe3f0e3cebc64357707ba743fc46e2
Author: hjyun <hj...@jam2in.com>
AuthorDate: Sat Sep 19 16:50:42 2020 +0900

    CURATOR-583: Fix ArrayIndexOutOfBoundsException when passing empty list parameter to reconfigure API
    
    This closes #374 .
---
 .../framework/imps/ReconfigBuilderImpl.java        |  6 +--
 .../framework/imps/TestReconfiguration.java        | 51 +++++++++++++++++++++-
 2 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/curator-framework/src/main/java/org/apache/curator/framework/imps/ReconfigBuilderImpl.java b/curator-framework/src/main/java/org/apache/curator/framework/imps/ReconfigBuilderImpl.java
index 0386e5e..c351dbf 100644
--- a/curator-framework/src/main/java/org/apache/curator/framework/imps/ReconfigBuilderImpl.java
+++ b/curator-framework/src/main/java/org/apache/curator/framework/imps/ReconfigBuilderImpl.java
@@ -130,7 +130,7 @@ public class ReconfigBuilderImpl implements ReconfigBuilder, BackgroundOperation
     @Override
     public StatConfigureEnsembleable withNewMembers(List<String> servers)
     {
-        newMembers = (servers != null) ? ImmutableList.copyOf(servers) : ImmutableList.<String>of();
+        newMembers = (servers != null && !servers.isEmpty()) ? ImmutableList.copyOf(servers) : null;
         return new StatConfigureEnsembleable()
         {
             @Override
@@ -164,7 +164,7 @@ public class ReconfigBuilderImpl implements ReconfigBuilder, BackgroundOperation
     @Override
     public LeaveStatConfigEnsembleable joining(List<String> servers)
     {
-        joining = (servers != null) ? ImmutableList.copyOf(servers) : ImmutableList.<String>of();
+        joining = (servers != null && !servers.isEmpty()) ? ImmutableList.copyOf(servers) : null;
 
         return new LeaveStatConfigEnsembleable()
         {
@@ -211,7 +211,7 @@ public class ReconfigBuilderImpl implements ReconfigBuilder, BackgroundOperation
     @Override
     public JoinStatConfigEnsembleable leaving(List<String> servers)
     {
-        leaving = (servers != null) ? ImmutableList.copyOf(servers) : ImmutableList.<String>of();
+        leaving = (servers != null && !servers.isEmpty()) ? ImmutableList.copyOf(servers) : null;
 
         return new JoinStatConfigEnsembleable()
         {
diff --git a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java
index 500e728..96fa384 100644
--- a/curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java
+++ b/curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java
@@ -51,6 +51,7 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Properties;
@@ -332,6 +333,54 @@ public class TestReconfiguration extends CuratorTestBase
         }
     }
 
+    @Test
+    public void testAddAndRemoveWithEmptyList() throws Exception
+    {
+        try ( CuratorFramework client = newClient())
+        {
+            client.start();
+
+            QuorumVerifier oldConfig = toQuorumVerifier(client.getConfig().forEnsemble());
+            assertConfig(oldConfig, cluster.getInstances());
+
+            CountDownLatch latch = setChangeWaiter(client);
+
+            Collection<InstanceSpec> oldInstances = cluster.getInstances();
+            client.reconfig().leaving(Collections.emptyList()).joining(Collections.emptyList()).fromConfig(oldConfig.getVersion()).forEnsemble();
+
+            Assert.assertTrue(timing.awaitLatch(latch));
+
+            byte[] newConfigData = client.getConfig().forEnsemble();
+            QuorumVerifier newConfig = toQuorumVerifier(newConfigData);
+            assertConfig(newConfig, oldInstances);
+            Assert.assertEquals(EnsembleTracker.configToConnectionString(newConfig), ensembleProvider.getConnectionString());
+        }
+    }
+
+    @Test
+    public void testNewMembersWithEmptyList() throws Exception
+    {
+        try ( CuratorFramework client = newClient())
+        {
+            client.start();
+
+            QuorumVerifier oldConfig = toQuorumVerifier(client.getConfig().forEnsemble());
+            assertConfig(oldConfig, cluster.getInstances());
+
+            CountDownLatch latch = setChangeWaiter(client);
+
+            Collection<InstanceSpec> oldInstances = cluster.getInstances();
+            client.reconfig().withNewMembers(Collections.emptyList()).fromConfig(oldConfig.getVersion()).forEnsemble();
+
+            Assert.assertTrue(timing.awaitLatch(latch));
+
+            byte[] newConfigData = client.getConfig().forEnsemble();
+            QuorumVerifier newConfig = toQuorumVerifier(newConfigData);
+            assertConfig(newConfig, oldInstances);
+            Assert.assertEquals(EnsembleTracker.configToConnectionString(newConfig), ensembleProvider.getConnectionString());
+        }
+    }
+
     @Test(enabled = false)  // it's what this test is inteded to do and it keeps failing - disable for now
     public void testNewMembers() throws Exception
     {
@@ -554,4 +603,4 @@ public class TestReconfiguration extends CuratorTestBase
         properties.load(new ByteArrayInputStream(bytes));
         return new QuorumMaj(properties);
     }
-}
\ No newline at end of file
+}