From 0cfdca87fc75d8288340dd90ca7c11fa64c4f242 Mon Sep 17 00:00:00 2001 From: Eugene Leonovich Date: Sun, 11 Oct 2026 00:41:33 +0200 Subject: [PATCH 1/4] fix: reject malformed IPROTO response --- src/Exception/RequestFailed.php | 22 +++++++--- src/Response.php | 19 +++++++-- tests/Unit/Exception/RequestFailedTest.php | 48 ++++++++++++++++++++++ tests/Unit/ResponseTest.php | 45 ++++++++++++++++++-- 4 files changed, 121 insertions(+), 13 deletions(-) create mode 100644 tests/Unit/Exception/RequestFailedTest.php diff --git a/src/Exception/RequestFailed.php b/src/Exception/RequestFailed.php index a1519624..32a1073b 100644 --- a/src/Exception/RequestFailed.php +++ b/src/Exception/RequestFailed.php @@ -28,15 +28,25 @@ public function getError() : ?Error public static function fromErrorResponse(Response $response) : self { - $self = new self( - $response->getBodyField(Keys::ERROR_24), - $response->getCode() & (Response::TYPE_ERROR - 1) - ); + $error = null; + if ($errorMap = $response->tryGetBodyField(Keys::ERROR)) { + $error = Error::fromMap($errorMap); + } + + $message = $response->tryGetBodyField(Keys::ERROR_24); + if ($response->hasBodyField(Keys::ERROR_24) && !\is_string($message)) { + throw new UnexpectedResponse(\sprintf('Invalid response body field 0x%x', Keys::ERROR_24)); + } - if ($error = $response->tryGetBodyField(Keys::ERROR)) { - $self->error = Error::fromMap($error); + $message ??= $error?->getMessage(); + if (!\is_string($message)) { + throw new UnexpectedResponse(\sprintf('Missing response body field 0x%x', Keys::ERROR_24)); } + $self = new self($message, $response->getCode() & (Response::TYPE_ERROR - 1)); + + $self->error = $error; + return $self; } diff --git a/src/Response.php b/src/Response.php index d16a28d4..ef9d288d 100644 --- a/src/Response.php +++ b/src/Response.php @@ -13,6 +13,8 @@ namespace Tarantool\Client; +use Tarantool\Client\Exception\UnexpectedResponse; + final class Response { public const TYPE_ERROR = 0x8000; @@ -22,15 +24,17 @@ final class Response public function __construct(array $header, array $body) { + self::validateHeaderField($header, Keys::CODE); + self::validateHeaderField($header, Keys::SYNC); + self::validateHeaderField($header, Keys::SCHEMA_ID); + $this->header = $header; $this->body = $body; } public function isError() : bool { - $code = $this->header[Keys::CODE]; - - return $code >= self::TYPE_ERROR; + return $this->getCode() >= self::TYPE_ERROR; } public function getCode() : int @@ -54,7 +58,7 @@ public function getBodyField(int $key) return $this->body[$key]; } - throw new \OutOfRangeException(\sprintf('The body key 0x%x does not exist', $key)); + throw new UnexpectedResponse(\sprintf('Missing response body field 0x%x', $key)); } public function tryGetBodyField(int $key, $default = null) @@ -66,4 +70,11 @@ public function hasBodyField(int $key) : bool { return \array_key_exists($key, $this->body); } + + private static function validateHeaderField(array $header, int $key) : void + { + if (!\array_key_exists($key, $header) || !\is_int($header[$key])) { + throw new UnexpectedResponse(\sprintf('Missing or invalid response header field 0x%x', $key)); + } + } } diff --git a/tests/Unit/Exception/RequestFailedTest.php b/tests/Unit/Exception/RequestFailedTest.php new file mode 100644 index 00000000..10ecd910 --- /dev/null +++ b/tests/Unit/Exception/RequestFailedTest.php @@ -0,0 +1,48 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Tarantool\Client\Tests\Unit\Exception; + +use PHPUnit\Framework\TestCase; +use Tarantool\Client\Error; +use Tarantool\Client\Exception\RequestFailed; +use Tarantool\Client\Exception\UnexpectedResponse; +use Tarantool\Client\Keys; +use Tarantool\Client\Response; + +final class RequestFailedTest extends TestCase +{ + public function testUsesStructuredErrorWhenLegacyMessageIsMissing() : void + { + $error = new Error('ClientError', 'file.lua', 1, 'The server error', 1, 42); + $response = new Response( + [Keys::CODE => Response::TYPE_ERROR + 42, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], + [Keys::ERROR => $error->toMap()] + ); + + $requestFailed = RequestFailed::fromErrorResponse($response); + $actualError = $requestFailed->getError(); + + self::assertSame('The server error', $requestFailed->getMessage()); + self::assertNotNull($actualError); + self::assertSame($error->getMessage(), $actualError->getMessage()); + } + + public function testMissingErrorMessageWithoutStructuredErrorIsUnexpectedResponse() : void + { + $response = new Response([Keys::CODE => Response::TYPE_ERROR, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], []); + + $this->expectException(UnexpectedResponse::class); + RequestFailed::fromErrorResponse($response); + } +} diff --git a/tests/Unit/ResponseTest.php b/tests/Unit/ResponseTest.php index 405baab0..2d0ad488 100644 --- a/tests/Unit/ResponseTest.php +++ b/tests/Unit/ResponseTest.php @@ -14,6 +14,7 @@ namespace Tarantool\Client\Tests\Unit; use PHPUnit\Framework\TestCase; +use Tarantool\Client\Exception\UnexpectedResponse; use Tarantool\Client\Keys; use Tarantool\Client\Response; @@ -22,21 +23,59 @@ final class ResponseTest extends TestCase public function testGetSchemaIdReturnsCorrectId() : void { $schemaId = 42; - $response = new Response([Keys::SCHEMA_ID => $schemaId], []); + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => $schemaId], []); self::assertSame($schemaId, $response->getSchemaId()); } + public function testConstructorRejectsMissingCode() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::SYNC => 1], []); + } + + public function testConstructorRejectsInvalidCodeType() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => '0', Keys::SYNC => 1], []); + } + + public function testConstructorRejectsMissingSync() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => 0], []); + } + + public function testConstructorRejectsInvalidSchemaIdType() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => null], []); + } + + public function testConstructorRejectsMissingSchemaId() : void + { + $this->expectException(UnexpectedResponse::class); + new Response([Keys::CODE => 0, Keys::SYNC => 1], []); + } + + public function testGetBodyFieldRejectsMissingFieldAsUnexpectedResponse() : void + { + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], []); + + $this->expectException(UnexpectedResponse::class); + $response->getBodyField(Keys::DATA); + } + public function testHasBodyKeyReturnsTrue() : void { - $response = new Response([], [42 => null]); + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], [42 => null]); self::assertTrue($response->hasBodyField(42)); } public function testHasBodyKeyReturnsFalse() : void { - $response = new Response([], [24 => null]); + $response = new Response([Keys::CODE => 0, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], [24 => null]); self::assertFalse($response->hasBodyField(42)); } From 3a574576f4287c1ef0fcbd2d23c890d392e40d52 Mon Sep 17 00:00:00 2001 From: Eugene Leonovich Date: Sun, 11 Oct 2026 00:45:27 +0200 Subject: [PATCH 2/4] fix: update test helpers and error fallback --- composer.json | 2 +- src/Exception/RequestFailed.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 04c5d571..f0e8419f 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ "monolog/monolog": "^3.0", "phpunit/phpunit": "^10.5", "psr/log": "^3.0", - "tarantool/phpunit-extras": "^0.3.0", + "tarantool/phpunit-extras": "^0.3.2", "vimeo/psalm": "^5.23|^6" }, "suggest": { diff --git a/src/Exception/RequestFailed.php b/src/Exception/RequestFailed.php index 32a1073b..f7c0aba5 100644 --- a/src/Exception/RequestFailed.php +++ b/src/Exception/RequestFailed.php @@ -39,7 +39,7 @@ public static function fromErrorResponse(Response $response) : self } $message ??= $error?->getMessage(); - if (!\is_string($message)) { + if (null === $message) { throw new UnexpectedResponse(\sprintf('Missing response body field 0x%x', Keys::ERROR_24)); } From d681f13d4f1dc653635fdb8d73a93833e7d09118 Mon Sep 17 00:00:00 2001 From: Eugene Leonovich Date: Sun, 11 Oct 2026 00:51:22 +0200 Subject: [PATCH 3/4] refactor: simplify response header validation --- src/Response.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Response.php b/src/Response.php index ef9d288d..fe1064e2 100644 --- a/src/Response.php +++ b/src/Response.php @@ -73,7 +73,7 @@ public function hasBodyField(int $key) : bool private static function validateHeaderField(array $header, int $key) : void { - if (!\array_key_exists($key, $header) || !\is_int($header[$key])) { + if (!\is_int($header[$key] ?? null)) { throw new UnexpectedResponse(\sprintf('Missing or invalid response header field 0x%x', $key)); } } From 49902530253fb953ef5570c38789ca668bd2bb33 Mon Sep 17 00:00:00 2001 From: Eugene Leonovich Date: Sun, 11 Oct 2026 01:35:09 +0200 Subject: [PATCH 4/4] fix: translate malformed structured errors --- src/Exception/RequestFailed.php | 7 +++++-- tests/Unit/Exception/RequestFailedTest.php | 22 ++++++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/Exception/RequestFailed.php b/src/Exception/RequestFailed.php index f7c0aba5..5e4fe3f3 100644 --- a/src/Exception/RequestFailed.php +++ b/src/Exception/RequestFailed.php @@ -30,7 +30,11 @@ public static function fromErrorResponse(Response $response) : self { $error = null; if ($errorMap = $response->tryGetBodyField(Keys::ERROR)) { - $error = Error::fromMap($errorMap); + try { + $error = Error::fromMap($errorMap); + } catch (\InvalidArgumentException|\TypeError $exception) { + throw new UnexpectedResponse(\sprintf('Invalid response body field 0x%x', Keys::ERROR), 0, $exception); + } } $message = $response->tryGetBodyField(Keys::ERROR_24); @@ -44,7 +48,6 @@ public static function fromErrorResponse(Response $response) : self } $self = new self($message, $response->getCode() & (Response::TYPE_ERROR - 1)); - $self->error = $error; return $self; diff --git a/tests/Unit/Exception/RequestFailedTest.php b/tests/Unit/Exception/RequestFailedTest.php index 10ecd910..8fdbf2d1 100644 --- a/tests/Unit/Exception/RequestFailedTest.php +++ b/tests/Unit/Exception/RequestFailedTest.php @@ -45,4 +45,26 @@ public function testMissingErrorMessageWithoutStructuredErrorIsUnexpectedRespons $this->expectException(UnexpectedResponse::class); RequestFailed::fromErrorResponse($response); } + + public function testRejectsNonArrayStructuredError() : void + { + $response = new Response( + [Keys::CODE => Response::TYPE_ERROR, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], + [Keys::ERROR => 'malformed', Keys::ERROR_24 => 'The server error'] + ); + + $this->expectException(UnexpectedResponse::class); + RequestFailed::fromErrorResponse($response); + } + + public function testTranslatesMalformedStructuredErrorMap() : void + { + $response = new Response( + [Keys::CODE => Response::TYPE_ERROR, Keys::SYNC => 1, Keys::SCHEMA_ID => 0], + [Keys::ERROR => [Keys::ERROR_STACK => []], Keys::ERROR_24 => 'The server error'] + ); + + $this->expectException(UnexpectedResponse::class); + RequestFailed::fromErrorResponse($response); + } }