From 800fdf4c5cbe3f0e3cebc64357707ba743fc46e2 Mon Sep 17 00:00:00 2001
From: hjyun <hjyun@jam2in.com>
Date: Sat, 19 Sep 2020 16:50:42 +0900
Subject: [PATCH] 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 0386e5e22e..c351dbfd73 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 StatConfigureEnsembleable withNewMembers(String... server)
     @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 LeaveStatConfigEnsembleable joining(String... server)
     @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 JoinStatConfigEnsembleable leaving(String... server)
     @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 500e728e7d..96fa384303 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.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 void testAddAndRemove() throws Exception
         }
     }
 
+    @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 @@ private static QuorumVerifier toQuorumVerifier(byte[] bytes) throws Exception
         properties.load(new ByteArrayInputStream(bytes));
         return new QuorumMaj(properties);
     }
-}
\ No newline at end of file
+}