diff options
| -rw-r--r-- | cluster_library.c | 112 | ||||
| -rw-r--r-- | cluster_library.h | 4 | ||||
| -rw-r--r-- | common.h | 2 | ||||
| -rw-r--r-- | library.c | 62 | ||||
| -rw-r--r-- | library.h | 2 | ||||
| -rw-r--r-- | redis.c | 1 | ||||
| -rw-r--r-- | redis_commands.c | 6 | ||||
| -rw-r--r-- | tests/RedisClusterTest.php | 20 | ||||
| -rw-r--r-- | tests/RedisTest.php | 46 |
9 files changed, 181 insertions, 74 deletions
diff --git a/cluster_library.c b/cluster_library.c index 3720f72c..5bcd2338 100644 --- a/cluster_library.c +++ b/cluster_library.c @@ -61,7 +61,7 @@ static void dump_reply(clusterReply *reply, int indent) { smart_string_appendl(&buf, "\"", 1); break; case TYPE_MULTIBULK: - if (reply->elements == (size_t)-1) { + if (reply->elements < 0) { smart_string_appendl(&buf, "(nil)", sizeof("(nil)")-1); } else { for (i = 0; i < reply->elements; i++) { @@ -91,7 +91,7 @@ static void dump_reply(clusterReply *reply, int indent) { /* Recursively free our reply object. If free_data is non-zero we'll also free * the payload data (strings) themselves. If not, we just free the structs */ void cluster_free_reply(clusterReply *reply, int free_data) { - int i; + long long i; switch(reply->type) { case TYPE_ERR: @@ -101,10 +101,14 @@ void cluster_free_reply(clusterReply *reply, int free_data) { efree(reply->str); break; case TYPE_MULTIBULK: - for (i = 0; i < reply->elements && reply->element[i]; i++) { - cluster_free_reply(reply->element[i], free_data); + if (reply->element) { + if (reply->elements > 0) { + for (i = 0; i < reply->elements && reply->element[i]; i++) { + cluster_free_reply(reply->element[i], free_data); + } + } + efree(reply->element); } - if (reply->element) efree(reply->element); break; default: break; @@ -154,13 +158,11 @@ cluster_multibulk_resp_recursive(RedisSock *sock, size_t elements, } break; case TYPE_MULTIBULK: - if (r->len >= 0) { - r->elements = r->len; - if (r->len > 0) { - r->element = ecalloc(r->len,sizeof(clusterReply*)); - if (cluster_multibulk_resp_recursive(sock, r->elements, r->element, status_strings) < 0) { - return FAILURE; - } + r->elements = r->len; + if (r->elements > 0) { + r->element = ecalloc(r->len, sizeof(*r->element)); + if (cluster_multibulk_resp_recursive(sock, r->elements, r->element, status_strings) < 0) { + return FAILURE; } } break; @@ -204,7 +206,7 @@ clusterReply *cluster_read_resp(redisCluster *c, int status_strings) { * command and consumed the reply type and meta info (length) */ clusterReply* cluster_read_sock_resp(RedisSock *redis_sock, REDIS_REPLY_TYPE type, - char *line_reply, size_t len) + char *line_reply, long long len) { clusterReply *r; @@ -232,7 +234,7 @@ cluster_read_sock_resp(RedisSock *redis_sock, REDIS_REPLY_TYPE type, break; case TYPE_MULTIBULK: r->elements = len; - if (len != (size_t)-1) { + if (r->elements > 0) { r->element = ecalloc(len, sizeof(clusterReply*)); if (cluster_multibulk_resp_recursive(redis_sock, len, r->element, line_reply != NULL) < 0) { cluster_free_reply(r, 1); @@ -241,7 +243,7 @@ cluster_read_sock_resp(RedisSock *redis_sock, REDIS_REPLY_TYPE type, } break; default: - cluster_free_reply(r,1); + cluster_free_reply(r, 1); return NULL; } @@ -1939,10 +1941,11 @@ PHP_REDIS_API void cluster_unsub_resp(INTERNAL_FUNCTION_PARAMETERS, } /* Recursive MULTI BULK -> PHP style response handling */ -static void cluster_mbulk_variant_resp(clusterReply *r, zval *z_ret) +static void cluster_mbulk_variant_resp(clusterReply *r, int null_mbulk_as_null, + zval *z_ret) { zval z_sub_ele; - int i; + long long i; switch(r->type) { case TYPE_INT: @@ -1963,11 +1966,15 @@ static void cluster_mbulk_variant_resp(clusterReply *r, zval *z_ret) } break; case TYPE_MULTIBULK: - array_init(&z_sub_ele); - for (i = 0; i < r->elements; i++) { - cluster_mbulk_variant_resp(r->element[i], &z_sub_ele); + if (r->elements < 0 && null_mbulk_as_null) { + add_next_index_null(z_ret); + } else { + array_init(&z_sub_ele); + for (i = 0; i < r->elements; i++) { + cluster_mbulk_variant_resp(r->element[i], null_mbulk_as_null, &z_sub_ele); + } + add_next_index_zval(z_ret, &z_sub_ele); } - add_next_index_zval(z_ret, &z_sub_ele); break; default: add_next_index_bool(z_ret, 0); @@ -1983,7 +1990,7 @@ cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, { clusterReply *r; zval zv, *z_arr = &zv; - int i; + long long i; // Make sure we can read it if ((r = cluster_read_resp(c, status_strings)) == NULL) { @@ -2014,12 +2021,15 @@ cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, } break; case TYPE_MULTIBULK: - array_init(z_arr); - - for (i = 0; i < r->elements; i++) { - cluster_mbulk_variant_resp(r->element[i], z_arr); + if (r->elements < 0 && c->flags->null_mbulk_as_null) { + RETVAL_NULL(); + } else { + array_init(z_arr); + for (i = 0; i < r->elements; i++) { + cluster_mbulk_variant_resp(r->element[i], c->flags->null_mbulk_as_null, z_arr); + } + RETVAL_ZVAL(z_arr, 0, 0); } - RETVAL_ZVAL(z_arr, 0, 0); break; default: RETVAL_FALSE; @@ -2048,7 +2058,11 @@ cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, } break; case TYPE_MULTIBULK: - cluster_mbulk_variant_resp(r, &c->multi_resp); + if (r->elements < 0 && c->flags->null_mbulk_as_null) { + add_next_index_null(&c->multi_resp); + } else { + cluster_mbulk_variant_resp(r, c->flags->null_mbulk_as_null, &c->multi_resp); + } break; default: add_next_index_bool(&c->multi_resp, 0); @@ -2069,7 +2083,8 @@ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisClust PHP_REDIS_API void cluster_variant_raw_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx) { - cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, c->flags->reply_literal, ctx); + cluster_variant_resp_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, c, + c->flags->reply_literal, ctx); } PHP_REDIS_API void cluster_variant_resp_strings(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, @@ -2084,23 +2099,25 @@ PHP_REDIS_API void cluster_gen_mbulk_resp(INTERNAL_FUNCTION_PARAMETERS, { zval z_result; - /* Return FALSE if we didn't get a multi-bulk response */ - if (c->reply_type != TYPE_MULTIBULK) { + /* Abort if the reply isn't MULTIBULK or has an invalid length */ + if (c->reply_type != TYPE_MULTIBULK || c->reply_len < -1) { CLUSTER_RETURN_FALSE(c); } - /* Allocate our array */ - array_init(&z_result); + if (c->reply_len == -1 && c->flags->null_mbulk_as_null) { + ZVAL_NULL(&z_result); + } else { + array_init(&z_result); - /* Consume replies as long as there are more than zero */ - if (c->reply_len > 0) { - /* Push serialization settings from the cluster into our socket */ - c->cmd_sock->serializer = c->flags->serializer; + if (c->reply_len > 0) { + /* Push serialization settings from the cluster into our socket */ + c->cmd_sock->serializer = c->flags->serializer; - /* Call our specified callback */ - if (cb(c->cmd_sock, &z_result, c->reply_len, ctx) == FAILURE) { - zval_dtor(&z_result); - CLUSTER_RETURN_FALSE(c); + /* Call our specified callback */ + if (cb(c->cmd_sock, &z_result, c->reply_len, ctx) == FAILURE) { + zval_dtor(&z_result); + CLUSTER_RETURN_FALSE(c); + } } } @@ -2245,14 +2262,17 @@ PHP_REDIS_API void cluster_xread_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx) { zval z_streams; - array_init(&z_streams); - c->cmd_sock->serializer = c->flags->serializer; c->cmd_sock->compression = c->flags->compression; - if (redis_read_stream_messages_multi(c->cmd_sock, c->reply_len, &z_streams) < 0) { - zval_dtor(&z_streams); - CLUSTER_RETURN_FALSE(c); + if (c->reply_len == -1 && c->flags->null_mbulk_as_null) { + ZVAL_NULL(&z_streams); + } else { + array_init(&z_streams); + if (redis_read_stream_messages_multi(c->cmd_sock, c->reply_len, &z_streams) < 0) { + zval_dtor(&z_streams); + CLUSTER_RETURN_FALSE(c); + } } if (CLUSTER_IS_ATOMIC(c)) { diff --git a/cluster_library.h b/cluster_library.h index 98e9b0ec..f8f1eec8 100644 --- a/cluster_library.h +++ b/cluster_library.h @@ -325,14 +325,14 @@ typedef struct clusterReply { size_t integer; /* Integer reply */ long long len; /* Length of our string */ char *str; /* String reply */ - size_t elements; /* Count of array elements */ + long long elements; /* Count of array elements */ struct clusterReply **element; /* Array elements */ } clusterReply; /* Direct variant response handler */ clusterReply *cluster_read_resp(redisCluster *c, int status_strings); clusterReply *cluster_read_sock_resp(RedisSock *redis_sock, - REDIS_REPLY_TYPE type, char *line_reply, size_t reply_len); + REDIS_REPLY_TYPE type, char *line_reply, long long reply_len); void cluster_free_reply(clusterReply *reply, int free_data); /* Cluster distribution helpers for WATCH */ @@ -82,6 +82,7 @@ typedef enum _PUBSUB_TYPE { #define REDIS_OPT_COMPRESSION 7 #define REDIS_OPT_REPLY_LITERAL 8 #define REDIS_OPT_COMPRESSION_LEVEL 9 +#define REDIS_OPT_NULL_MBULK_AS_NULL 10 /* cluster options */ #define REDIS_FAILOVER_NONE 0 @@ -296,6 +297,7 @@ typedef struct { int readonly; int reply_literal; + int null_mbulk_as_null; int tcp_keepalive; } RedisSock; /* }}} */ @@ -1600,10 +1600,13 @@ redis_xread_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, if (read_mbulk_header(redis_sock, &streams) < 0) goto failure; - array_init(&z_rv); - - if (redis_read_stream_messages_multi(redis_sock, streams, &z_rv) < 0) - goto cleanup; + if (streams == -1 && redis_sock->null_mbulk_as_null) { + ZVAL_NULL(&z_rv); + } else { + array_init(&z_rv); + if (redis_read_stream_messages_multi(redis_sock, streams, &z_rv) < 0) + goto cleanup; + } if (IS_ATOMIC(redis_sock)) { RETVAL_ZVAL(&z_rv, 0, 1); @@ -2427,6 +2430,7 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) { + zval z_multi_result; char inbuf[4096]; int numElems; size_t len; @@ -2448,10 +2452,13 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS, } numElems = atoi(inbuf+1); - zval z_multi_result; - array_init(&z_multi_result); /* pre-allocate array for multi's results. */ - redis_mbulk_reply_loop(redis_sock, &z_multi_result, numElems, UNSERIALIZE_ALL); + if (numElems == -1 && redis_sock->null_mbulk_as_null) { + ZVAL_NULL(&z_multi_result); + } else { + array_init(&z_multi_result); + redis_mbulk_reply_loop(redis_sock, &z_multi_result, numElems, UNSERIALIZE_ALL); + } if (IS_ATOMIC(redis_sock)) { RETVAL_ZVAL(&z_multi_result, 0, 1); @@ -2459,7 +2466,6 @@ PHP_REDIS_API int redis_sock_read_multibulk_reply(INTERNAL_FUNCTION_PARAMETERS, add_next_index_zval(z_tab, &z_multi_result); } - /*zval_copy_ctor(return_value); */ return 0; } @@ -3231,7 +3237,7 @@ redis_read_variant_bulk(RedisSock *redis_sock, int size, zval *z_ret } PHP_REDIS_API int -redis_read_multibulk_recursive(RedisSock *redis_sock, int elements, int status_strings, +redis_read_multibulk_recursive(RedisSock *redis_sock, long long elements, int status_strings, zval *z_ret) { long reply_info; @@ -3267,11 +3273,15 @@ redis_read_multibulk_recursive(RedisSock *redis_sock, int elements, int status_s add_next_index_zval(z_ret, &z_subelem); break; case TYPE_MULTIBULK: - // Construct an array for our sub element, and add it, and recurse - array_init(&z_subelem); - add_next_index_zval(z_ret, &z_subelem); - redis_read_multibulk_recursive(redis_sock, reply_info, status_strings, - &z_subelem); + if (reply_info < 0 && redis_sock->null_mbulk_as_null) { + add_next_index_null(z_ret); + } else { + array_init(&z_subelem); + if (reply_info > 0) { + redis_read_multibulk_recursive(redis_sock, reply_info, status_strings, &z_subelem); + } + add_next_index_zval(z_ret, &z_subelem); + } break; default: // Stop the compiler from whinging @@ -3287,7 +3297,8 @@ redis_read_multibulk_recursive(RedisSock *redis_sock, int elements, int status_s static int variant_reply_generic(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, - int status_strings, zval *z_tab, void *ctx) + int status_strings, int null_mbulk_as_null, + zval *z_tab, void *ctx) { // Reply type, and reply size vars REDIS_REPLY_TYPE reply_type; @@ -3312,13 +3323,15 @@ variant_reply_generic(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, redis_read_variant_bulk(redis_sock, reply_info, &z_ret); break; case TYPE_MULTIBULK: - /* Initialize an array for our multi-bulk response */ - array_init(&z_ret); - - // If we've got more than zero elements, parse our multi bulk - // response recursively if (reply_info > -1) { + array_init(&z_ret); redis_read_multibulk_recursive(redis_sock, reply_info, status_strings, &z_ret); + } else { + if (null_mbulk_as_null) { + ZVAL_NULL(&z_ret); + } else { + array_init(&z_ret); + } } break; default: @@ -3343,21 +3356,24 @@ redis_read_raw_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock zval *z_tab, void *ctx) { return variant_reply_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, - redis_sock->reply_literal, z_tab, ctx); + redis_sock->reply_literal, + redis_sock->null_mbulk_as_null, + z_tab, ctx); } PHP_REDIS_API int redis_read_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) { - return variant_reply_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, 0, z_tab, ctx); + return variant_reply_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, 0, + redis_sock->null_mbulk_as_null, z_tab, ctx); } PHP_REDIS_API int redis_read_variant_reply_strings(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx) { - return variant_reply_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, 1, z_tab, ctx); + return variant_reply_generic(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, 1, 0, z_tab, ctx); } PHP_REDIS_API @@ -144,7 +144,7 @@ PHP_REDIS_API int redis_acl_log_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *r PHP_REDIS_API int redis_read_reply_type(RedisSock *redis_sock, REDIS_REPLY_TYPE *reply_type, long *reply_info); PHP_REDIS_API int redis_read_variant_bulk(RedisSock *redis_sock, int size, zval *z_ret); -PHP_REDIS_API int redis_read_multibulk_recursive(RedisSock *redis_sock, int elements, int status_strings, zval *z_ret); +PHP_REDIS_API int redis_read_multibulk_recursive(RedisSock *redis_sock, long long elements, int status_strings, zval *z_ret); PHP_REDIS_API int redis_read_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx); PHP_REDIS_API int redis_read_raw_variant_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx); PHP_REDIS_API int redis_read_variant_reply_strings(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx); @@ -713,6 +713,7 @@ static void add_class_constants(zend_class_entry *ce, int is_cluster) { zend_declare_class_constant_long(ce, ZEND_STRL("OPT_COMPRESSION"), REDIS_OPT_COMPRESSION); zend_declare_class_constant_long(ce, ZEND_STRL("OPT_REPLY_LITERAL"), REDIS_OPT_REPLY_LITERAL); zend_declare_class_constant_long(ce, ZEND_STRL("OPT_COMPRESSION_LEVEL"), REDIS_OPT_COMPRESSION_LEVEL); + zend_declare_class_constant_long(ce, ZEND_STRL("OPT_NULL_MULTIBULK_AS_NULL"), REDIS_OPT_NULL_MBULK_AS_NULL); /* serializer */ zend_declare_class_constant_long(ce, ZEND_STRL("SERIALIZER_NONE"), REDIS_SERIALIZER_NONE); diff --git a/redis_commands.c b/redis_commands.c index d6bb2bcf..07f409c7 100644 --- a/redis_commands.c +++ b/redis_commands.c @@ -3996,6 +3996,8 @@ void redis_getoption_handler(INTERNAL_FUNCTION_PARAMETERS, RETURN_LONG(redis_sock->scan); case REDIS_OPT_REPLY_LITERAL: RETURN_LONG(redis_sock->reply_literal); + case REDIS_OPT_NULL_MBULK_AS_NULL: + RETURN_LONG(redis_sock->null_mbulk_as_null); case REDIS_OPT_FAILOVER: RETURN_LONG(c->failover); default: @@ -4040,6 +4042,10 @@ void redis_setoption_handler(INTERNAL_FUNCTION_PARAMETERS, val_long = zval_get_long(val); redis_sock->reply_literal = val_long != 0; RETURN_TRUE; + case REDIS_OPT_NULL_MBULK_AS_NULL: + val_long = zval_get_long(val); + redis_sock->null_mbulk_as_null = val_long != 0; + RETURN_TRUE; case REDIS_OPT_COMPRESSION: val_long = zval_get_long(val); if (val_long == REDIS_COMPRESSION_NONE diff --git a/tests/RedisClusterTest.php b/tests/RedisClusterTest.php index 0da0904c..a1baf90d 100644 --- a/tests/RedisClusterTest.php +++ b/tests/RedisClusterTest.php @@ -751,5 +751,25 @@ class Redis_Cluster_Test extends Redis_Test { return 'seed[]=' . $host; }, self::$_arr_node_map)) . ($auth ? "&$auth" : ''); } + + /* Test correct handling of null multibulk replies */ + public function testNullArray() { + $key = "key:arr"; + $this->redis->del($key); + + foreach ([false => [], true => NULL] as $opt => $test) { + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, $opt); + + $r = $this->redis->rawCommand($key, "BLPOP", $key, .05); + $this->assertEquals($test, $r); + + $this->redis->multi(); + $this->redis->rawCommand($key, "BLPOP", $key, .05); + $r = $this->redis->exec(); + $this->assertEquals([$test], $r); + } + + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, false); + } } ?> diff --git a/tests/RedisTest.php b/tests/RedisTest.php index 3c848853..781fb0ea 100644 --- a/tests/RedisTest.php +++ b/tests/RedisTest.php @@ -935,8 +935,15 @@ class Redis_Test extends TestSuite // blocking blpop, brpop $this->redis->del('list'); - $this->assertTrue($this->redis->blPop(['list'], 1) === []); - $this->assertTrue($this->redis->brPop(['list'], 1) === []); + + /* Also test our option that we want *-1 to be returned as NULL */ + foreach ([false => [], true => NULL] as $opt => $val) { + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, $opt); + $this->assertEquals($val, $this->redis->blPop(['list'], 1)); + $this->assertEquals($val, $this->redis->brPop(['list'], 1)); + } + + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, false); } public function testllen() @@ -4952,6 +4959,41 @@ class Redis_Test extends TestSuite $this->redis->setOption(Redis::OPT_REPLY_LITERAL, false); } + public function testNullArray() { + $key = "key:arr"; + $this->redis->del($key); + + foreach ([false => [], true => NULL] as $opt => $test) { + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, $opt); + + $r = $this->redis->rawCommand("BLPOP", $key, .05); + $this->assertEquals($test, $r); + + $this->redis->multi(); + $this->redis->rawCommand("BLPOP", $key, .05); + $r = $this->redis->exec(); + $this->assertEquals([$test], $r); + } + + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, false); + } + + /* Test that we can configure PhpRedis to return NULL for *-1 even nestedwithin replies */ + public function testNestedNullArray() { + $this->redis->del('{notaset}'); + + foreach ([false => [], true => NULL] as $opt => $test) { + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, $opt); + $this->assertEquals([$test, $test], $this->redis->geoPos('{notaset}', 'm1', 'm2')); + + $this->redis->multi(); + $this->redis->geoPos('{notaset}', 'm1', 'm2'); + $this->assertEquals([[$test, $test]], $this->redis->exec()); + } + + $this->redis->setOption(Redis::OPT_NULL_MULTIBULK_AS_NULL, false); + } + public function testReconnectSelect() { $key = 'reconnect-select'; $value = 'Has been set!'; |