Merge branch 'develop' into 4.2.0RC2

6 files changed, 62 insertions, 34 deletions

diff --git a/README.markdown b/README.markdown
index 23c9b257..5dbea00f 100644
--- a/README.markdown
+++ b/README.markdown
@@ -64,6 +64,10 @@ The session handler requires a version of Redis with the `SETEX` command (at lea
 phpredis can also connect to a unix domain socket: `session.save_path = "unix:///var/run/redis/redis.sock?persistent=1&weight=1&database=0`.
 
 ### Session locking
+
+**Support**: Locking feature is currently only supported for Redis setup with single master instance (e.g. classic master/slave Sentinel environment).
+So locking may not work properly in RedisArray or RedisCluster environments.
+
 Following INI variables can be used to configure session locking:
 ~~~
 ; Should the locking be enabled? Defaults to: 0.
diff --git a/cluster_library.c b/cluster_library.c
index 224932bf..2159a2e6 100644
--- a/cluster_library.c
+++ b/cluster_library.c
@@ -1419,8 +1419,11 @@ PHP_REDIS_API short cluster_send_command(redisCluster *c, short slot, const char
             return -1;
         }
 
-        /* Now check the response from the node we queried. */
+        /* Check response and short-circuit on success or communication error */
         resp = cluster_check_response(c, &c->reply_type TSRMLS_CC);
+        if (resp <= 0) {
+            break;
+        }
 
         /* Handle MOVED or ASKING redirection */
         if (resp == 1) {
@@ -1439,21 +1442,28 @@ PHP_REDIS_API short cluster_send_command(redisCluster *c, short slot, const char
            }
         }
 
-        /* Figure out if we've timed out trying to read or write the data */
-        timedout = resp && c->waitms ? mstime() - msstart >= c->waitms : 0;
-    } while (resp != 0 && !c->clusterdown && !timedout);
+        /* See if we've timed out in the command loop */
+        timedout = c->waitms ? mstime() - msstart >= c->waitms : 0;
+    } while (!c->clusterdown && !timedout);
 
     // If we've detected the cluster is down, throw an exception
     if (c->clusterdown) {
         zend_throw_exception(redis_cluster_exception_ce,
             "The Redis Cluster is down (CLUSTERDOWN)", 0 TSRMLS_CC);
         return -1;
-    } else if (timedout) {
+    } else if (timedout || resp == -1) {
         // Make sure the socket is reconnected, it such that it is in a clean state
         redis_sock_disconnect(c->cmd_sock, 1 TSRMLS_CC);
 
-        zend_throw_exception(redis_cluster_exception_ce,
-            "Timed out attempting to find data in the correct node!", 0 TSRMLS_CC);
+        if (timedout) {
+            zend_throw_exception(redis_cluster_exception_ce,
+                "Timed out attempting to find data in the correct node!", 0 TSRMLS_CC);
+        } else {
+            zend_throw_exception(redis_cluster_exception_ce,
+                "Error processing response from Redis node!", 0 TSRMLS_CC);
+        }
+
+        return -1;
     }
 
     /* Clear redirection flag */
