Merge branch 'php7-ipv6' into php7

8 files changed, 89 insertions, 53 deletions

diff --git a/cluster_library.c b/cluster_library.c
index 38e4827a..b7b299a8 100644
--- a/cluster_library.c
+++ b/cluster_library.c
@@ -846,8 +846,9 @@ cluster_init_seeds(redisCluster *cluster, HashTable *ht_seeds) {
         // Grab a copy of the string
         str = Z_STRVAL_P(z_seed);
 
-        // Must be in host:port form
-        if(!(psep = strchr(str, ':')))
+        /* Make sure we have a colon for host:port.  Search right to left in the
+         * case of IPv6 */
+        if ((psep = strrchr(str, ':')) == NULL)
             continue;
 
         // Allocate a structure for this seed
@@ -922,12 +923,12 @@ static int cluster_set_redirection(redisCluster* c, char *msg, int moved)
     /* Move past "MOVED" or "ASK */
     msg += moved ? MOVED_LEN : ASK_LEN;
 
-    // We need a slot seperator
-    if(!(host = strchr(msg, ' '))) return -1;
+    /* Make sure we can find host */
+    if ((host = strchr(msg, ' ')) == NULL) return -1;
     *host++ = '\0';
 
-    // We need a : that seperates host from port
-    if(!(port = strchr(host,':'))) return -1;
+    /* Find port, searching right to left in case of IPv6 */
+    if ((port = strrchr(host, ':')) == NULL) return -1;
     *port++ = '\0';
 
     // Success, apply it
diff --git a/library.c b/library.c
index 883992e7..4628a06b 100644
--- a/library.c
+++ b/library.c
@@ -1563,6 +1563,7 @@ PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC)
     struct timeval tv, read_tv, *tv_ptr = NULL;
     char *host = NULL, *persistent_id = NULL;
 	zend_string *errstr;
+    const char *fmtstr = "%s:%d";
     int host_len, err = 0;
     php_netstream_data_t *sock;
     int tcp_flag = 1;
