bug #31031 [Serializer] MetadataAwareNameConverter: Do not assume that property names are strings (soyuka)

fabpot · fabpot · commit de53bd6c6766 · 2019-04-10T12:55:09.000+02:00
This PR was merged into the 4.2 branch. Discussion ---------- [Serializer] MetadataAwareNameConverter: Do not assume that property names are strings | Q | A | ------------- | --- | Branch? | 4.2 (class introduced in v4.2.3) | Bug fix? | yes | New feature? | no | BC breaks? | no  | Deprecations? | no | Tests pass? | yes | Fixed tickets | api-platform/core#2709 | License | MIT | Doc PR | n/a When this class was introduced, there was an assumption made about the type of `propertyNames` and therefore a `: ?string` return type was introduced in the fallbacks/normalization private methods. Because symfony doesn't use strict mode yet (compatibility issues with php IIRC), when using a non-string property name (for example the integer `0` which is a valid property name in an array), it will convert the integer to a string. This is not good, especially if you have a name converter that returns the given property name (ie no transformation) you'll have it's type changed which isn't correct. I've discovered this bug while working on adding this name converter in api platform (api-platform/core#2709). Commits ------- af1e136 MetadataAwareNameConverter: Do not assume that property names are strings
diff --git a/src/Symfony/Component/Serializer/NameConverter/MetadataAwareNameConverter.php b/src/Symfony/Component/Serializer/NameConverter/MetadataAwareNameConverter.php
@@ -69,7 +69,7 @@ public function denormalize($propertyName, string $class = null, string $format
         return self::$denormalizeCache[$class][$propertyName] ?? $this->denormalizeFallback($propertyName, $class, $format, $context);
     }
 
-    private function getCacheValueForNormalization(string $propertyName, string $class): ?string
+    private function getCacheValueForNormalization($propertyName, string $class)
     {
         if (!$this->metadataFactory->hasMetadataFor($class)) {
             return null;
@@ -83,12 +83,12 @@ private function getCacheValueForNormalization(string $propertyName, string $cla
         return $attributesMetadata[$propertyName]->getSerializedName() ?? null;
     }
 
-    private function normalizeFallback(string $propertyName, string $class = null, string $format = null, array $context = []): string
+    private function normalizeFallback($propertyName, string $class = null, string $format = null, array $context = [])
     {
         return $this->fallbackNameConverter ? $this->fallbackNameConverter->normalize($propertyName, $class, $format, $context) : $propertyName;
     }
 
-    private function getCacheValueForDenormalization(string $propertyName, string $class): ?string
+    private function getCacheValueForDenormalization($propertyName, string $class)
     {
         if (!isset(self::$attributesMetadataCache[$class])) {
             self::$attributesMetadataCache[$class] = $this->getCacheValueForAttributesMetadata($class);
@@ -97,7 +97,7 @@ private function getCacheValueForDenormalization(string $propertyName, string $c
         return self::$attributesMetadataCache[$class][$propertyName] ?? null;
     }
 
-    private function denormalizeFallback(string $propertyName, string $class = null, string $format = null, array $context = []): string
+    private function denormalizeFallback($propertyName, string $class = null, string $format = null, array $context = [])
     {
         return $this->fallbackNameConverter ? $this->fallbackNameConverter->denormalize($propertyName, $class, $format, $context) : $propertyName;
     }
diff --git a/src/Symfony/Component/Serializer/Tests/NameConverter/MetadataAwareNameConverterTest.php b/src/Symfony/Component/Serializer/Tests/NameConverter/MetadataAwareNameConverterTest.php
@@ -102,6 +102,7 @@ public function attributeProvider()
             ['foo', 'baz'],
             ['bar', 'qux'],
             ['quux', 'quux'],
+            [0, 0],
         ];
     }
 
@@ -111,6 +112,7 @@ public function fallbackAttributeProvider()
             ['foo', 'baz'],
             ['bar', 'qux'],
             ['quux', 'QUUX'],
+            [0, 0],
         ];
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -69,7 +69,7 @@ public function denormalize($propertyName, string $class = null, string $format`
`69`	`69`	`return self::$denormalizeCache[$class][$propertyName] ?? $this->denormalizeFallback($propertyName, $class, $format, $context);`
`70`	`70`	`}`
`71`	`71`
`72`		`- private function getCacheValueForNormalization(string $propertyName, string $class): ?string`
	`72`	`+ private function getCacheValueForNormalization($propertyName, string $class)`
`73`	`73`	`{`
`74`	`74`	`if (!$this->metadataFactory->hasMetadataFor($class)) {`
`75`	`75`	`return null;`
`@@ -83,12 +83,12 @@ private function getCacheValueForNormalization(string $propertyName, string $cla`
`83`	`83`	`return $attributesMetadata[$propertyName]->getSerializedName() ?? null;`
`84`	`84`	`}`
`85`	`85`
`86`		`- private function normalizeFallback(string $propertyName, string $class = null, string $format = null, array $context = []): string`
	`86`	`+ private function normalizeFallback($propertyName, string $class = null, string $format = null, array $context = [])`
`87`	`87`	`{`
`88`	`88`	`return $this->fallbackNameConverter ? $this->fallbackNameConverter->normalize($propertyName, $class, $format, $context) : $propertyName;`
`89`	`89`	`}`
`90`	`90`
`91`		`- private function getCacheValueForDenormalization(string $propertyName, string $class): ?string`
	`91`	`+ private function getCacheValueForDenormalization($propertyName, string $class)`
`92`	`92`	`{`
`93`	`93`	`if (!isset(self::$attributesMetadataCache[$class])) {`
`94`	`94`	`self::$attributesMetadataCache[$class] = $this->getCacheValueForAttributesMetadata($class);`
`@@ -97,7 +97,7 @@ private function getCacheValueForDenormalization(string $propertyName, string $c`
`97`	`97`	`return self::$attributesMetadataCache[$class][$propertyName] ?? null;`
`98`	`98`	`}`
`99`	`99`
`100`		`- private function denormalizeFallback(string $propertyName, string $class = null, string $format = null, array $context = []): string`
	`100`	`+ private function denormalizeFallback($propertyName, string $class = null, string $format = null, array $context = [])`
`101`	`101`	`{`
`102`	`102`	`return $this->fallbackNameConverter ? $this->fallbackNameConverter->denormalize($propertyName, $class, $format, $context) : $propertyName;`
`103`	`103`	`}`
Original file line number	Diff line number	Diff line change
`@@ -102,6 +102,7 @@ public function attributeProvider()`
`102`	`102`	`['foo', 'baz'],`
`103`	`103`	`['bar', 'qux'],`
`104`	`104`	`['quux', 'quux'],`
	`105`	`+ [0, 0],`
`105`	`106`	`];`
`106`	`107`	`}`
`107`	`108`
`@@ -111,6 +112,7 @@ public function fallbackAttributeProvider()`
`111`	`112`	`['foo', 'baz'],`
`112`	`113`	`['bar', 'qux'],`
`113`	`114`	`['quux', 'QUUX'],`
	`115`	`+ [0, 0],`
`114`	`116`	`];`
`115`	`117`	`}`
`116`	`118`	`}`