ZOOKEEPER-3878: Client connection fails if IPV6 is not enclosed in sq…

…uare brackets Author: Mohammad Arshad 72626 <mohammad.arshad@huawei.com> Reviewers: Enrico Olivelli <eolivelli@apache.org> Closes #1398 from arshadmohammad/ZOOKEEPER-3878-master
apache · Jul 13, 2020 · f362313 · f362313
1 parent 6ab1822
commit f362313
Show file tree

Hide file tree

Showing 4 changed files with 123 additions and 7 deletions.
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/client/ConnectStringParser.java b/zookeeper-server/src/main/java/org/apache/zookeeper/client/ConnectStringParser.java
@@ -22,9 +22,8 @@
 import java.net.InetSocketAddress;
 import java.util.ArrayList;
 import java.util.List;
+import org.apache.zookeeper.common.NetUtils;
 import org.apache.zookeeper.common.PathUtils;
-import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
-import org.apache.zookeeper.server.util.ConfigUtils;
 
 /**
  * A parser for ZooKeeper Client connect strings.
@@ -70,16 +69,22 @@ public ConnectStringParser(String connectString) {
         List<String> hostsList = split(connectString, ",");
         for (String host : hostsList) {
             int port = DEFAULT_PORT;
-            try {
-                String[] hostAndPort = ConfigUtils.getHostAndPort(host);
+            String[] hostAndPort = NetUtils.getIPV6HostAndPort(host);
+            if (hostAndPort.length != 0) {
                 host = hostAndPort[0];
                 if (hostAndPort.length == 2) {
                     port = Integer.parseInt(hostAndPort[1]);
                 }
-            } catch (ConfigException e) {
-                e.printStackTrace();
+            } else {
+                int pidx = host.lastIndexOf(':');
+                if (pidx >= 0) {
+                    // otherwise : is at the end of the string, ignore
+                    if (pidx < host.length() - 1) {
+                        port = Integer.parseInt(host.substring(pidx + 1));
+                    }
+                    host = host.substring(0, pidx);
+                }
             }
-
             serverAddresses.add(InetSocketAddress.createUnresolved(host, port));
         }
     }

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/common/NetUtils.java b/zookeeper-server/src/main/java/org/apache/zookeeper/common/NetUtils.java
@@ -41,4 +41,48 @@ public static String formatInetAddr(InetSocketAddress addr) {
         }
     }
 
+    /**
+     * Separates host and port from given host port string if host port string is enclosed
+     * within square bracket.
+     *
+     * @param hostPort host port string
+     * @return String[]{host, port} if host port string is host:port
+     * or String[] {host, port:port} if host port string is host:port:port
+     * or String[] {host} if host port string is host
+     * or String[]{} if not a ipv6 host port string.
+     */
+    public static String[] getIPV6HostAndPort(String hostPort) {
+        if (hostPort.startsWith("[")) {
+            int i = hostPort.lastIndexOf(']');
+            if (i < 0) {
+                throw new IllegalArgumentException(
+                    hostPort + " starts with '[' but has no matching ']'");
+            }
+            String host = hostPort.substring(1, i);
+            if (host.isEmpty()) {
+                throw new IllegalArgumentException(host + " is empty.");
+            }
+            if (hostPort.length() > i + 1) {
+                return getHostPort(hostPort, i, host);
+            }
+            return new String[] { host };
+        } else {
+            //Not an IPV6 host port string
+            return new String[] {};
+        }
+    }
+
+    private static String[] getHostPort(String hostPort, int indexOfClosingBracket, String host) {
+        // [127::1]:2181 , check separator : exits
+        if (hostPort.charAt(indexOfClosingBracket + 1) != ':') {
+            throw new IllegalArgumentException(hostPort + " does not have : after ]");
+        }
+        // [127::1]: scenario
+        if (indexOfClosingBracket + 2 == hostPort.length()) {
+            throw new IllegalArgumentException(hostPort + " doesn't have a port after colon.");
+        }
+        //do not include
+        String port = hostPort.substring(indexOfClosingBracket + 2);
+        return new String[] { host, port };
+    }
 }
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/common/NetUtilsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/common/NetUtilsTest.java
@@ -69,4 +69,49 @@ public void testFormatAddrUnresolved() {
         assertEquals("doesnt.exist.com:1234", NetUtils.formatInetAddr(isa));
     }
 
