[2.x] Fix the problem of protocol loss during cluster MOVED/ASK (#1530)

Co-authored-by: Lynn Club <lynncho@live.com>
Co-authored-by: Lynn Club <10293848+lynnclub@users.noreply.github.com>
Co-authored-by: Till Krüss <tillkruss@users.noreply.github.com>

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## Changelog
 
+## Unreleased
+### Fixed
+- Fixed protocol loss during redis cluster `MOVED` / `ASK`
+
 ## v2.4.0-RC1 (2024-11-21)
 ### Added
 - Added new hash-field expiration commands (#1520)

src/Connection/Cluster/RedisCluster.php

@@ -254,7 +254,7 @@ class RedisCluster implements ClusterInterface, IteratorAggregate, Countable
                 }
 
                 if (!$connection = $this->getRandomConnection()) {
-                    throw new ClientException('No connections left in the pool for `CLUSTER SLOTS`');
+                    throw new ClientException('No connections left in the pool for `CLUSTER SLOTS` (' . $exception->getMessage() . ')');
                 }
 
                 usleep($retryAfter * 1000);
@@ -337,10 +337,19 @@ class RedisCluster implements ClusterInterface, IteratorAggregate, Countable
     {
         $separator = strrpos($connectionID, ':');
 
-        return $this->connections->create([
+        $parameters = [
             'host' => substr($connectionID, 0, $separator),
             'port' => substr($connectionID, $separator + 1),
-        ]);
+        ];
+
+        $existConnection = current($this->pool);
+        if ($existConnection instanceof NodeConnectionInterface) {
+            $existParameters = $existConnection->getParameters()->toArray();
+            unset($existParameters['alias'], $existParameters['slots']);
+            $parameters = array_merge($existParameters, $parameters);
+        }
+
+        return $this->connections->create($parameters);
     }
 
     /**

tests/Predis/Connection/Cluster/RedisClusterTest.php

@@ -319,12 +319,14 @@ class RedisClusterTest extends PredisTestCase
             ->withConsecutive(
                 [
                     [
+                        'scheme' => 'tcp',
                         'host' => '127.0.0.1',
                         'port' => '6383',
                     ],
                 ],
                 [
                     [
+                        'scheme' => 'tcp',
                         'host' => '127.0.0.1',
                         'port' => '6384',
                     ],
@@ -644,6 +646,7 @@ class RedisClusterTest extends PredisTestCase
             ->expects($this->once())
             ->method('create')
             ->with([
+                'scheme' => 'tcp',
                 'host' => '127.0.0.1',
                 'port' => '9381',
             ])
@@ -710,6 +713,7 @@ class RedisClusterTest extends PredisTestCase
             ->expects($this->once())
             ->method('create')
             ->with([
+                'scheme' => 'tcp',
                 'host' => '127.0.0.1',
                 'port' => '9381',
             ])
@@ -1020,6 +1024,7 @@ class RedisClusterTest extends PredisTestCase
             ->expects($this->once())
             ->method('create')
             ->with([
+                'scheme' => 'tcp',
                 'host' => '127.0.0.1',
                 'port' => '6381',
             ])
@@ -1108,6 +1113,58 @@ class RedisClusterTest extends PredisTestCase
             ->expects($this->once())
             ->method('create')
             ->with([
+                'scheme' => 'tcp',
+                'host' => '127.0.0.1',
+                'port' => '6381',
+            ])
+            ->willReturn($connection3);
+
+        $cluster = new RedisCluster($factory);
+        $cluster->useClusterSlots(false);
+
+        $cluster->add($connection1);
+        $cluster->add($connection2);
+
+        $this->assertSame('foobar', $cluster->executeCommand($command));
+        $this->assertSame('foobar', $cluster->executeCommand($command));
+        $this->assertCount(3, $cluster);
+    }
+
+    /**
+     * @group disconnected
+     */
+    public function testNotTCPMovedResponseWithConnectionNotInPool(): void
+    {
+        $movedResponse = new Response\Error('MOVED 1970 127.0.0.1:6381');
+
+        $command = $this->getCommandFactory()->create('get', ['node:1001']);
+
+        $connection1 = $this->getMockConnection('tls://127.0.0.1:6379');
+        $connection1
+            ->expects($this->once())
+            ->method('executeCommand')
+            ->with($command)
+            ->willReturn($movedResponse);
+
+        $connection2 = $this->getMockConnection('tls://127.0.0.1:6380');
+        $connection2
+            ->expects($this->never())
+            ->method('executeCommand');
+
+        $connection3 = $this->getMockConnection('tls://127.0.0.1:6381');
+        $connection3
+            ->expects($this->exactly(2))
+            ->method('executeCommand')
+            ->with($command)
+            ->willReturnOnConsecutiveCalls('foobar', 'foobar');
+
+        /** @var Connection\FactoryInterface|MockObject */
+        $factory = $this->getMockBuilder('Predis\Connection\FactoryInterface')->getMock();
+        $factory
+            ->expects($this->once())
+            ->method('create')
+            ->with([
+                'scheme' => 'tls',
                 'host' => '127.0.0.1',
                 'port' => '6381',
             ])
@@ -1153,6 +1210,7 @@ class RedisClusterTest extends PredisTestCase
             ->expects($this->once())
             ->method('create')
             ->with([
+                'scheme' => 'tcp',
                 'host' => '2001:db8:0:f101::2',
                 'port' => '6379',
             ])
@@ -1250,6 +1308,7 @@ class RedisClusterTest extends PredisTestCase
             ->expects($this->once())
             ->method('create')
             ->with([
+                'scheme' => 'tcp',
                 'host' => '127.0.0.1',
                 'port' => '6380',
             ])