@@ -1585,8 +1586,15 @@ PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC)
     } else {
         if(redis_sock->port == 0)
             redis_sock->port = 6379;
-        host_len = spprintf(&host, 0, "%s:%d", redis_sock->host,
-            redis_sock->port);
+
+#ifdef HAVE_IPV6
+        /* If we've got IPv6 and find a colon in our address, convert to proper
+         * IPv6 [host]:port format */
+        if (strchr(redis_sock->host, ':') != NULL) {
+            fmtstr = "[%s]:%d";
+        }
+#endif
+        host_len = spprintf(&host, 0, fmtstr, redis_sock->host, redis_sock->port);
     }
 
     if (redis_sock->persistent) {
diff --git a/redis_array_impl.c b/redis_array_impl.c
index bb229dd0..f6650a8b 100644
--- a/redis_array_impl.c
+++ b/redis_array_impl.c
@@ -61,13 +61,12 @@ ra_load_hosts(RedisArray *ra, HashTable *hosts, long retry_interval, zend_bool b
         host_len = Z_STRLEN_P(zpData);
         port = 6379;
 
-        /* Split host:port by colon */
-        if ((p = memchr(host, ':', host_len))) {
-            host_len = p - host;
-            port = (short)atoi(p+1);
-        } else if(strchr(host,'/') != NULL) { /* unix socket */
-            port = -1;
-        }
+		if(((p = strrchr(host, ':')))) { /* found port */
+			host_len = p - host;
+			port = (short)atoi(p+1);
+		} else if(strchr(host,'/') != NULL) { /* unix socket */
+		    port = -1;
+		}
 
         /* create Redis object */
         object_init_ex(&ra->redis[i], redis_ce);
diff --git a/tests/RedisArrayTest.php b/tests/RedisArrayTest.php
index e5c18ac4..6a1025b0 100644
--- a/tests/RedisArrayTest.php
+++ b/tests/RedisArrayTest.php
@@ -12,6 +12,12 @@ function custom_hash($str) {
     return $str;
 }
 
+function parseHostPort($str, &$host, &$port) {
+    $pos = strrpos($str, ':');
+    $host = substr($str, 0, $pos);
+    $port = substr($str, $pos+1);
+}
+
 class Redis_Array_Test extends TestSuite
 {
     private $strings;
@@ -19,7 +25,6 @@ class Redis_Array_Test extends TestSuite
     private $data = NULL;
 
     public function setUp() {
-
         // initialize strings.
         $n = REDIS_ARRAY_DATA_SIZE;
         $this->strings = array();
@@ -42,7 +47,13 @@ class Redis_Array_Test extends TestSuite
         
         // check each key individually using a new connection
         foreach($this->strings as $k => $v) {
-            list($host, $port) = explode(':', $this->ra->_target($k));
+            parseHostPort($this->ra->_target($k), $host, $port);
+
+            $target = $this->ra->_target($k);
+            $pos = strrpos($target, ':');
+
+            $host = substr($target, 0, $pos);
+            $port = substr($target, $pos+1);
 
             $r = new Redis;
             $r->pconnect($host, (int)$port);
@@ -180,7 +191,7 @@ class Redis_Rehashing_Test extends TestSuite
         // flush all servers first.
         global $serverList;
         foreach($serverList as $s) {
-            list($host, $port) = explode(':', $s);
+            parseHostPort($s, $host, $port);
 
             $r = new Redis();
             $r->pconnect($host, (int)$port, 0);
@@ -351,11 +362,8 @@ class Redis_Auto_Rehashing_Test extends TestSuite {
     // Read and migrate keys on fallback, causing the whole ring to be rehashed.
     public function testAllKeysHaveBeenMigrated() {
         foreach($this->strings as $k => $v) {
-            // get the target for each key
-            $target = $this->ra->_target($k);
+            parseHostPort($this->ra->_target($k), $host, $port);
 
-            // connect to the target host
-            list($host,$port) = explode(':', $target);
             $r = new Redis;
             $r->pconnect($host, $port);
 
@@ -528,12 +536,13 @@ class Redis_Distributor_Test extends TestSuite {
     }
 }
 
-function run_tests($className, $str_filter) {
+function run_tests($className, $str_filter, $str_host) {
         // reset rings
         global $newRing, $oldRing, $serverList;
-        $newRing = array('localhost:6379', 'localhost:6380', 'localhost:6381');
-        $oldRing = array();
-        $serverList = array('localhost:6379', 'localhost:6380', 'localhost:6381', 'localhost:6382');
+
+        $newRing = Array("$str_host:6379", "$str_host:6380", "$str_host:6381");
+        $oldRing = Array();
+        $serverList = Array("$str_host:6379", "$str_host:6380", "$str_host:6381", "$str_host:6382");
 
         // run
         TestSuite::run($className, $str_filter);
diff --git a/tests/RedisTest.php b/tests/RedisTest.php
index 9111e3bf..b4e2b387 100644
--- a/tests/RedisTest.php
+++ b/tests/RedisTest.php
@@ -4,7 +4,6 @@ require_once(dirname($_SERVER['PHP_SELF'])."/TestSuite.php");
 
 class Redis_Test extends TestSuite
 {
-    const HOST = '127.0.0.1';
     const PORT = 6379;
     const AUTH = NULL; //replace with a string to use Redis authentication
 
@@ -21,7 +20,8 @@ class Redis_Test extends TestSuite
 
     protected function newInstance() {
         $r = new Redis();
-        $r->connect(self::HOST, self::PORT);
+
+        $r->connect($this->getHost(), self::PORT);
 
         if(self::AUTH) {
             $this->assertTrue($r->auth(self::AUTH));
@@ -4485,7 +4485,7 @@ class Redis_Test extends TestSuite
 
     public function testIntrospection() {
         // Simple introspection tests
-        $this->assertTrue($this->redis->getHost() === self::HOST);
+        $this->assertTrue($this->redis->getHost() === $this->getHost());
         $this->assertTrue($this->redis->getPort() === self::PORT);
         $this->assertTrue($this->redis->getAuth() === self::AUTH);
     }
diff --git a/tests/TestRedis.php b/tests/TestRedis.php
index 14bab8ae..d2236372 100644
--- a/tests/TestRedis.php
+++ b/tests/TestRedis.php
@@ -10,7 +10,7 @@ error_reporting(E_ALL);
 ini_set( 'display_errors','1');
 
 /* Grab options */
-$arr_args = getopt('', Array('class:', 'test:', 'nocolors'));
+$arr_args = getopt('', Array('host:', 'class:', 'test:', 'nocolors'));
 
 /* Grab the test the user is trying to run */
 $arr_valid_classes = Array('redis', 'redisarray', 'rediscluster');
@@ -20,6 +20,9 @@ $boo_colorize = !isset($arr_args['nocolors']);
 /* Get our test filter if provided one */
 $str_filter = isset($arr_args['test']) ? $arr_args['test'] : NULL;
 
+/* Grab override test host if it was passed */
+$str_host = isset($arr_args['host']) ? $arr_args['host'] : '127.0.0.1';
+
 /* Validate the class is known */
 if (!in_array($str_class, $arr_valid_classes)) {
     echo "Error:  Valid test classes are Redis, RedisArray, and RedisCluster!\n";
@@ -36,21 +39,21 @@ echo "Note: these tests might take up to a minute. Don't worry :-)\n";
 echo "Testing class ";
 if ($str_class == 'redis') {
     echo TestSuite::make_bold("Redis") . "\n";
-    exit(TestSuite::run("Redis_Test", $str_filter));
+    exit(TestSuite::run("Redis_Test", $str_filter, $str_host));
 } else if ($str_class == 'redisarray') {
     echo TestSuite::make_bold("RedisArray") . "\n";
     global $useIndex;
     foreach(array(true, false) as $useIndex) {
         echo "\n".($useIndex?"WITH":"WITHOUT"). " per-node index:\n";
 
-        run_tests('Redis_Array_Test', $str_filter);
-        run_tests('Redis_Rehashing_Test', $str_filter);
-        run_tests('Redis_Auto_Rehashing_Test', $str_filter);
-        run_tests('Redis_Multi_Exec_Test', $str_filter);
-        run_tests('Redis_Distributor_Test', $str_filter);
+        run_tests('Redis_Array_Test', $str_filter, $str_host);
+        run_tests('Redis_Rehashing_Test', $str_filter, $str_host);
+        run_tests('Redis_Auto_Rehashing_Test', $str_filter, $str_host);
+        run_tests('Redis_Multi_Exec_Test', $str_filter, $str_host);
+        run_tests('Redis_Distributor_Test', $str_filter, $str_host);
     }
 } else {
     echo TestSuite::make_bold("RedisCluster") . "\n";
-    exit(TestSuite::run("Redis_Cluster_Test", $str_filter));
+    exit(TestSuite::run("Redis_Cluster_Test", $str_filter, $str_host));
 }
 ?>
diff --git a/tests/TestSuite.php b/tests/TestSuite.php
index ecdefbd0..5bbd98bc 100644
--- a/tests/TestSuite.php
+++ b/tests/TestSuite.php
@@ -2,6 +2,9 @@
 
 // phpunit is such a pain to install, we're going with pure-PHP here.
 class TestSuite {
+    /* Host the tests will use */
+    private $str_host;
+
     private static $_boo_colorize = false;
 
     private static $BOLD_ON = "\033[1m";
@@ -18,8 +21,14 @@ class TestSuite {
     public static $errors = array();
     public static $warnings = array();
 
+    public function __construct($str_host) {
+        $this->str_host = $str_host;
+    }
+
+    public function getHost() { return $this->str_host; }
+
     public static function make_bold($str_msg) {
-        return self::$_boo_colorize 
+        return self::$_boo_colorize
             ? self::$BOLD_ON . $str_msg . self::$BOLD_OFF
             : $str_msg;
     }
@@ -56,7 +65,7 @@ class TestSuite {
     }
 
     protected function assertLess($a, $b) {
-        if($a < $b) 
+        if($a < $b)
             return;
 
         $bt = debug_backtrace(false);
@@ -85,12 +94,12 @@ class TestSuite {
 
     private static function getMaxTestLen($arr_methods, $str_limit) {
         $i_result = 0;
-        
+
         $str_limit = strtolower($str_limit);
         foreach ($arr_methods as $obj_method) {
             $str_name = strtolower($obj_method->name);
 
-            if (substr($str_name, 0, 4) != 'test') 
+            if (substr($str_name, 0, 4) != 'test')
                 continue;
             if ($str_limit && !strstr($str_name, $str_limit))
                 continue;
@@ -101,14 +110,14 @@ class TestSuite {
         }
         return $i_result;
     }
-    
+
     /* Flag colorization */
     public static function flagColorization($boo_override) {
         self::$_boo_colorize = $boo_override && function_exists('posix_isatty') &&
             posix_isatty(STDOUT);
     }
 
-    public static function run($className, $str_limit = NULL) {
+    public static function run($className, $str_limit = NULL, $str_host = NULL) {
         /* Lowercase our limit arg if we're passed one */
         $str_limit = $str_limit ? strtolower($str_limit) : $str_limit;
 
@@ -121,7 +130,7 @@ class TestSuite {
             $name = $m->name;
             if(substr($name, 0, 4) !== 'test')
                 continue;
-            
+
             /* If we're trying to limit to a specific test and can't match the
              * substring, skip */
             if ($str_limit && strstr(strtolower($name), $str_limit)===FALSE) {
@@ -132,7 +141,8 @@ class TestSuite {
             echo self::make_bold($str_out_name);
 
             $count = count($className::$errors);
-            $rt = new $className();
+            $rt = new $className($str_host);
+
             try {
                 $rt->setUp();
                 $rt->$name();
diff --git a/tests/make-cluster.sh b/tests/make-cluster.sh
index 64061486..b3a91086 100755
--- a/tests/make-cluster.sh
+++ b/tests/make-cluster.sh
@@ -5,15 +5,16 @@
 # simplifying the process of running unit tests.
 #
 # Usage:
-#   ./make-cluster.sh start
-#   ./make-cluster.sh stop
+#   ./make-cluster.sh start [host]
+#   ./make-cluster.sh stop [host]
 #
 
-BASEDIR=`dirname $@`
+BASEDIR=`pwd`
 NODEDIR=$BASEDIR/nodes
 MAPFILE=$NODEDIR/nodemap
 
-# Nodes, replicas, ports, etc.  Change if you want different values
+# Host, nodes, replicas, ports, etc.  Change if you want different values
+HOST="127.0.0.1"
 NODES=12
 REPLICAS=3
 START_PORT=7000
@@ -38,7 +39,7 @@ spawnNode() {
     # Attempt to spawn the node
     verboseRun redis-server --cluster-enabled yes --dir $NODEDIR --port $PORT \
         --cluster-config-file node-$PORT.conf --daemonize yes --save \'\' \
-        --bind '127.0.0.1' --dbfilename node-$PORT.rdb
+        --bind $HOST --dbfilename node-$PORT.rdb
 
     # Abort if we can't spin this instance
     if [ $? -ne 0 ]; then 
@@ -54,7 +55,7 @@ spawnNodes() {
         spawnNode $PORT
 
         # Add this host:port to our nodemap so the tests can get seeds
-        echo "127.0.0.1:$PORT" >> $MAPFILE
+        echo "$HOST:$PORT" >> $MAPFILE
     done
 }
 
@@ -85,7 +86,7 @@ cleanConfigInfo() {
 initCluster() {
     TRIBARGS=""
     for PORT in `seq $START_PORT $END_PORT`; do
-        TRIBARGS="$TRIBARGS 127.0.0.1:$PORT"
+        TRIBARGS="$TRIBARGS $HOST:$PORT"
     done
 
     verboseRun redis-trib.rb create --replicas $REPLICAS $TRIBARGS
@@ -122,6 +123,11 @@ stopCluster() {
 checkExe redis-server
 checkExe redis-trib.rb
 
+# Override the host if we've got $2
+if [[ ! -z "$2" ]]; then
+   HOST=$2
+fi
+
 # Main entry point to start or stop/kill a cluster
 case "$1" in
     start)
@@ -131,6 +137,6 @@ case "$1" in
         stopCluster
         ;;
     *)
-        echo "Usage $0 [start|stop]"
+        echo "Usage $0 <start|stop> [host]"
         ;;
 esac