+    @Test(expected = IllegalArgumentException.class)
+    public void tetGetIPV6HostAndPort_WhenHostDoesNotEndWithBracket() {
+        NetUtils.getIPV6HostAndPort("[2001:0db8:85a3:0000:0000:8a2e:0370:7334:443");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void tetGetIPV6HostAndPort_WhenNoPortAfterColon() {
+        NetUtils.getIPV6HostAndPort("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void tetGetIPV6HostAndPort_WhenPortIsNotSeparatedProperly() {
+        NetUtils.getIPV6HostAndPort("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]2181");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void tetGetIPV6HostAndPort_WhenHostIsEmpty() {
+        NetUtils.getIPV6HostAndPort("[]:2181");
+    }
+
+    @Test
+    public void tetGetIPV6HostAndPort_EmptyStringArrayIfDoesNotStartWithBracket() {
+        String[] ipv6HostAndPort =
+            NetUtils.getIPV6HostAndPort("2001:0db8:85a3:0000:0000:8a2e:0370:7334]");
+        assertEquals(0, ipv6HostAndPort.length);
+    }
+
+    @Test
+    public void tetGetIPV6HostAndPort_ReturnHostPort() {
+        String[] ipv6HostAndPort =
+            NetUtils.getIPV6HostAndPort("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:2181");
+        assertEquals(2, ipv6HostAndPort.length);
+        assertEquals("2001:0db8:85a3:0000:0000:8a2e:0370:7334", ipv6HostAndPort[0]);
+        assertEquals("2181", ipv6HostAndPort[1]);
+    }
+
+    @Test
+    public void tetGetIPV6HostAndPort_ReturnHostPortPort() {
+        String[] ipv6HostAndPort =
+            NetUtils.getIPV6HostAndPort("[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:2181:3181");
+        assertEquals(2, ipv6HostAndPort.length);
+        assertEquals("2001:0db8:85a3:0000:0000:8a2e:0370:7334", ipv6HostAndPort[0]);
+        assertEquals("2181:3181", ipv6HostAndPort[1]);
+    }
+
 }
diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ConnectStringParserTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ConnectStringParserTest.java
@@ -21,6 +21,7 @@
 import static org.junit.Assert.assertEquals;
 import org.apache.zookeeper.ZKTestCase;
 import org.apache.zookeeper.client.ConnectStringParser;
+import org.junit.Assert;
 import org.junit.Test;
 
 public class ConnectStringParserTest extends ZKTestCase {
@@ -85,4 +86,25 @@ private void assertChrootPath(String expected, ConnectStringParser parser) {
         assertEquals(expected, parser.getChrootPath());
     }
 
+    @Test
+    public void testParseIPV6ConnectionString() {
+        String servers = "[127::1],127.0.10.2";
+        ConnectStringParser parser = new ConnectStringParser(servers);
+
+        Assert.assertEquals("127::1", parser.getServerAddresses().get(0).getHostString());
+        Assert.assertEquals("127.0.10.2", parser.getServerAddresses().get(1).getHostString());
+        Assert.assertEquals(2181, parser.getServerAddresses().get(0).getPort());
+        Assert.assertEquals(2181, parser.getServerAddresses().get(1).getPort());
+
+        servers = "[127::1]:2181,[127::2]:2182,[127::3]:2183";
+        parser = new ConnectStringParser(servers);
+
+        Assert.assertEquals("127::1", parser.getServerAddresses().get(0).getHostString());
+        Assert.assertEquals("127::2", parser.getServerAddresses().get(1).getHostString());
+        Assert.assertEquals("127::3", parser.getServerAddresses().get(2).getHostString());
+        Assert.assertEquals(2181, parser.getServerAddresses().get(0).getPort());
+        Assert.assertEquals(2182, parser.getServerAddresses().get(1).getPort());
+        Assert.assertEquals(2183, parser.getServerAddresses().get(2).getPort());
+    }
+
 }