- do not allow dynamic class name to be used as union type

- use early returns - simplify the reordering of union types
schmittjoh · Aug 13, 2024 · 1bf60d9 · 1bf60d9
1 parent df02675
commit 1bf60d9
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 85 deletions.
diff --git a/src/Handler/UnionHandler.php b/src/Handler/UnionHandler.php
@@ -67,44 +67,29 @@ public function deserializeUnion(DeserializationVisitorInterface $visitor, mixed
             throw new RuntimeException('XML deserialisation into union types is not supported yet.');
         }
 
-        $finalType = null;
-        if (2 === count($type['params'])) {
-            if (!is_array($type['params'][0]) || !array_key_exists('name', $type['params'][0])) {
-                $lookupField = $type['params'][0];
-                $unionMap = $type['params'][1];
-
-                if (!array_key_exists($lookupField, $data)) {
-                    throw new NonVisitableTypeException('Union Discriminator Field \'' . $lookupField . '\' not found in data');
-                }
-
-                $lkup = $data[$lookupField];
-                if (!empty($unionMap)) {
-                    if (array_key_exists($lkup, $unionMap)) {
-                        $finalType = [
-                            'name' => $unionMap[$lkup],
-                            'params' => [],
-                        ];
-                    } else {
-                        throw new NonVisitableTypeException('Union Discriminator Map does not contain key \'' . $lkup . '\'');
-                    }
-                } else {
-                    $finalType = [
-                        'name' => $lkup,
-                        'params' => [],
-                    ];
-                }
+        if (3 === count($type['params'])) {
+            $lookupField = $type['params'][1];
+            if (empty($data[$lookupField])) {
+                throw new NonVisitableTypeException(sprintf('Union Discriminator Field "%s" not found in data', $lookupField));
+            }
+
+            $unionMap = $type['params'][2];
+            $lookupValue = $data[$lookupField];
+            if (empty($unionMap[$lookupValue])) {
+                throw new NonVisitableTypeException(sprintf('Union Discriminator Map does not contain key "%s"', $lookupValue));
             }
-        }
 
-        if (null !== $finalType && null !== $finalType['name']) {
+            $finalType = [
+                'name' => $unionMap[$lookupValue],
+                'params' => [],
+            ];
+
             return $context->getNavigator()->accept($data, $finalType);
-        } else {
-            foreach ($type['params'] as $possibleType) {
-                $finalType = null;
+        }
 
-                if ($this->isPrimitiveType($possibleType['name']) && $this->testPrimitive($data, $possibleType['name'], $context->getFormat())) {
-                    return $context->getNavigator()->accept($data, $possibleType);
-                }
+        foreach ($type['params'][0] as $possibleType) {
+            if ($this->isPrimitiveType($possibleType['name']) && $this->testPrimitive($data, $possibleType['name'], $context->getFormat())) {
+                return $context->getNavigator()->accept($data, $possibleType);
             }
         }
 
@@ -113,7 +98,7 @@ public function deserializeUnion(DeserializationVisitorInterface $visitor, mixed
 
     private function matchSimpleType(mixed $data, array $type, Context $context): mixed
     {
-        foreach ($type['params'] as $possibleType) {
+        foreach ($type['params'][0] as $possibleType) {
             if ($this->isPrimitiveType($possibleType['name']) && !$this->testPrimitive($data, $possibleType['name'], $context->getFormat())) {
                 continue;
             }
@@ -130,7 +115,7 @@ private function matchSimpleType(mixed $data, array $type, Context $context): mi
 
     private function isPrimitiveType(string $type): bool
     {
-        return in_array($type, ['int', 'integer', 'float', 'double', 'bool', 'boolean', 'string']);
+        return in_array($type, ['int', 'integer', 'float', 'double', 'bool', 'boolean', 'string'], true);
     }
 
     private function testPrimitive(mixed $data, string $type, string $format): bool

diff --git a/src/Metadata/Driver/AnnotationOrAttributeDriver.php b/src/Metadata/Driver/AnnotationOrAttributeDriver.php
@@ -263,7 +263,7 @@ public function loadMetadataForClass(\ReflectionClass $class): ?BaseClassMetadat
                         $propertyMetadata->setUnionDiscriminator($annot->field, $annot->map);
                         $propertyMetadata->setType([
                             'name' => 'union',
-                            'params' => [$annot->field, $annot->map],
+                            'params' => [null, $annot->field, $annot->map],
                         ]);
                     }
                 }

diff --git a/src/Metadata/Driver/TypedPropertiesDriver.php b/src/Metadata/Driver/TypedPropertiesDriver.php
@@ -57,17 +57,15 @@ public function __construct(DriverInterface $delegate, ?ParserInterface $typePar
      * For determining types of primitives, it is necessary to reorder primitives so that they are tested from lowest specificity to highest:
      * i.e. null, true, false, int, float, bool, string
      */
-    private function reorderTypes(array $type): array
+    private function reorderTypes(array $types): array
     {
-        if ($type['params']) {
-            uasort($type['params'], static function ($a, $b) {
-                $order = ['null' => 0, 'true' => 1, 'false' => 2, 'bool' => 3, 'int' => 4, 'float' => 5, 'string' => 6];
+        uasort($types, static function ($a, $b) {
+            $order = ['null' => 0, 'true' => 1, 'false' => 2, 'bool' => 3, 'int' => 4, 'float' => 5, 'string' => 6];
 
-                return ($order[$a['name']] ?? 7) <=> ($order[$b['name']] ?? 7);
-            });
-        }
+            return ($order[$a['name']] ?? 7) <=> ($order[$b['name']] ?? 7);
+        });
 
-        return $type;
+        return $types;
     }
 
     private function getDefaultWhiteList(): array
@@ -102,30 +100,24 @@ public function loadMetadataForClass(ReflectionClass $class): ?ClassMetadata
         foreach ($classMetadata->propertyMetadata as $propertyMetadata) {
             // If the inner driver provides a type, don't guess anymore.
             if ($propertyMetadata->type) {
-                if ('union' === $propertyMetadata->type['name']) {
-                    // If this union is discriminated, the params will not contain types.
-                    // If the union isn't discriminated, the params will contain types, and we should reorder them
-                    if (is_array($propertyMetadata->type['params'][0]) && $propertyMetadata->type['params'][0]['name']) {
-                        $propertyMetadata->setType($this->reorderTypes($propertyMetadata->type));
-                    }
-                }
-            } else {
-                try {
-                    $reflectionType = $this->getReflectionType($propertyMetadata);
-
-                    if ($this->shouldTypeHint($reflectionType)) {
-                        $type = $reflectionType->getName();
-
-                        $propertyMetadata->setType($this->typeParser->parse($type));
-                    } elseif ($this->shouldTypeHintUnion($reflectionType)) {
-                        $propertyMetadata->setType($this->reorderTypes([
-                            'name' => 'union',
-                            'params' => array_map(fn (string $type) => $this->typeParser->parse($type), $reflectionType->getTypes()),
-                        ]));
-                    }
-                } catch (ReflectionException $e) {
-                    continue;
+                continue;
+            }
+
+            try {
+                $reflectionType = $this->getReflectionType($propertyMetadata);
+
+                if ($this->shouldTypeHint($reflectionType)) {
+                    $type = $reflectionType->getName();
+
+                    $propertyMetadata->setType($this->typeParser->parse($type));
+                } elseif ($this->shouldTypeHintUnion($reflectionType)) {
+                    $propertyMetadata->setType([
+                        'name' => 'union',
+                        'params' => [$this->reorderTypes(array_map(fn (string $type) => $this->typeParser->parse($type), $reflectionType->getTypes()))],
+                    ]);
                 }
+            } catch (ReflectionException $e) {
+                continue;
             }
         }
 

diff --git a/tests/Fixtures/DiscriminatedComment.php b/tests/Fixtures/DiscriminatedComment.php
@@ -4,8 +4,8 @@
 
 namespace JMS\Serializer\Tests\Fixtures;
 
-use JMS\Serializer\Annotation\Type;
 use JMS\Serializer\Annotation\SerializedName;
+use JMS\Serializer\Annotation\Type;
 
 class DiscriminatedComment
 {

diff --git a/tests/Metadata/Driver/UnionTypedPropertiesDriverTest.php b/tests/Metadata/Driver/UnionTypedPropertiesDriverTest.php
@@ -31,24 +31,27 @@ public function testInferUnionTypesShouldResultInManyTypes()
         self::assertEquals(
             [
                 'name' => 'union',
-                'params' => [
+                'params' =>
                     [
-                        'name' => 'string',
-                        'params' => [],
+                        [
+                            [
+                                'name' => 'string',
+                                'params' => [],
+                            ],
+                            [
+                                'name' => 'int',
+                                'params' => [],
+                            ],
+                            [
+                                'name' => 'float',
+                                'params' => [],
+                            ],
+                            [
+                                'name' => 'bool',
+                                'params' => [],
+                            ],
+                        ],
                     ],
-                    [
-                        'name' => 'int',
-                        'params' => [],
-                    ],
-                    [
-                        'name' => 'float',
-                        'params' => [],
-                    ],
-                    [
-                        'name' => 'bool',
-                        'params' => [],
-                    ],
-                ],
             ],
             $m->propertyMetadata['data']->type,
         );