8000 [Serializer] Fix unexpected allowed attributes · symfony/symfony@46f78a9 · GitHub
[go: up one dir, main page]

Skip to content

Commit 46f78a9

Browse files
committed
[Serializer] Fix unexpected allowed attributes
1 parent 9afa8a8 commit 46f78a9

File tree

6 files changed

+182
-12
lines changed

6 files changed

+182
-12
lines changed

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

Lines changed: 2 additions & 0 deletions
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

Lines changed: 4 additions & 0 deletions
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

Lines changed: 73 additions & 10 deletions
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,22 +64,28 @@ 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)) {
78-
return true;
79-
}
75+
if (!isset(self::$reflectionCache[$class])) {
76+
self::$reflectionCache[$class] = new \ReflectionClass($class);
8077
}
8178

79+
$reflection = self::$reflectionCache[$class];
80+
81+
do {
82+
foreach ($reflection->getMethods(\ReflectionMethod::IS_PUBLIC) as $reflectionMethod) {
83+
if ($readAttributes && $this->isGetMethod($reflectionMethod) || !$readAttributes && $this->isSetMethod($reflectionMethod)) {
84+
return true;
85+
}
86+
}
87+
} while ($reflection = $reflection->getParentClass());
88+
8289
return false;
8390
}
8491

@@ -95,6 +102,17 @@ private function isGetMethod(\ReflectionMethod $method): bool
95102
);
96103
}
97104

