Skip to content

Commit 058753e

Browse files
albertofemyatsukhnenko
authored andcommitted
Fix Null Bulk String response parsing in cluster library (phpredis#1104)
* Failing test case when running LUA with bulk empty response * Fix issue when parsing bulk array response from eval commands * Added test for bulk LUA responses and changed condition * Added multi tests and fixes in C code format
1 parent cddd610 commit 058753e

File tree

2 files changed

+74
-3
lines changed

2 files changed

+74
-3
lines changed

cluster_library.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1870,7 +1870,11 @@ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisClust
18701870
RETVAL_TRUE;
18711871
break;
18721872
case TYPE_BULK:
1873-
RETVAL_STRINGL(r->str, r->len);
1873+
if (r->len < 0) {
1874+
RETVAL_NULL();
1875+
} else {
1876+
RETVAL_STRINGL(r->str, r->len);
1877+
}
18741878
break;
18751879
case TYPE_MULTIBULK:
18761880
array_init(z_arr);
@@ -1896,8 +1900,12 @@ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisClust
18961900
add_next_index_bool(&c->multi_resp, 1);
18971901
break;
18981902
case TYPE_BULK:
1899-
add_next_index_stringl(&c->multi_resp, r->str, r->len);
1900-
efree(r->str);
1903+
if (r->len < 0) {
1904+
add_next_index_null(&c->multi_resp);
1905+
} else {
1906+
add_next_index_stringl(&c->multi_resp, r->str, r->len);
1907+
efree(r->str);
1908+
}
19011909
break;
19021910
case TYPE_MULTIBULK:
19031911
cluster_mbulk_variant_resp(r, &c->multi_resp);

tests/RedisClusterTest.php

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,69 @@ public function testEvalSHA() {
345345
$this->assertTrue(1 === $this->redis->eval($scr,Array($str_key), 1));
346346
$this->assertTrue(1 === $this->redis->evalsha($sha,Array($str_key), 1));
347347
}
348+
349+
public function testEvalBulkResponse() {
350+
$str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
351+
$str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
352+
353+
$this->redis->script($str_key1, 'flush');
354+
$this->redis->script($str_key2, 'flush');
355+
356+
$scr = "return {KEYS[1],KEYS[2]}";
357+
358+
$result = $this->redis->eval($scr,Array($str_key1, $str_key2), 2);
359+
360+
$this->assertTrue($str_key1 === $result[0]);
361+
$this->assertTrue($str_key2 === $result[1]);
362+
}
363+
364+
public function testEvalBulkResponseMulti() {
365+
$str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
366+
$str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
367+
368+
$this->redis->script($str_key1, 'flush');
369+
$this->redis->script($str_key2, 'flush');
370+
371+
$scr = "return {KEYS[1],KEYS[2]}";
372+
373+
$this->redis->multi();
374+
$this->redis->eval($scr,Array($str_key1, $str_key2), 2);
375+
376+
$result = $this->redis->exec();
377+
378+
$this->assertTrue($str_key1 === $result[0][0]);
379+
$this->assertTrue($str_key2 === $result[0][1]);
380+
}
381+
382+
public function testEvalBulkEmptyResponse() {
383+
$str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
384+
$str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
385+
386+
$this->redis->script($str_key1, 'flush');
387+
$this->redis->script($str_key2, 'flush');
388+
389+
$scr = "for _,key in ipairs(KEYS) do redis.call('SET', key, 'value') end";
390+
391+
$result = $this->redis->eval($scr,Array($str_key1, $str_key2), 2);
392+
393+
$this->assertTrue(null === $result);
394+
}
395+
396+
public function testEvalBulkEmptyResponseMulti() {
397+
$str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
398+
$str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
399+
400+
$this->redis->script($str_key1, 'flush');
401+
$this->redis->script($str_key2, 'flush');
402+
403+
$scr = "for _,key in ipairs(KEYS) do redis.call('SET', key, 'value') end";
404+
405+
$this->redis->multi();
406+
$this->redis->eval($scr,Array($str_key1, $str_key2), 2);
407+
$result = $this->redis->exec();
408+
409+
$this->assertTrue(null === $result[0]);
410+
}
348411

349412
/* Cluster specific introspection stuff */
350413
public function testIntrospection() {

0 commit comments

Comments
 (0)