From 29540e37a9adcbf89741dfea69c37bebc8697a29 Mon Sep 17 00:00:00 2001 From: Pascal CESCON Date: Wed, 7 Oct 2026 10:42:14 +0200 Subject: [PATCH] Only wrap known statuses in Status responses on CompositeStreamConnection (#1734) Since #1293, StreamConnection only returns a Status for OK, QUEUED, NOKEY and PONG. ProtocolProcessor and the StatusResponse handler, used by CompositeStreamConnection, still wrapped every simple string. Fixes #1147 Co-authored-by: Vladyslav Vildanov <117659936+vladvildanov@users.noreply.github.com> --- CHANGELOG.md | 2 ++ src/Protocol/Text/Handler/StatusResponse.php | 13 +++++++++++- src/Protocol/Text/ProtocolProcessor.php | 13 +++++++++++- tests/PHPUnit/PredisConnectionTestCase.php | 14 +++++++++++++ .../Text/Handler/StatusResponseTest.php | 3 +-- .../Protocol/Text/ProtocolProcessorTest.php | 20 +++++++++++++++++++ 6 files changed, 61 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8330d6c7..ce558df6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased ### Changed +- Changed RESP3 double parsing to return `NAN` for NaN payloads instead of `0.0` +- Changed `CompositeStreamConnection` to return simple strings other than `OK`, `QUEUED`, `NOKEY` and `PONG` as plain strings instead of `Status` responses, like `StreamConnection` (#1147) - Changed RESP3 double parsing to return `NAN` for NaN payloads instead of `0.0` (#1718) ### Fixed diff --git a/src/Protocol/Text/Handler/StatusResponse.php b/src/Protocol/Text/Handler/StatusResponse.php index 4abad87c..b3588b12 100644 --- a/src/Protocol/Text/Handler/StatusResponse.php +++ b/src/Protocol/Text/Handler/StatusResponse.php @@ -24,11 +24,22 @@ use Predis\Response\Status; */ class StatusResponse implements ResponseHandlerInterface { + protected $statusResponse = [ + 'OK', + 'QUEUED', + 'NOKEY', + 'PONG', + ]; + /** * {@inheritdoc} */ public function handle(CompositeConnectionInterface $connection, $payload) { - return Status::get($payload); + if (in_array($payload, $this->statusResponse)) { + return Status::get($payload); + } + + return $payload; } } diff --git a/src/Protocol/Text/ProtocolProcessor.php b/src/Protocol/Text/ProtocolProcessor.php index 3aa75639..a140948b 100644 --- a/src/Protocol/Text/ProtocolProcessor.php +++ b/src/Protocol/Text/ProtocolProcessor.php @@ -31,6 +31,13 @@ class ProtocolProcessor implements ProtocolProcessorInterface protected $mbiterable; protected $serializer; + protected $statusResponse = [ + 'OK', + 'QUEUED', + 'NOKEY', + 'PONG', + ]; + public function __construct() { $this->mbiterable = false; @@ -57,7 +64,11 @@ class ProtocolProcessor implements ProtocolProcessorInterface switch ($prefix) { case '+': - return new StatusResponse($payload); + if (in_array($payload, $this->statusResponse)) { + return new StatusResponse($payload); + } + + return $payload; case '$': $size = (int) $payload; diff --git a/tests/PHPUnit/PredisConnectionTestCase.php b/tests/PHPUnit/PredisConnectionTestCase.php index 5cf4ba7b..03488230 100644 --- a/tests/PHPUnit/PredisConnectionTestCase.php +++ b/tests/PHPUnit/PredisConnectionTestCase.php @@ -393,6 +393,20 @@ abstract class PredisConnectionTestCase extends PredisTestCase $this->assertInstanceOf('Predis\Response\Status', $connection->read()); } + /** + * @group connected + */ + public function testReadsSimpleStringsOtherThanStatusesAsStrings(): void + { + $commands = $this->getCommandFactory(); + $connection = $this->createConnection(true); + + $connection->executeCommand($commands->create('set', ['foo', 'bar'])); + + $connection->writeRequest($commands->create('type', ['foo'])); + $this->assertSame('string', $connection->read()); + } + /** * @group connected */ diff --git a/tests/Predis/Protocol/Text/Handler/StatusResponseTest.php b/tests/Predis/Protocol/Text/Handler/StatusResponseTest.php index bb768bb4..d765b195 100644 --- a/tests/Predis/Protocol/Text/Handler/StatusResponseTest.php +++ b/tests/Predis/Protocol/Text/Handler/StatusResponseTest.php @@ -72,7 +72,6 @@ class StatusResponseTest extends PredisTestCase $handler = new Handler\StatusResponse(); $response = $handler->handle($connection, 'Background saving started'); - $this->assertInstanceOf('Predis\Response\Status', $response); - $this->assertEquals('Background saving started', $response); + $this->assertSame('Background saving started', $response); } } diff --git a/tests/Predis/Protocol/Text/ProtocolProcessorTest.php b/tests/Predis/Protocol/Text/ProtocolProcessorTest.php index a60efac8..ab3f23de 100644 --- a/tests/Predis/Protocol/Text/ProtocolProcessorTest.php +++ b/tests/Predis/Protocol/Text/ProtocolProcessorTest.php @@ -72,6 +72,26 @@ class ProtocolProcessorTest extends PredisTestCase $this->assertNull($protocol->read($connection)); } + /** + * @group disconnected + */ + public function testReadsOnlyKnownStatusesAsStatusResponses(): void + { + $protocol = new ProtocolProcessor(); + + $connection = $this->getMockConnectionOfType('Predis\Connection\CompositeConnectionInterface'); + $connection + ->expects($this->exactly(2)) + ->method('readLine') + ->willReturnOnConsecutiveCalls( + '+OK', + '+string' + ); + + $this->assertInstanceOf('Predis\Response\Status', $protocol->read($connection)); + $this->assertSame('string', $protocol->read($connection)); + } + /** * @group disconnected */