fix: validate malformed response fields

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": {

src/Exception/RequestFailed.php

@@ -28,15 +28,28 @@ 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)) {
+            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);
+        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 (null === $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;
     }
 

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 (!\is_int($header[$key] ?? null)) {
+            throw new UnexpectedResponse(\sprintf('Missing or invalid response header field 0x%x', $key));
+        }
+    }
 }

tests/Unit/Exception/RequestFailedTest.php

@@ -0,0 +1,70 @@
+<?php
+
+/**
+ * This file is part of the tarantool/client package.
+ *
+ * (c) Eugene Leonovich <gen.work@gmail.com>
+ *
+ * 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);
+    }
+
+    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);
+    }
+}

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));
     }