From 8b8feff246bae26f896e327bdc2817197e2761ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Vieilledent?= Date: Mon, 11 May 2015 11:21:20 +0200 Subject: [PATCH] [PropertyAccess] Fix setting public property on a class having a magic getter | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | N/A | License | MIT | Doc PR | N/A When using PropertyAccessor with an object having both a public property and a magic getter, and one wants to update this property, PropertyAccessor may lose the property reference. This occurs when the public property value is a hash: ```php class Foo { /** * Example: $this->someProperty['foo']['bar'] = 'baz' * @var array */ public $someProperty; public function __get($name) { // ... } } $obj = new Foo(); $obj->someProperty = ['foo' => ['bar' => 'some_value']]; $propertyAccessor->setValue($obj, 'someProperty[foo][bar]', 'another_value'); echo $obj->someProperty['foo']['bar']; // Before this patch: 'some_value' => fail // After this patch: 'another_value' => correct ``` Furthermore, public properties are always used before `__get()` by PHP. This bug is visible since v2.6.5 as d733a887 changed the way `setValue()` works. --- src/Symfony/Component/PropertyAccess/PropertyAccessor.php | 8 ++++---- .../PropertyAccess/Tests/Fixtures/TestClassMagicGet.php | 2 ++ .../PropertyAccess/Tests/PropertyAccessorTest.php | 8 ++++++++ 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php index 5f18b22a37..f13a5c9175 100644 --- a/src/Symfony/Component/PropertyAccess/PropertyAccessor.php +++ b/src/Symfony/Component/PropertyAccess/PropertyAccessor.php @@ -314,11 +314,11 @@ class PropertyAccessor implements PropertyAccessorInterface $result[self::VALUE] = $object->$isser(); } elseif ($reflClass->hasMethod($hasser) && $reflClass->getMethod($hasser)->isPublic()) { $result[self::VALUE] = $object->$hasser(); - } elseif ($reflClass->hasMethod('__get') && $reflClass->getMethod('__get')->isPublic()) { - $result[self::VALUE] = $object->$property; } elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) { $result[self::VALUE] = &$object->$property; $result[self::IS_REF] = true; + } elseif ($reflClass->hasMethod('__get') && $reflClass->getMethod('__get')->isPublic()) { + $result[self::VALUE] = $object->$property; } elseif (!$classHasProperty && property_exists($object, $property)) { // Needed to support \stdClass instances. We need to explicitly // exclude $classHasProperty, otherwise if in the previous clause @@ -410,10 +410,10 @@ class PropertyAccessor implements PropertyAccessorInterface $object->$setter($value); } elseif ($this->isMethodAccessible($reflClass, $getsetter, 1)) { $object->$getsetter($value); - } elseif ($this->isMethodAccessible($reflClass, '__set', 2)) { - $object->$property = $value; } elseif ($classHasProperty && $reflClass->getProperty($property)->isPublic()) { $object->$property = $value; + } elseif ($this->isMethodAccessible($reflClass, '__set', 2)) { + $object->$property = $value; } elseif (!$classHasProperty && property_exists($object, $property)) { // Needed to support \stdClass instances. We need to explicitly // exclude $classHasProperty, otherwise if in the previous clause diff --git a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php index 2f325aa6a7..e465325305 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php +++ b/src/Symfony/Component/PropertyAccess/Tests/Fixtures/TestClassMagicGet.php @@ -15,6 +15,8 @@ class TestClassMagicGet { private $magicProperty; + public $publicProperty; + public function __construct($value) { $this->magicProperty = $value; diff --git a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php index 0c46467567..9a5324f781 100644 --- a/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php +++ b/src/Symfony/Component/PropertyAccess/Tests/PropertyAccessorTest.php @@ -438,4 +438,12 @@ class PropertyAccessorTest extends \PHPUnit_Framework_TestCase $this->assertEquals('foobar', $object->getProperty()); } + + public function testSetValueDeepWithMagicGetter() + { + $obj = new TestClassMagicGet('foo'); + $obj->publicProperty = array('foo' => array('bar' => 'some_value')); + $this->propertyAccessor->setValue($obj, 'publicProperty[foo][bar]', 'Updated'); + $this->assertSame('Updated', $obj->publicProperty['foo']['bar']); + } }