Fix Null Bulk String response parsing in cluster library (#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
phpredis · Feb 9, 2017 · 058753e · 058753e
1 parent cddd610
commit 058753e
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 3 deletions.
diff --git a/cluster_library.c b/cluster_library.c
@@ -1870,7 +1870,11 @@ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisClust
                 RETVAL_TRUE;
                 break;
             case TYPE_BULK:
-                RETVAL_STRINGL(r->str, r->len);
+                if (r->len < 0) {
+                    RETVAL_NULL();
+                } else {
+                    RETVAL_STRINGL(r->str, r->len);
+                }
                 break;
             case TYPE_MULTIBULK:
                 array_init(z_arr);
@@ -1896,8 +1900,12 @@ PHP_REDIS_API void cluster_variant_resp(INTERNAL_FUNCTION_PARAMETERS, redisClust
                 add_next_index_bool(&c->multi_resp, 1);
                 break;
             case TYPE_BULK:
-                add_next_index_stringl(&c->multi_resp, r->str, r->len);
-                efree(r->str);
+                if (r->len < 0) {
+                    add_next_index_null(&c->multi_resp);
+                } else {
+                    add_next_index_stringl(&c->multi_resp, r->str, r->len);
+                    efree(r->str);
+                }
                 break;
             case TYPE_MULTIBULK:
                 cluster_mbulk_variant_resp(r, &c->multi_resp);

diff --git a/tests/RedisClusterTest.php b/tests/RedisClusterTest.php
@@ -345,6 +345,69 @@ public function testEvalSHA() {
         $this->assertTrue(1 === $this->redis->eval($scr,Array($str_key), 1));
         $this->assertTrue(1 === $this->redis->evalsha($sha,Array($str_key), 1));
     }
+
+    public function testEvalBulkResponse() {
+        $str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
+        $str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
+
+        $this->redis->script($str_key1, 'flush');
+        $this->redis->script($str_key2, 'flush');
+
+        $scr = "return {KEYS[1],KEYS[2]}";
+
+        $result = $this->redis->eval($scr,Array($str_key1, $str_key2), 2);
+
+        $this->assertTrue($str_key1 === $result[0]);
+        $this->assertTrue($str_key2 === $result[1]);
+    }
+
+    public function testEvalBulkResponseMulti() {
+        $str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
+        $str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
+
+        $this->redis->script($str_key1, 'flush');
+        $this->redis->script($str_key2, 'flush');
+
+        $scr = "return {KEYS[1],KEYS[2]}";
+
+        $this->redis->multi();
+        $this->redis->eval($scr,Array($str_key1, $str_key2), 2);
+
+        $result = $this->redis->exec();
+
+        $this->assertTrue($str_key1 === $result[0][0]);
+        $this->assertTrue($str_key2 === $result[0][1]);
+    }
+
+    public function testEvalBulkEmptyResponse() {
+        $str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
+        $str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
+
+        $this->redis->script($str_key1, 'flush');
+        $this->redis->script($str_key2, 'flush');
+
+        $scr = "for _,key in ipairs(KEYS) do redis.call('SET', key, 'value') end";
+
+        $result = $this->redis->eval($scr,Array($str_key1, $str_key2), 2);
+
+        $this->assertTrue(null === $result);
+    }
+
+    public function testEvalBulkEmptyResponseMulti() {
+        $str_key1 = uniqid() . '-' . rand(1,1000) . '{hash}';
+        $str_key2 = uniqid() . '-' . rand(1,1000) . '{hash}';
+
+        $this->redis->script($str_key1, 'flush');
+        $this->redis->script($str_key2, 'flush');
+
+        $scr = "for _,key in ipairs(KEYS) do redis.call('SET', key, 'value') end";
+
+        $this->redis->multi();
+        $this->redis->eval($scr,Array($str_key1, $str_key2), 2);
+        $result = $this->redis->exec();
+
+        $this->assertTrue(null === $result[0]);
+    }
 
     /* Cluster specific introspection stuff */
     public function testIntrospection() {