diff --git a/library.c b/library.c
index 8fecaef1..d5752f0d 100644
--- a/library.c
+++ b/library.c
@@ -1269,17 +1269,18 @@ redis_read_stream_messages(RedisSock *redis_sock, int count, zval *z_ret
 {
     zval zv, *z_message = &zv;
     int i, mhdr, fields;
-    char id[1024];
-    size_t idlen;
+    char *id = NULL;
+    int idlen;
 
     /* Iterate over each message */
     for (i = 0; i < count; i++) {
         /* Consume inner multi-bulk header, message ID itself and finaly
          * the multi-bulk header for field and values */
         if ((read_mbulk_header(redis_sock, &mhdr TSRMLS_CC) < 0 || mhdr != 2) ||
-            redis_sock_read_single_line(redis_sock, id, sizeof(id), &idlen, 0 TSRMLS_CC) < 0 ||
+            ((id = redis_sock_read(redis_sock, &idlen TSRMLS_CC)) == NULL) ||
             (read_mbulk_header(redis_sock, &fields TSRMLS_CC) < 0 || fields % 2 != 0))
         {
+            if (id) efree(id);
             return -1;
         }
 
@@ -1289,6 +1290,7 @@ redis_read_stream_messages(RedisSock *redis_sock, int count, zval *z_ret
         redis_mbulk_reply_loop(redis_sock, z_message, fields, UNSERIALIZE_VALS TSRMLS_CC);
         array_zip_values_and_scores(redis_sock, z_message, SCORE_DECODE_NONE TSRMLS_CC);
         add_assoc_zval_ex(z_ret, id, idlen, z_message);
+        efree(id);
     }
 
     return 0;
@@ -1404,24 +1406,30 @@ PHP_REDIS_API int
 redis_read_xclaim_response(RedisSock *redis_sock, int count, zval *rv TSRMLS_DC) {
     zval zv, *z_msg = &zv;
     REDIS_REPLY_TYPE type;
-    char id[1024];
-    int i, fields;
+    char *id;
+    int i, fields, idlen;
     long li;
-    size_t idlen;
 
     for (i = 0; i < count; i++) {
         /* Consume inner reply type */
         if (redis_read_reply_type(redis_sock, &type, &li TSRMLS_CC) < 0 ||
-            (type != TYPE_LINE && type != TYPE_MULTIBULK)) return -1;
+            (type != TYPE_BULK && type != TYPE_MULTIBULK) ||
+            (type == TYPE_BULK && li <= 0)) return -1;
 
-        if (type == TYPE_LINE) {
-            /* JUSTID variant */
-            if (redis_sock_gets(redis_sock, id, sizeof(id), &idlen TSRMLS_CC) < 0)
+        /* TYPE_BULK is the JUSTID variant, otherwise it's standard xclaim response */
+        if (type == TYPE_BULK) {
+            if ((id = redis_sock_read_bulk_reply(redis_sock, (size_t)li TSRMLS_CC)) == NULL)
                 return -1;
-            add_next_index_stringl(rv, id, idlen);
+
+            add_next_index_stringl(rv, id, li);
+            efree(id);
         } else {
-            if (li != 2 || redis_sock_read_single_line(redis_sock, id, sizeof(id), &idlen, 0 TSRMLS_CC) < 0 ||
-                (read_mbulk_header(redis_sock, &fields TSRMLS_CC) < 0 || fields % 2 != 0)) return -1;
+            if ((li != 2 || (id = redis_sock_read(redis_sock, &idlen TSRMLS_CC)) == NULL) ||
+                (read_mbulk_header(redis_sock, &fields TSRMLS_CC) < 0 || fields % 2 != 0))
+            {
+                if (id) efree(id);
+                return -1;
+            }
 
             REDIS_MAKE_STD_ZVAL(z_msg);
             array_init(z_msg);
@@ -1429,6 +1437,7 @@ redis_read_xclaim_response(RedisSock *redis_sock, int count, zval *rv TSRMLS_DC)
             redis_mbulk_reply_loop(redis_sock, z_msg, fields, UNSERIALIZE_VALS TSRMLS_CC);
             array_zip_values_and_scores(redis_sock, z_msg, SCORE_DECODE_NONE TSRMLS_CC);
             add_assoc_zval_ex(rv, id, idlen, z_msg);
+            efree(id);
         }
     }
 
diff --git a/redis.c b/redis.c
index b8244718..f8290750 100644
--- a/redis.c
+++ b/redis.c
@@ -93,6 +93,10 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_info, 0, 0, 0)
     ZEND_ARG_INFO(0, option)
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_INFO_EX(arginfo_multi, 0, 0, 0)
+    ZEND_ARG_INFO(0, mode)
+ZEND_END_ARG_INFO()
+
 ZEND_BEGIN_ARG_INFO_EX(arginfo_client, 0, 0, 1)
     ZEND_ARG_INFO(0, cmd)
 #if PHP_VERSION_ID >= 50600
@@ -330,7 +334,7 @@ static zend_function_entry redis_functions[] = {
      PHP_ME(Redis, move, arginfo_move, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, mset, arginfo_pairs, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, msetnx, arginfo_pairs, ZEND_ACC_PUBLIC)
-     PHP_ME(Redis, multi, arginfo_void, ZEND_ACC_PUBLIC)
+     PHP_ME(Redis, multi, arginfo_multi, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, object, arginfo_object, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, pconnect, arginfo_pconnect, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, persist, arginfo_key, ZEND_ACC_PUBLIC)
@@ -368,7 +372,7 @@ static zend_function_entry redis_functions[] = {
      PHP_ME(Redis, sMove, arginfo_smove, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, sPop, arginfo_key, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, sRandMember, arginfo_srand_member, ZEND_ACC_PUBLIC)
-     PHP_ME(Redis, sRemove, arginfo_key_value, ZEND_ACC_PUBLIC)
+     PHP_ME(Redis, sRemove, arginfo_key_members, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, sSize, arginfo_key, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, sUnion, arginfo_nkeys, ZEND_ACC_PUBLIC)
      PHP_ME(Redis, sUnionStore, arginfo_dst_nkeys, ZEND_ACC_PUBLIC)
@@ -454,7 +458,7 @@ static zend_function_entry redis_functions[] = {
      PHP_MALIAS(Redis, scard, sSize, arginfo_key, ZEND_ACC_PUBLIC)
      PHP_MALIAS(Redis, sendEcho, echo, arginfo_echo, ZEND_ACC_PUBLIC)
      PHP_MALIAS(Redis, sismember, sContains, arginfo_key_value, ZEND_ACC_PUBLIC)
-     PHP_MALIAS(Redis, srem, sRemove, arginfo_key_value, ZEND_ACC_PUBLIC)
+     PHP_MALIAS(Redis, srem, sRemove, arginfo_key_members, ZEND_ACC_PUBLIC)
      PHP_MALIAS(Redis, substr, getRange, arginfo_key_start_end, ZEND_ACC_PUBLIC)
      PHP_MALIAS(Redis, zRem, zDelete, arginfo_key_members, ZEND_ACC_PUBLIC)
      PHP_MALIAS(Redis, zRemRangeByRank, zDeleteRangeByRank, arginfo_key_min_max, ZEND_ACC_PUBLIC)
@@ -3592,7 +3596,7 @@ PHP_METHOD(Redis, xack) {
 }
 
 PHP_METHOD(Redis, xadd) {
-    REDIS_PROCESS_CMD(xadd, redis_single_line_reply);
+    REDIS_PROCESS_CMD(xadd, redis_read_variant_reply);
 }
 
 PHP_METHOD(Redis, xclaim) {
diff --git a/redis_cluster.c b/redis_cluster.c
index 130b961a..8f10bcb0 100644
--- a/redis_cluster.c
+++ b/redis_cluster.c
@@ -2983,7 +2983,7 @@ PHP_METHOD(RedisCluster, xack) {
 
 /* {{{ proto string RedisCluster::xadd(string key, string id, array field_values) }}} */
 PHP_METHOD(RedisCluster, xadd) {
-    CLUSTER_PROCESS_CMD(xadd, cluster_single_line_resp, 0);
+    CLUSTER_PROCESS_CMD(xadd, cluster_bulk_raw_resp, 0);
 }
 
 /* {{{ proto array RedisCluster::xclaim(string key, string group, string consumer,
diff --git a/tests/RedisTest.php b/tests/RedisTest.php
index df650ee5..e353829d 100644
--- a/tests/RedisTest.php
+++ b/tests/RedisTest.php
@@ -4508,10 +4508,10 @@ class Redis_Test extends TestSuite
         $this->redis->rpush('{eval-key}-list', 'c');
 
         // Make a set
-        $this->redis->del('{eval-key}-set');
-        $this->redis->sadd('{eval-key}-set', 'd');
-        $this->redis->sadd('{eval-key}-set', 'e');
-        $this->redis->sadd('{eval-key}-set', 'f');
+        $this->redis->del('{eval-key}-zset');
+        $this->redis->zadd('{eval-key}-zset', 0, 'd');
+        $this->redis->zadd('{eval-key}-zset', 1, 'e');
+        $this->redis->zadd('{eval-key}-zset', 2, 'f');
 
         // Basic keys
         $this->redis->set('{eval-key}-str1', 'hello, world');
@@ -4521,9 +4521,9 @@ class Redis_Test extends TestSuite
         $list = $this->redis->eval("return redis.call('lrange', KEYS[1], 0, -1)", Array('{eval-key}-list'), 1);
         $this->assertTrue($list === Array('a','b','c'));
 
-        // Use a script to return our set
-        $set = $this->redis->eval("return redis.call('smembers', KEYS[1])", Array('{eval-key}-set'), 1);
-        $this->assertTrue($set == Array('d','e','f'));
+        // Use a script to return our zset
+        $zset = $this->redis->eval("return redis.call('zrange', KEYS[1], 0, -1)", Array('{eval-key}-zset'), 1);
+        $this->assertTrue($zset == Array('d','e','f'));
 
         // Test an empty MULTI BULK response
         $this->redis->del('{eval-key}-nolist');
@@ -4539,7 +4539,7 @@ class Redis_Test extends TestSuite
                     redis.call('get', '{eval-key}-str2'),
                     redis.call('lrange', 'not-any-kind-of-list', 0, -1),
                     {
-                        redis.call('smembers','{eval-key}-set'),
+                        redis.call('zrange','{eval-key}-zset', 0, -1),
                         redis.call('lrange', '{eval-key}-list', 0, -1)
                     }
                 }
@@ -4559,7 +4559,7 @@ class Redis_Test extends TestSuite
         );
 
         // Now run our script, and check our values against each other
-        $eval_result = $this->redis->eval($nested_script, Array('{eval-key}-str1', '{eval-key}-str2', '{eval-key}-set', '{eval-key}-list'), 4);
+        $eval_result = $this->redis->eval($nested_script, Array('{eval-key}-str1', '{eval-key}-str2', '{eval-key}-zset', '{eval-key}-list'), 4);
         $this->assertTrue(is_array($eval_result) && count($this->array_diff_recursive($eval_result, $expected)) == 0);
 
         /*
@@ -6109,6 +6109,7 @@ class Redis_Test extends TestSuite
             $cmd  = (getenv('TEST_PHP_EXECUTABLE') ?: (defined('PHP_BINARY') ? PHP_BINARY : 'php')); // PHP_BINARY is 5.4+
 
             if ($test_args = getenv('TEST_PHP_ARGS')) {
+                $cmd .= ' ';
                 $cmd .= $test_args;
             } else {
                 /* Only append specific extension directives if PHP hasn't been compiled with what we need statically */