Fix EncodedValues::getValue(): add index out of bounds exception

temporalio · Oct 6, 2024 · ffd3e08 · ffd3e08
1 parent df6f9eb
commit ffd3e08
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 11 deletions.
diff --git a/src/DataConverter/EncodedValues.php b/src/DataConverter/EncodedValues.php
@@ -100,7 +100,7 @@ public static function sliceValues(
     public static function decodePromise(PromiseInterface $promise, $type = null): PromiseInterface
     {
         return $promise->then(
-            static function ($value) use ($type) {
+            static function (mixed $value) use ($type) {
                 if (!$value instanceof ValuesInterface || $value instanceof \Throwable) {
                     return $value;
                 }
@@ -153,17 +153,21 @@ public function getValue(int|string $index, $type = null): mixed
             return $this->values[$index];
         }
 
+        $count = $this->count();
         // External SDKs might return an empty array with metadata, alias to null
         // Most likely this is a void type
-        if ($index === 0 && $this->count() === 0 && $this->isVoidType($type)) {
+        if ($index === 0 && $count === 0 && $this->isVoidType($type)) {
             return null;
         }
 
-        if ($this->converter === null) {
-            throw new \LogicException('DataConverter is not set');
-        }
+        $count > $index or throw new \OutOfBoundsException("Index {$index} is out of bounds.");
+        $this->converter === null and throw new \LogicException('DataConverter is not set.');
 
-        return $this->converter->fromPayload($this->payloads[$index], $type);
+        \assert($this->payloads !== null);
+        return $this->converter->fromPayload(
+            $this->payloads[$index],
+            $type,
+        );
     }
 
     public function getValues(): array

diff --git a/src/Internal/Declaration/Dispatcher/AutowiredPayloads.php b/src/Internal/Declaration/Dispatcher/AutowiredPayloads.php
@@ -23,12 +23,12 @@ class AutowiredPayloads extends Dispatcher
     public function dispatchValues(object $ctx, ValuesInterface $values): mixed
     {
         $arguments = [];
-        for ($i = 0; $i < $values->count(); $i++) {
-            try {
+        try {
+            for ($i = 0, $count = $values->count(); $i < $count; $i++) {
                 $arguments[] = $values->getValue($i, $this->getArgumentTypes()[$i] ?? null);
-            } catch (\Throwable $e) {
-                throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
             }
+        } catch (\Throwable $e) {
+            throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
         }
 
         try {

diff --git a/tests/Unit/DataConverter/EncodedValuesTestCase.php b/tests/Unit/DataConverter/EncodedValuesTestCase.php
@@ -86,7 +86,9 @@ public function payloadWithoutValueDecoding(mixed $type): void
     #[DataProvider('getNotNullableTypes')]
     public function payloadWithoutValueDecodingNotNullable(mixed $type): void
     {
-        $encodedValues = EncodedValues::fromPayloadCollection(new \ArrayIterator([]));
+        $encodedValues = EncodedValues::fromPayloadCollection(new \ArrayIterator([
+            new Payloads(),
+        ]));
 
         self::expectException(\LogicException::class);
         self::expectExceptionMessage('DataConverter is not set');
@@ -113,6 +115,25 @@ public function testGetValuesFromEmptyPayloads(): void
         $this->assertNull($ev->getValue(0));
     }
 
+    public function testGetValueFromEmptyValues(): void
+    {
+        $ev = EncodedValues::fromValues([]);
+
+        $this->assertInstanceOf(EncodedValues::class, $ev);
+        $this->assertEmpty($ev->getValues());
+        $this->assertNull($ev->getValue(0));
+    }
+
+    public function testOutOfBounds(): void
+    {
+        $ev = EncodedValues::fromValues([]);
+
+        $this->expectException(\OutOfBoundsException::class);
+        $this->expectExceptionMessage('Index 1 is out of bounds.');
+
+        $ev->getValue(1);
+    }
+
     private static function getReturnType(\Closure $closure): \ReflectionType
     {
         return (new \ReflectionFunction($closure))->getReturnType();