Fixed IPv6 parsing logic (#1342)

Introduced tests for HostAndPort

Introduced comments on IPv6 parsing logic

Introduced 'aliases' of localhost address
Conflicts:
	src/main/java/redis/clients/jedis/HostAndPort.java

Author: smagellan

src/main/java/redis/clients/jedis/HostAndPort.java

@@ -1,11 +1,16 @@
 package redis.clients.jedis;
 
 import java.io.Serializable;
+import java.net.InetAddress;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 public class HostAndPort implements Serializable {
   private static final long serialVersionUID = -519876229978427751L;
 
-  public static final String LOCALHOST_STR = "localhost";
+  protected static Logger log = Logger.getLogger(HostAndPort.class.getName());
+  public static final String LOCALHOST_STR = getLocalHostQuietly();
+
 
   private String host;
   private int port;
@@ -47,10 +52,61 @@ public String toString() {
     return host + ":" + port;
   }
 
-  private String convertHost(String host) {
-    if (host.equals("127.0.0.1")) return LOCALHOST_STR;
-    else if (host.equals("::1")) return LOCALHOST_STR;
+  /**
+   * Splits String into host and port parts.
+   * String must be in ( host + ":" + port ) format.
+   * Port is optional
+   * @param from String to parse
+   * @return array of host and port strings
+     */
+  public static String[] extractParts(String from){
+    int idx     = from.lastIndexOf(":");
+    String host = idx != -1 ? from.substring(0, idx)  : from;
+    String port = idx != -1 ? from.substring(idx + 1) : "";
+    return new String[] { host, port };
+  }
 
-    return host;
+  /**
+   * Creates HostAndPort instance from string.
+   * String must be in ( host + ":" + port ) format.
+   * Port is mandatory. Can convert host part.
+   * @see #convertHost(String)
+   * @param from String to parse
+   * @return HostAndPort instance
+     */
+  public static HostAndPort parseString(String from){
+    // NOTE: redis answers with
+    // '99aa9999aa9a99aa099aaa990aa99a09aa9a9999 9a09:9a9:a090:9a::99a slave 8c88888888cc08088cc8c8c888c88c8888c88cc8 0 1468251272993 37 connected'
+    // for CLUSTER NODES, ASK and MOVED scenarios. That's why there is no possibility to parse address in 'correct' way.
+    // Redis should switch to 'bracketized' (RFC 3986) IPv6 address.
+    try {
+      String[] parts = extractParts(from);
+      String host = parts[0];
+      int port = Integer.valueOf(parts[1]);
+      return new HostAndPort(convertHost(host), port);
+    } catch (NumberFormatException ex) {
+      throw new IllegalArgumentException(ex);
+    }
+  }
+
+  public static String convertHost(String host) {
+    if (host.equals("127.0.0.1") || host.startsWith("localhost") || host.equals("0.0.0.0") ||
+        host.startsWith("169.254") ||
+        host.startsWith("::1") || host.startsWith("0:0:0:0:0:0:0:1")) {
+      return LOCALHOST_STR;
+    } else {
+      return host;
+    }
+  }
+
+  public static String getLocalHostQuietly() {
+    String localAddress;
+    try {
+      localAddress = InetAddress.getLocalHost().getHostAddress();
+    } catch (Exception ex) {
+      log.logp(Level.SEVERE, HostAndPort.class.getName(), "getLocalHostQuietly", "cant resolve localhost address", ex);
+      localAddress = "localhost";
+    }
+    return localAddress;
   }
 }

src/main/java/redis/clients/jedis/JedisSentinelPool.java

@@ -136,7 +136,7 @@ private HostAndPort initSentinels(Set<String> sentinels, final String masterName
     log.info("Trying to find master from available Sentinels...");
 
     for (String sentinel : sentinels) {
-      final HostAndPort hap = toHostAndPort(Arrays.asList(sentinel.split(":")));
+      final HostAndPort hap = HostAndPort.parseString(sentinel);
 
       log.fine("Connecting to Sentinel " + hap);
 
@@ -185,7 +185,7 @@ private HostAndPort initSentinels(Set<String> sentinels, final String masterName
     log.info("Redis master running at " + master + ", starting Sentinel listeners...");
 
     for (String sentinel : sentinels) {
-      final HostAndPort hap = toHostAndPort(Arrays.asList(sentinel.split(":")));
+      final HostAndPort hap = HostAndPort.parseString(sentinel);
       MasterListener masterListener = new MasterListener(masterName, hap.getHost(), hap.getPort());
       // whether MasterListener threads are alive or not, process can be stopped
       masterListener.setDaemon(true);

src/main/java/redis/clients/jedis/Protocol.java

@@ -135,7 +135,7 @@ public static String readErrorLineIfPossible(RedisInputStream is) {
   private static String[] parseTargetHostAndSlot(String clusterRedirectResponse) {
     String[] response = new String[3];
     String[] messageInfo = clusterRedirectResponse.split(" ");
-    String[] targetHostAndPort = messageInfo[2].split(":");
+    String[] targetHostAndPort = HostAndPort.extractParts(messageInfo[2]);
     response[0] = messageInfo[1];
     response[1] = targetHostAndPort[0];
     response[2] = targetHostAndPort[1];

src/test/java/redis/clients/jedis/HostAndPortTest.java

@@ -0,0 +1,60 @@
+package redis.clients.jedis;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+
+import static org.junit.Assert.*;
+
+/**
+ * Created by smagellan on 7/11/16.
+ */
+public class HostAndPortTest {
+  @Test
+  public void checkExtractParts() throws Exception {
+    String host = "2a11:1b1:0:111:e111:1f11:1111:1f1e:1999";
+    String port = "6379";
+
+    Assert.assertEquals(Arrays.asList(HostAndPort.extractParts(host + ":" + port)),
+            Arrays.asList(host, port));
+
+    host = "";
+    port = "";
+    Assert.assertEquals(Arrays.asList(HostAndPort.extractParts(host + ":" + port)),
+            Arrays.asList(host, port));
+
+    host = "localhost";
+    port = "";
+    Assert.assertEquals(Arrays.asList(HostAndPort.extractParts(host + ":" + port)),
+            Arrays.asList(host, port));
+
+
+    host = "";
+    port = "6379";
+    Assert.assertEquals(Arrays.asList(HostAndPort.extractParts(host + ":" + port)),
+            Arrays.asList(host, port));
+
+    host = "11:22:33:44:55";
+    port = "";
+    Assert.assertEquals(Arrays.asList(HostAndPort.extractParts(host + ":" + port)),
+            Arrays.asList(host, port));
+  }
+
+  @Test
+  public void checkParseString() throws Exception {
+    String host = "2a11:1b1:0:111:e111:1f11:1111:1f1e:1999";
+    int port = 6379;
+    HostAndPort hp = HostAndPort.parseString(host + ":" + Integer.toString(port));
+    Assert.assertEquals(host, hp.getHost());
+    Assert.assertEquals(port, hp.getPort());
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void checkParseStringWithoutPort() throws Exception {
+    String host = "localhost";
+    HostAndPort.parseString(host + ":");
+  }
+}

src/test/java/redis/clients/jedis/tests/HostAndPortUtil.java

@@ -58,14 +58,14 @@ public static List<HostAndPort> parseHosts(String envHosts,
 
         for (String hostDef : hostDefs) {
 
-          String[] hostAndPort = hostDef.split(":");
+          String[] hostAndPortParts = HostAndPort.extractParts(hostDef);
 
-          if (null != hostAndPort && 2 == hostAndPort.length) {
-            String host = hostAndPort[0];
+          if (null != hostAndPortParts && 2 == hostAndPortParts.length) {
+            String host = hostAndPortParts[0];
             int port = Protocol.DEFAULT_PORT;
 
             try {
-              port = Integer.parseInt(hostAndPort[1]);
+              port = Integer.parseInt(hostAndPortParts[1]);
             } catch (final NumberFormatException nfe) {
             }
 

src/test/java/redis/clients/jedis/tests/utils/ClientKillerUtil.java

@@ -1,16 +1,18 @@
 package redis.clients.jedis.tests.utils;
 
+import redis.clients.jedis.HostAndPort;
 import redis.clients.jedis.Jedis;
 
 public class ClientKillerUtil {
   public static void killClient(Jedis jedis, String clientName) {
     for (String clientInfo : jedis.clientList().split("\n")) {
       if (clientInfo.contains("name=" + clientName)) {
         // Ugly, but cmon, it's a test.
-        String[] hostAndPort = clientInfo.split(" ")[1].split("=")[1].split(":");
+        String hostAndPortString  = clientInfo.split(" ")[1].split("=")[1];
+        String[] hostAndPortParts = HostAndPort.extractParts(hostAndPortString);
         // It would be better if we kill the client by Id as it's safer but jedis doesn't implement
         // the command yet.
-        jedis.clientKill(hostAndPort[0] + ":" + hostAndPort[1]);
+        jedis.clientKill(hostAndPortParts[0] + ":" + hostAndPortParts[1]);
       }
     }
   }