From a0487ec5f445b3f18ed3cf0eac6d1ac4d736be6b Mon Sep 17 00:00:00 2001
From: Daniele Alessandri <suppakilla@gmail.com>
Date: Sun, 8 May 2016 18:35:06 +0200
Subject: [PATCH] Prevent failures serializing commands with "holes" in
 arguments array.

This could be triggered when passing an array with "holes" to variadic commands.
Connection classes based on the protocol serialized exposed by phpiredis were
not affected by this bug.

Fixes #316.
---
 CHANGELOG.md                                  |  5 ++++
 src/Connection/StreamConnection.php           |  3 +--
 src/Protocol/Text/RequestSerializer.php       |  3 +--
 tests/PHPUnit/PredisConnectionTestCase.php    | 13 +++++++++++
 .../Protocol/Text/RequestSerializerTest.php   | 23 +++++++++++++++++++
 5 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index f8c1961a..10faeb7b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,6 +30,11 @@ v1.1.0 (2015-xx-xx)
   must use either the `tls` or `rediss` scheme.
 
 
+- __FIX__: prevent failures when `Predis\Connection\StreamConnection` serializes
+  commands with holes in their arguments (e.g. `[0 => 'key:0', 1 => 'key:1']`).
+  The same fix has been applied to `Predis\Protocol\Text\RequestSerializer`.
+  (ISSUE #316).
+
 v1.0.3 (2015-07-30)
 ================================================================================
 
diff --git a/src/Connection/StreamConnection.php b/src/Connection/StreamConnection.php
index 946e25d6..b1c682c7 100644
--- a/src/Connection/StreamConnection.php
+++ b/src/Connection/StreamConnection.php
@@ -380,8 +380,7 @@ class StreamConnection extends AbstractConnection
 
         $buffer = "*{$reqlen}\r\n\${$cmdlen}\r\n{$commandID}\r\n";
 
-        for ($i = 0, $reqlen--; $i < $reqlen; ++$i) {
-            $argument = $arguments[$i];
+        foreach ($arguments as $argument) {
             $arglen = strlen($argument);
             $buffer .= "\${$arglen}\r\n{$argument}\r\n";
         }
diff --git a/src/Protocol/Text/RequestSerializer.php b/src/Protocol/Text/RequestSerializer.php
index c8cbbfbc..859595b1 100644
--- a/src/Protocol/Text/RequestSerializer.php
+++ b/src/Protocol/Text/RequestSerializer.php
@@ -36,8 +36,7 @@ class RequestSerializer implements RequestSerializerInterface
 
         $buffer = "*{$reqlen}\r\n\${$cmdlen}\r\n{$commandID}\r\n";
 
-        for ($i = 0, $reqlen--; $i < $reqlen; ++$i) {
-            $argument = $arguments[$i];
+        foreach ($arguments as $argument) {
             $arglen = strlen($argument);
             $buffer .= "\${$arglen}\r\n{$argument}\r\n";
         }
diff --git a/tests/PHPUnit/PredisConnectionTestCase.php b/tests/PHPUnit/PredisConnectionTestCase.php
index 1120b58e..be0e6160 100644
--- a/tests/PHPUnit/PredisConnectionTestCase.php
+++ b/tests/PHPUnit/PredisConnectionTestCase.php
@@ -241,6 +241,19 @@ abstract class PredisConnectionTestCase extends PredisTestCase
         $this->assertEquals('PONG', $connection->executeCommand($cmdPing));
     }
 
+    /**
+     * @group disconnected
+     */
+    public function testExecutesCommandWithHolesInArguments()
+    {
+        $profile = $this->getCurrentProfile();
+        $cmdDel = $profile->createCommand('mget', array(0 => 'key:0', 2 => 'key:2'));
+
+        $connection = $this->createConnection();
+
+        $this->assertSame(array(null, null), $connection->executeCommand($cmdDel));
+    }
+
     /**
      * @group connected
      */
diff --git a/tests/Predis/Protocol/Text/RequestSerializerTest.php b/tests/Predis/Protocol/Text/RequestSerializerTest.php
index 84ce5098..896f3f08 100644
--- a/tests/Predis/Protocol/Text/RequestSerializerTest.php
+++ b/tests/Predis/Protocol/Text/RequestSerializerTest.php
@@ -61,4 +61,27 @@ class RequestSerializerTest extends PredisTestCase
 
         $this->assertSame("*3\r\n$3\r\nSET\r\n$3\r\nkey\r\n$5\r\nvalue\r\n", $result);
     }
+
+    /**
+     * @group disconnected
+     */
+    public function testSerializerDoesNotBreakOnArgumentsWithHoles()
+    {
+        $serializer = new RequestSerializer();
+
+        $command = $this->getMock('Predis\Command\CommandInterface');
+
+        $command->expects($this->once())
+                ->method('getId')
+                ->will($this->returnValue('DEL'));
+
+        $command->expects($this->once())
+                ->method('getArguments')
+                ->will($this->returnValue(array(0 => 'key:1', 2 => 'key:2')));
+
+        $result = $serializer->serialize($command);
+
+        $this->assertSame("*3\r\n$3\r\nDEL\r\n$5\r\nkey:1\r\n$5\r\nkey:2\r\n", $result);
+    }
+
 }