105+
/**
106+
* Checks if a method's name matches /^set.+$/ and can be called non-statically with one parameter.
107+
*/
108+
private function isSetMethod(\ReflectionMethod $method): bool
109+
{
110+
return !$method->isStatic()
111+
&& (\PHP_VERSION_ID < 80000 || !$method->getAttributes(Ignore::class))
112+
&& 1 === $method->getNumberOfRequiredParameters()
113+
&& str_starts_with($method->name, 'set');
114+
}
115+
98116
/**
99117
* {@inheritdoc}
100118
*/
@@ -160,4 +178,49 @@ protected function setAttributeValue(object $object, string $attribute, $value,
160178
$object->$setter($value);
161179
}
162180
}
181+
182+
protected function isAllowedAttribute($classOrObject, string $attribute, string $format = null, array $context = [])
183+
{
184+
if (!parent::isAllowedAttribute($classOrObject, $attribute, $format, $context)) {
185+
return false;
186+
}
187+
188+
$class = \is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject;
189+
190+
if (!isset(self::$reflectionCache[$class])) {
191+
self::$reflectionCache[$class] = new \ReflectionClass($class);
192+
}
193+
194+
$reflection = self::$reflectionCache[$class];
195+
196+
do {
197+
if ($context['_read_attributes'] ?? true) {
198+
foreach (['get', 'is', 'has'] as $getterPrefix) {
199+
$getter = $getterPrefix.ucfirst($attribute);
200+
$reflectionMethod = $reflection->hasMethod($getter) ? $reflection->getMethod($getter) : null;
201+
if ($reflectionMethod && $this->isGetMethod($reflectionMethod)) {
202+
return true;
203+
}
204+
}
205+
} else {
206+
$setter = 'set'.ucfirst($attribute);
207+
$reflectionMethod = $reflection->hasMethod($setter) ? $reflection->getMethod($setter) : null;
208+
if ($reflectionMethod && $this->isSetMethod($reflectionMethod)) {
209+
return true;
210+
}
211+
212+
$constructor = $reflection->getConstructor();
213+
214+
if ($constructor && $constructor->isPublic()) {
215+
foreach ($constructor->getParameters() as $parameter) {
216+
if ($parameter->getName() === $attribute) {
217+
return true;
218+
}
219+
}
220+
}
221+
}
222+
} while ($reflection = $reflection->getParentClass());
223+
224+
return false;
225+
}
163226
}

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

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
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;
1821
use Symfony\Component\Serializer\Exception\LogicException;
1922
use Symfony\Component\Serializer\Mapping\AttributeMetadata;
2023
use Symfony\Component\Serializer\Mapping\ClassDiscriminatorResolverInterface;
@@ -29,11 +32,20 @@
2932
class ObjectNormalizer extends AbstractObjectNormalizer
3033
{
3134
protected $propertyAccessor;
35+
protected $propertyInfoExtractor;
3236

3337
private $objectClassResolver;
3438

35-
public function __construct(?ClassMetadataFactoryInterface $classMetadataFactory = null, ?NameConverterInterface $nameConverter = null, ?PropertyAccessorInterface $propertyAccessor = null, ?PropertyTypeExtractorInterface $propertyTypeExtractor = null, ?ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null, ?callable $objectClassResolver = null, array $defaultContext = [])
36-
{
39+
public function __construct(
40+
?ClassMetadataFactoryInterface $classMetadataFactory = null,
41+
?NameConverterInterface $nameConverter = null,
42+
?PropertyAccessorInterface $propertyAccessor = null,
43+
?PropertyTypeExtractorInterface $propertyTypeExtractor = null,
44+
?ClassDiscriminatorResolverInterface $classDiscriminatorResolver = null,
45+
?callable $objectClassResolver = null,
46+
array $defaultContext = [],
47+
?PropertyInfoExtractorInterface $propertyInfoExtractor = null,
48+
) {
3749
if (!class_exists(PropertyAccess::class)) {
3850
throw new LogicException('The ObjectNormalizer class requires the "PropertyAccess" component. Install "symfony/property-access" to use it.');
3951
}
@@ -45,6 +57,8 @@ public function __construct(?ClassMetadataFactoryInterface $classMetadataFactory
4557
$this->objectClassResolver = $objectClassResolver ?? function ($class) {
4658
return \is_object($class) ? \get_class($class) : $class;
4759
};
60+
61+
$this->propertyInfoExtractor = $propertyInfoExtractor ?: new ReflectionExtractor();
4862
}
4963

5064
/**
@@ -174,4 +188,19 @@ protected function getAllowedAttributes($classOrObject, array $context, bool $at
174188

175189
return $allowedAttributes;
176190
}
191+
192+
protected function isAllowedAttribute($classOrObject, string $attribute, string $format = null, array $context = [])
193+
{
194+
if (!parent::isAllowedAttribute($classOrObject, $attribute, $format, $context)) {
195+
return false;
196+
}
197+
$class = \is_object($classOrObject) ? \get_class($classOrObject) : $classOrObject;
198+
199+
if ($context['_read_attributes'] ?? true) {
200+
return $this->propertyInfoExtractor->isReadable($class, $attribute);
201+
}
202+
203+
return $this->propertyInfoExtractor->isWritable($class, $attribute)
204+
|| null !== ($writeInfo = $this->propertyInfoExtractor->getWriteInfo($class, $attribute)) && PropertyWriteInfo::TYPE_NONE !== $writeInfo->getType();
205+
}
177206
}

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

Lines changed: 36 additions & 0 deletions
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

Lines changed: 36 additions & 0 deletions
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;
@@ -869,6 +870,31 @@ public function testNormalizeStdClass()
869870

870871
$this->assertSame(['baz' => 'baz'], $this->normalizer->normalize($o2));
871872
}
873+
874+
public function testNormalizeWithIgnoreAnnotationAndPrivateProperties()
875+
{
876+
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
877+
$normalizer = new ObjectNormalizer($classMetadataFactory);
878+
879+
$this->assertSame(['foo' => 'foo'], $normalizer->normalize(new ObjectDummyWithIgnoreAnnotationAndPrivateProperty()));
880+
}
881+
882+
public function testDenormalizeWithIgnoreAnnotationAndPrivateProperties()
883+
{
884+
$classMetadataFactory = new ClassMetadataFactory(new AnnotationLoader(new AnnotationReader()));
885+
$normalizer = new ObjectNormalizer($classMetadataFactory);
886+
887+
$obj = $normalizer->denormalize([
888+
'foo' => 'set',
889+
'ignore' => 'set',
890+
'private' => 'set',
891+
], ObjectDummyWithIgnoreAnnotationAndPrivateProperty::class);
892+
893+
$expected = new ObjectDummyWithIgnoreAnnotationAndPrivateProperty();
894+
$expected->foo = 'set';
895+
896+
$this->assertEquals($expected, $obj);
897+
}
872898
}
873899

874900
class ProxyObjectDummy extends ObjectDummy
@@ -1152,3 +1178,13 @@ public function getInner()
11521178
return $this->inner;
11531179
}
11541180
}
1181+
1182+
class ObjectDummyWithIgnoreAnnotationAndPrivateProperty
1183+
{
1184+
public $foo = 'foo';
1185+
1186+
/** @Ignore */
1187+
public $ignored = 'ignored';
1188+
1189+
private $private = 'private';
1190+
}

0 commit comments

Comments
 (0)
0