Browse Source

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.
Daniele Alessandri 8 years ago
parent
commit
a0487ec5f4

+ 5 - 0
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)
 ================================================================================
 

+ 1 - 2
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";
         }

+ 1 - 2
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";
         }

+ 13 - 0
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
      */

+ 23 - 0
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);
+    }
+
 }