[Serializer] Fix unexpected allowed attributes

symfony · Mar 19, 2024 · 900d034 · 900d034
1 parent 998fa9d
commit 900d034
Show file tree

Hide file tree

Showing 6 changed files with 194 additions and 15 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.php b/src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.php
@@ -137,6 +137,8 @@
                 service('property_info')->ignoreOnInvalid(),
                 service('serializer.mapping.class_discriminator_resolver')->ignoreOnInvalid(),
                 null,
+                [],
+                service('property_info')->ignoreOnInvalid(),
             ])
 
         ->alias(PropertyNormalizer::class, 'serializer.normalizer.property')

diff --git a/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php
@@ -147,6 +147,8 @@ public function supportsNormalization($data, ?string $format = null)
      */
     public function normalize($object, ?string $format = null, array $context = [])
     {
+        $context['_read_attributes'] = true;
+
         if (!isset($context['cache_key'])) {
             $context['cache_key'] = $this->getCacheKey($format, $context);
         }
@@ -359,6 +361,8 @@ public function supportsDenormalization($data, string $type, ?string $format = n
      */
     public function denormalize($data, string $type, ?string $format = null, array $context = [])
     {
+        $context['_read_attributes'] = false;
+
         if (!isset($context['cache_key'])) {
             $context['cache_key'] = $this->getCacheKey($format, $context);
         }

diff --git a/src/Symfony/Component/Serializer/Normalizer/GetSetMethodNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/GetSetMethodNormalizer.php
@@ -36,22 +36,23 @@
  */
 class GetSetMethodNormalizer extends AbstractObjectNormalizer
 {
+    private static $reflectionCache = [];
     private static $setterAccessibleCache = [];
 
     /**
      * {@inheritdoc}
      */
     public function supportsNormalization($data, ?string $format = null)
     {
-        return parent::supportsNormalization($data, $format) && $this->supports(\get_class($data));
+        return parent::supportsNormalization($data, $format) && $this->supports(\get_class($data), true);
     }
 
     /**
      * {@inheritdoc}
      */
     public function supportsDenormalization($data, string $type, ?string $format = null)
     {
-        return parent::supportsDenormalization($data, $type, $format) && $this->supports($type);
+        return parent::supportsDenormalization($data, $type, $format) && $this->supports($type, false);
     }
 
     /**
@@ -63,18 +64,22 @@ public function hasCacheableSupportsMethod(): bool
     }
 
     /**
-     * Checks if the given class has any getter method.
+     * Checks if the given class has any getter or setter method.
      */
-    private function supports(string $class): bool
+    private function supports(string $class, bool $readAttributes): bool
     {
         if (null !== $this->classDiscriminatorResolver && $this->classDiscriminatorResolver->getMappingForClass($class)) {
             return true;
         }
 
-        $class = new \ReflectionClass($class);
-        $methods = $class->getMethods(\ReflectionMethod::IS_PUBLIC);
-        foreach ($methods as $method) {
-            if ($this->isGetMethod($method)) {
+        if (!isset(self::$reflectionCache[$class])) {
+            self::$reflectionCache[$class] = new \ReflectionClass($class);
+        }
+
+        $reflection = self::$reflectionCache[$class];
+
+        foreach ($reflection->getMethods(\ReflectionMethod::IS_PUBLIC) as $reflectionMethod) {
+            if ($readAttributes ? $this->isGetMethod($reflectionMethod) : $this->isSetMethod($reflectionMethod)) {
                 return true;
             }
         }
@@ -95,6 +100,17 @@ private function isGetMethod(\ReflectionMethod $method): bool
             );
     }
 
+    /**
+     * Checks if a method's name matches /^set.+$/ and can be called non-statically with one parameter.
+     */
+    private function isSetMethod(\ReflectionMethod $method): bool
+    {
+        return !$method->isStatic()
+            && (\PHP_VERSION_ID < 80000 || !$method->getAttributes(Ignore::class))
+            && 1 === $method->getNumberOfRequiredParameters()
+            && str_starts_with($method->name, 'set');
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -124,19 +140,17 @@ protected function extractAttributes(object $object, ?string $format = null, arr
      */
     protected function getAttributeValue(object $object, string $attribute, ?string $format = null, array $context = [])
     {
-        $ucfirsted = ucfirst($attribute);
-
-        $getter = 'get'.$ucfirsted;
+        $getter = 'get'.$attribute;
         if (method_exists($object, $getter) && \is_callable([$object, $getter])) {
             return $object->$getter();
         }
 
-        $isser = 'is'.$ucfirsted;
+        $isser = 'is'.$attribute;
         if (method_exists($object, $isser) && \is_callable([$object, $isser])) {
             return $object->$isser();
         }
 
-        $haser = 'has'.$ucfirsted;
+        $haser = 'has'.$attribute;
         if (method_exists($object, $haser) && \is_callable([$object, $haser])) {
             return $object->$haser();
         }
@@ -149,7 +163,7 @@ protected function getAttributeValue(object $object, string $attribute, ?string
      */
     protected function setAttributeValue(object $object, string $attribute, $value, ?string $format = null, array $context = [])
     {
-        $setter = 'set'.ucfirst($attribute);
+        $setter = 'set'.$attribute;
         $key = \get_class($object).':'.$setter;
 
         if (!isset(self::$setterAccessibleCache[$key])) {
@@ -160,4 +174,48 @@ protected function setAttributeValue(object $object, string $attribute, $value,
             $object->$setter($value);
         }
     }
+
+    protected function isAllowedAttribute($classOrObject, string $attribute, ?string $format = null, array $context = [])
+    {
+        if (!parent::isAllowedAttribute($classOrObject, $attribute, $format, $context)) {
+            return false;
+        }
+
+        $class = \is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject;
+
+        if (!isset(self::$reflectionCache[$class])) {
+            self::$reflectionCache[$class] = new \ReflectionClass($class);
+        }
+
+        $reflection = self::$reflectionCache[$class];
+
+        if ($context['_read_attributes'] ?? true) {
+            foreach (['get', 'is', 'has'] as $getterPrefix) {
+                $getter = $getterPrefix.$attribute;
+                $reflectionMethod = $reflection->hasMethod($getter) ? $reflection->getMethod($getter) : null;
+                if ($reflectionMethod && $this->isGetMethod($reflectionMethod)) {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
+        $setter = 'set'.$attribute;
+        if ($reflection->hasMethod($setter) && $this->isSetMethod($reflection->getMethod($setter))) {
+            return true;
+        }
+
+        $constructor = $reflection->getConstructor();
+
+        if ($constructor && $constructor->isPublic()) {
+            foreach ($constructor->getParameters() as $parameter) {
+                if ($parameter->getName() === $attribute) {
+                    return true;
+                }
+            }
+        }
+
+        return false;
+    }
 }
diff --git a/src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php b/src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php
@@ -14,7 +14,11 @@
 use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
+use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
+use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
 use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface;
+use Symfony\Component\PropertyInfo\PropertyWriteInfo;
+use Symfony\Component\Serializer\Annotation\Ignore;
 use Symfony\Component\Serializer\Exception\LogicException;
 use Symfony\Component\Serializer\Mapping\AttributeMetadata;
 use Symfony\Component\Serializer\Mapping\ClassDiscriminatorResolverInterface;
@@ -28,11 +32,14 @@
  */
 class ObjectNormalizer extends AbstractObjectNormalizer
 {
+    private static $reflectionCache = [];
+
     protected $propertyAccessor;
+    protected $propertyInfoExtractor;
 
     private $objectClassResolver;
 
-    public function __construct(?ClassMetadataFactoryInterface $classMetadataFactory = null, ?NameConverterInterface $nameConverter = null, ?PropertyAccessorInterface $propertyAccessor = null, ?PropertyTypeExtractorInterface $propertyTypeExtractor = null, ?ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, ?callable $objectClassResolver = null, array $defaultContext = [])
+    public function __construct(?ClassMetadataFactoryInterface $classMetadataFactory = null, ?NameConverterInterface $nameConverter = null, ?PropertyAccessorInterface $propertyAccessor = null, ?PropertyTypeExtractorInterface $propertyTypeExtractor = null, ?ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, ?callable $objectClassResolver = null, array $defaultContext = [], ?PropertyInfoExtractorInterface $propertyInfoExtractor = null)
     {
         if (!class_exists(PropertyAccess::class)) {
             throw new LogicException('The ObjectNormalizer class requires the "PropertyAccess" component. Install "symfony/property-access" to use it.');
@@ -45,6 +52,8 @@ public function __construct(?ClassMetadataFactoryInterface $classMetadataFactory
         $this->objectClassResolver = $objectClassResolver ?? function ($class) {
             return \is_object($class) ? \get_class($class) : $class;
         };
+
+        $this->propertyInfoExtractor = $propertyInfoExtractor ?: new ReflectionExtractor();
     }
 
     /**
@@ -182,4 +191,38 @@ protected function getAllowedAttributes($classOrObject, array $context, bool $at
 
         return $allowedAttributes;
     }
+
+    protected function isAllowedAttribute($classOrObject, string $attribute, ?string $format = null, array $context = [])
+    {
+        if (!parent::isAllowedAttribute($classOrObject, $attribute, $format, $context)) {
+            return false;
+        }
+        $class = \is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject;
+
+        if ($context['_read_attributes'] ?? true) {
+            return $this->propertyInfoExtractor->isReadable($class, $attribute) || $this->hasAttributeAccessorMethod($class, $attribute);
+        }
+
+        return $this->propertyInfoExtractor->isWritable($class, $attribute)
+            || ($writeInfo = $this->propertyInfoExtractor->getWriteInfo($class, $attribute)) && PropertyWriteInfo::TYPE_NONE !== $writeInfo->getType();
+    }
+
+    private function hasAttributeAccessorMethod(string $class, string $attribute): bool
+    {
+        if (!isset(self::$reflectionCache[$class])) {
+            self::$reflectionCache[$class] = new \ReflectionClass($class);
+        }
+
+        $reflection = self::$reflectionCache[$class];
+
+        if (!$reflection->hasMethod($attribute)) {
+            return false;
+        }
+
+        $method = $reflection->getMethod($attribute);
+
+        return !$method->isStatic()
+            && (\PHP_VERSION_ID < 80000 || !$method->getAttributes(Ignore::class))
+            && !$method->getNumberOfRequiredParameters();
+    }
 }
diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/GetSetMethodNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/GetSetMethodNormalizerTest.php
@@ -521,6 +521,23 @@ public function testDenormalizeWithDiscriminator()
 
         $this->assertEquals($denormalized, $normalizer->denormalize(['type' => 'two', 'url' => 'url'], GetSetMethodDummyInterface::class));
     }
+
+    public function testSupportsAndNormalizeWithOnlyParentGetter()
+    {
+        $obj = new GetSetDummyChild();
+        $obj->setFoo('foo');
+
+        $this->assertTrue($this->normalizer->supportsNormalization($obj));
+        $this->assertSame(['foo' => 'foo'], $this->normalizer->normalize($obj));
+    }
+
+    public function testSupportsAndDenormalizeWithOnlyParentSetter()
+    {
+        $this->assertTrue($this->normalizer->supportsDenormalization(['foo' => 'foo'], GetSetDummyChild::class));
+
+        $obj = $this->normalizer->denormalize(['foo' => 'foo'], GetSetDummyChild::class);
+        $this->assertSame('foo', $obj->getFoo());
+    }
 }
 
 class GetSetDummy
@@ -825,3 +842,22 @@ public function setUrl(string $url): void
         $this->url = $url;
     }
 }
+
+class GetSetDummyChild extends GetSetDummyParent
+{
+}
+
+class GetSetDummyParent
+{
+    private $foo;
+
+    public function getFoo()
+    {
+        return $this->foo;
+    }
+
+    public function setFoo($foo)
+    {
+        $this->foo = $foo;
+    }
+}
diff --git a/src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php b/src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php
@@ -18,6 +18,7 @@
 use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor;
 use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
 use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
+use Symfony\Component\Serializer\Annotation\Ignore;
 use Symfony\Component\Serializer\Exception\LogicException;
 use Symfony\Component\Serializer\Exception\RuntimeException;
 use Symfony\Component\Serializer\Exception\UnexpectedValueException;
@@ -919,6 +920,31 @@ public function testSamePropertyAsMethodWithMethodSerializedName()
 
         $this->assertSame($expected, $this->normalizer->normalize($object));
     }
+
+    public function testNormalizeWithIgnoreAnnotationAndPrivateProperties()
+    {
+        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
+        $normalizer = new ObjectNormalizer($classMetadataFactory);
+
+        $this->assertSame(['foo' => 'foo'], $normalizer->normalize(new ObjectDummyWithIgnoreAnnotationAndPrivateProperty()));
+    }
+
+    public function testDenormalizeWithIgnoreAnnotationAndPrivateProperties()
+    {
+        $classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
+        $normalizer = new ObjectNormalizer($classMetadataFactory);
+
+        $obj = $normalizer->denormalize([
+            'foo' => 'set',
+            'ignore' => 'set',
+            'private' => 'set',
+        ], ObjectDummyWithIgnoreAnnotationAndPrivateProperty::class);
+
+        $expected = new ObjectDummyWithIgnoreAnnotationAndPrivateProperty();
+        $expected->foo = 'set';
+
+        $this->assertEquals($expected, $obj);
+    }
 }
 
 class ProxyObjectDummy extends ObjectDummy
@@ -1207,3 +1233,13 @@ public function getInner()
         return $this->inner;
     }
 }
+
+class ObjectDummyWithIgnoreAnnotationAndPrivateProperty
+{
+    public $foo = 'foo';
+
+    /** @Ignore */
+    public $ignored = 'ignored';
+
+    private $private = 'private';
+}