Skip to content

Commit 900d034

Browse files
mtarldmathias-drdata
mtarld
authored and
mathias-drdata
committed
[Serializer] Fix unexpected allowed attributes
1 parent 998fa9d commit 900d034

File tree

6 files changed

+194
-15
lines changed

6 files changed

+194
-15
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.php

+2
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,8 @@
137137
service('property_info')->ignoreOnInvalid(),
138138
service('serializer.mapping.class_discriminator_resolver')->ignoreOnInvalid(),
139139
null,
140+
[],
141+
service('property_info')->ignoreOnInvalid(),
140142
])
141143

142144
->alias(PropertyNormalizer::class, 'serializer.normalizer.property')

‎src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php

+4
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,8 @@ public function supportsNormalization($data, ?string $format = null)
147147
*/
148148
public function normalize($object, ?string $format = null, array $context = [])
149149
{
150+
$context['_read_attributes'] = true;
151+
150152
if (!isset($context['cache_key'])) {
151153
$context['cache_key'] = $this->getCacheKey($format, $context);
152154
}
@@ -359,6 +361,8 @@ public function supportsDenormalization($data, string $type, ?string $format = n
359361
*/
360362
public function denormalize($data, string $type, ?string $format = null, array $context = [])
361363
{
364+
$context['_read_attributes'] = false;
365+
362366
if (!isset($context['cache_key'])) {
363367
$context['cache_key'] = $this->getCacheKey($format, $context);
364368
}

‎src/Symfony/Component/Serializer/Normalizer/GetSetMethodNormalizer.php

+72-14
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,23 @@
3636
*/
3737
class GetSetMethodNormalizer extends AbstractObjectNormalizer
3838
{
39+
private static $reflectionCache = [];
3940
private static $setterAccessibleCache = [];
4041

4142
/**
4243
* {@inheritdoc}
4344
*/
4445
public function supportsNormalization($data, ?string $format = null)
4546
{
46-
return parent::supportsNormalization($data, $format) && $this->supports(\get_class($data));
47+
return parent::supportsNormalization($data, $format) && $this->supports(\get_class($data), true);
4748
}
4849

4950
/**
5051
* {@inheritdoc}
5152
*/
5253
public function supportsDenormalization($data, string $type, ?string $format = null)
5354
{
54-
return parent::supportsDenormalization($data, $type, $format) && $this->supports($type);
55+
return parent::supportsDenormalization($data, $type, $format) && $this->supports($type, false);
5556
}
5657

5758
/**
@@ -63,18 +64,22 @@ public function hasCacheableSupportsMethod(): bool
6364
}
6465

6566
/**
66-
* Checks if the given class has any getter method.
67+
* Checks if the given class has any getter or setter method.
6768
*/
68-
private function supports(string $class): bool
69+
private function supports(string $class, bool $readAttributes): bool
6970
{
7071
if (null !== $this->classDiscriminatorResolver && $this->classDiscriminatorResolver->getMappingForClass($class)) {
7172
return true;
7273
}
7374

74-
$class = new \ReflectionClass($class);
75-
$methods = $class->getMethods(\ReflectionMethod::IS_PUBLIC);
76-
foreach ($methods as $method) {
77-
if ($this->isGetMethod($method)) {
75+
if (!isset(self::$reflectionCache[$class])) {
76+
self::$reflectionCache[$class] = new \ReflectionClass($class);
77+
}
78+
79+
$reflection = self::$reflectionCache[$class];
80+
81+
foreach ($reflection->getMethods(\ReflectionMethod::IS_PUBLIC) as $reflectionMethod) {
82+
if ($readAttributes ? $this->isGetMethod($reflectionMethod) : $this->isSetMethod($reflectionMethod)) {
7883
return true;
7984
}
8085
}
@@ -95,6 +100,17 @@ private function isGetMethod(\ReflectionMethod $method): bool
95100
);
96101
}
97102

103+
/**
104+
* Checks if a method's name matches /^set.+$/ and can be called non-statically with one parameter.
105+
*/
106+
private function isSetMethod(\ReflectionMethod $method): bool
107+
{
108+
return !$method->isStatic()
109+
&& (\PHP_VERSION_ID < 80000 || !$method->getAttributes(Ignore::class))
110+
&& 1 === $method->getNumberOfRequiredParameters()
111+
&& str_starts_with($method->name, 'set');
112+
}
113+
98114
/**
99115
* {@inheritdoc}
100116
*/
@@ -124,19 +140,17 @@ protected function extractAttributes(object $object, ?string $format = null, arr
124140
*/
125141
protected function getAttributeValue(object $object, string $attribute, ?string $format = null, array $context = [])
126142
{
127-
$ucfirsted = ucfirst($attribute);
128-
129-
$getter = 'get'.$ucfirsted;
143+
$getter = 'get'.$attribute;
130144
if (method_exists($object, $getter) && \is_callable([$object, $getter])) {
131145
return $object->$getter();
132146
}
133147

134-
$isser = 'is'.$ucfirsted;
148+
$isser = 'is'.$attribute;
135149
if (method_exists($object, $isser) && \is_callable([$object, $isser])) {
136150
return $object->$isser();
137151
}
138152

139-
$haser = 'has'.$ucfirsted;
153+
$haser = 'has'.$attribute;
140154
if (method_exists($object, $haser) && \is_callable([$object, $haser])) {
141155
return $object->$haser();
142156
}
@@ -149,7 +163,7 @@ protected function getAttributeValue(object $object, string $attribute, ?string
149163
*/
150164
protected function setAttributeValue(object $object, string $attribute, $value, ?string $format = null, array $context = [])
151165
{
152-
$setter = 'set'.ucfirst($attribute);
166+
$setter = 'set'.$attribute;
153167
$key = \get_class($object).':'.$setter;
154168

155169
if (!isset(self::$setterAccessibleCache[$key])) {
@@ -160,4 +174,48 @@ protected function setAttributeValue(object $object, string $attribute, $value,
160174
$object->$setter($value);
161175
}
162176
}
177+
178+
protected function isAllowedAttribute($classOrObject, string $attribute, ?string $format = null, array $context = [])
179+
{
180+
if (!parent::isAllowedAttribute($classOrObject, $attribute, $format, $context)) {
181+
return false;
182+
}
183+
184+
$class = \is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject;
185+
186+
if (!isset(self::$reflectionCache[$class])) {
187+
self::$reflectionCache[$class] = new \ReflectionClass($class);
188+
}
189+
190+
$reflection = self::$reflectionCache[$class];
191+
192+
if ($context['_read_attributes'] ?? true) {
193+
foreach (['get', 'is', 'has'] as $getterPrefix) {
194+
$getter = $getterPrefix.$attribute;
195+
$reflectionMethod = $reflection->hasMethod($getter) ? $reflection->getMethod($getter) : null;
196+
if ($reflectionMethod && $this->isGetMethod($reflectionMethod)) {
197+
return true;
198+
}
199+
}
200+
201+
return false;
202+
}
203+
204+
$setter = 'set'.$attribute;
205+
if ($reflection->hasMethod($setter) && $this->isSetMethod($reflection->getMethod($setter))) {
206+
return true;
207+
}
208+
209+
$constructor = $reflection->getConstructor();
210+
211+
if ($constructor && $constructor->isPublic()) {
212+
foreach ($constructor->getParameters() as $parameter) {
213+
if ($parameter->getName() === $attribute) {
214+
return true;
215+
}
216+
}
217+
}
218+
219+
return false;
220+
}
163221
}

‎src/Symfony/Component/Serializer/Normalizer/ObjectNormalizer.php

+44-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414
use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
1515
use Symfony\Component\PropertyAccess\PropertyAccess;
1616
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
17+
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
18+
use Symfony\Component\PropertyInfo\PropertyInfoExtractorInterface;
1719
use Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface;
20+
use Symfony\Component\PropertyInfo\PropertyWriteInfo;
21+
use Symfony\Component\Serializer\Annotation\Ignore;
1822
use Symfony\Component\Serializer\Exception\LogicException;
1923
use Symfony\Component\Serializer\Mapping\AttributeMetadata;
2024
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorResolverInterface;
@@ -28,11 +32,14 @@
2832
*/
2933
class ObjectNormalizer extends AbstractObjectNormalizer
3034
{
35+
private static $reflectionCache = [];
36+
3137
protected $propertyAccessor;
38+
protected $propertyInfoExtractor;
3239

3340
private $objectClassResolver;
3441

35-
public function __construct(?ClassMetadataFactoryInterface $classMetadataFactory = null, ?NameConverterInterface $nameConverter = null, ?PropertyAccessorInterface $propertyAccessor = null, ?PropertyTypeExtractorInterface $propertyTypeExtractor = null, ?ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, ?callable $objectClassResolver = null, array $defaultContext = [])
42+
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)
3643
{
3744
if (!class_exists(PropertyAccess::class)) {
3845
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
4552
$this->objectClassResolver = $objectClassResolver ?? function ($class) {
4653
return \is_object($class) ? \get_class($class) : $class;
4754
};
55+
56+
$this->propertyInfoExtractor = $propertyInfoExtractor ?: new ReflectionExtractor();
4857
}
4958

5059
/**
@@ -182,4 +191,38 @@ protected function getAllowedAttributes($classOrObject, array $context, bool $at
182191

183192
return $allowedAttributes;
184193
}
194+
195+
protected function isAllowedAttribute($classOrObject, string $attribute, ?string $format = null, array $context = [])
196+
{
197+
if (!parent::isAllowedAttribute($classOrObject, $attribute, $format, $context)) {
198+
return false;
199+
}
200+
$class = \is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject;
201+
202+
if ($context['_read_attributes'] ?? true) {
203+
return $this->propertyInfoExtractor->isReadable($class, $attribute) || $this->hasAttributeAccessorMethod($class, $attribute);
204+
}
205+
206+
return $this->propertyInfoExtractor->isWritable($class, $attribute)
207+
|| ($writeInfo = $this->propertyInfoExtractor->getWriteInfo($class, $attribute)) && PropertyWriteInfo::TYPE_NONE !== $writeInfo->getType();
208+
}
209+
210+
private function hasAttributeAccessorMethod(string $class, string $attribute): bool
211+
{
212+
if (!isset(self::$reflectionCache[$class])) {
213+
self::$reflectionCache[$class] = new \ReflectionClass($class);
214+
}
215+
216+
$reflection = self::$reflectionCache[$class];
217+
218+
if (!$reflection->hasMethod($attribute)) {
219+
return false;
220+
}
221+
222+
$method = $reflection->getMethod($attribute);
223+
224+
return !$method->isStatic()
225+
&& (\PHP_VERSION_ID < 80000 || !$method->getAttributes(Ignore::class))
226+
&& !$method->getNumberOfRequiredParameters();
227+
}
185228
}

‎src/Symfony/Component/Serializer/Tests/Normalizer/GetSetMethodNormalizerTest.php

+36
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,23 @@ public function testDenormalizeWithDiscriminator()
521521

522522
$this->assertEquals($denormalized, $normalizer->denormalize(['type' => 'two', 'url' => 'url'], GetSetMethodDummyInterface::class));
523523
}
524+
525+
public function testSupportsAndNormalizeWithOnlyParentGetter()
526+
{
527+
$obj = new GetSetDummyChild();
528+
$obj->setFoo('foo');
529+
530+
$this->assertTrue($this->normalizer->supportsNormalization($obj));
531+
$this->assertSame(['foo' => 'foo'], $this->normalizer->normalize($obj));
532+
}
533+
534+
public function testSupportsAndDenormalizeWithOnlyParentSetter()
535+
{
536+
$this->assertTrue($this->normalizer->supportsDenormalization(['foo' => 'foo'], GetSetDummyChild::class));
537+
538+
$obj = $this->normalizer->denormalize(['foo' => 'foo'], GetSetDummyChild::class);
539+
$this->assertSame('foo', $obj->getFoo());
540+
}
524541
}
525542

526543
class GetSetDummy
@@ -825,3 +842,22 @@ public function setUrl(string $url): void
825842
$this->url = $url;
826843
}
827844
}
845+
846+
class GetSetDummyChild extends GetSetDummyParent
847+
{
848+
}
849+
850+
class GetSetDummyParent
851+
{
852+
private $foo;
853+
854+
public function getFoo()
855+
{
856+
return $this->foo;
857+
}
858+
859+
public function setFoo($foo)
860+
{
861+
$this->foo = $foo;
862+
}
863+
}

‎src/Symfony/Component/Serializer/Tests/Normalizer/ObjectNormalizerTest.php

+36
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
use Symfony\Component\PropertyInfo\Extractor\PhpStanExtractor;
1919
use Symfony\Component\PropertyInfo\Extractor\ReflectionExtractor;
2020
use Symfony\Component\PropertyInfo\PropertyInfoExtractor;
21+
use Symfony\Component\Serializer\Annotation\Ignore;
2122
use Symfony\Component\Serializer\Exception\LogicException;
2223
use Symfony\Component\Serializer\Exception\RuntimeException;
2324
use Symfony\Component\Serializer\Exception\UnexpectedValueException;
@@ -919,6 +920,31 @@ public function testSamePropertyAsMethodWithMethodSerializedName()
919920

920921
$this->assertSame($expected, $this->normalizer->normalize($object));
921922
}
923+
924+
public function testNormalizeWithIgnoreAnnotationAndPrivateProperties()
925+
{
926+
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
927+
$normalizer = new ObjectNormalizer($classMetadataFactory);
928+
929+
$this->assertSame(['foo' => 'foo'], $normalizer->normalize(new ObjectDummyWithIgnoreAnnotationAndPrivateProperty()));
930+
}
931+
932+
public function testDenormalizeWithIgnoreAnnotationAndPrivateProperties()
933+
{
934+
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
935+
$normalizer = new ObjectNormalizer($classMetadataFactory);
936+
937+
$obj = $normalizer->denormalize([
938+
'foo' => 'set',
939+
'ignore' => 'set',
940+
'private' => 'set',
941+
], ObjectDummyWithIgnoreAnnotationAndPrivateProperty::class);
942+
943+
$expected = new ObjectDummyWithIgnoreAnnotationAndPrivateProperty();
944+
$expected->foo = 'set';
945+
946+
$this->assertEquals($expected, $obj);
947+
}
922948
}
923949

924950
class ProxyObjectDummy extends ObjectDummy
@@ -1207,3 +1233,13 @@ public function getInner()
12071233
return $this->inner;
12081234
}
12091235
}
1236+
1237+
class ObjectDummyWithIgnoreAnnotationAndPrivateProperty
1238+
{
1239+
public $foo = 'foo';
1240+
1241+
/** @Ignore */
1242+
public $ignored = 'ignored';
1243+
1244+
private $private = 'private';
1245+
}

0 commit comments

Comments
 (0)