From 04fd5f1b217b73e9cf9fac61af37e6e35d5f79ce Mon Sep 17 00:00:00 2001 From: Bernhard Schussek Date: Fri, 31 Aug 2012 14:34:27 +0200 Subject: [PATCH] [Form] Fixed PropertyPath to not modify Collection instances (not even their clones) --- .../Component/Form/Tests/Util/PropertyPathCollectionTest.php | 4 ++++ src/Symfony/Component/Form/Util/PropertyPath.php | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php b/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php index c6f3451435..8909b7a83e 100644 --- a/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php +++ b/src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php @@ -168,6 +168,7 @@ abstract class PropertyPathCollectionTest extends \PHPUnit_Framework_TestCase $axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth', 4 => 'fifth')); $axesMerged = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third')); $axesAfter = $this->getCollection(array(1 => 'second', 5 => 'first', 6 => 'third')); + $axesMergedCopy = is_object($axesMerged) ? clone $axesMerged : $axesMerged; // Don't use a mock in order to test whether the collections are // modified while iterating them @@ -178,6 +179,9 @@ abstract class PropertyPathCollectionTest extends \PHPUnit_Framework_TestCase $path->setValue($car, $axesMerged); $this->assertEquals($axesAfter, $car->getAxes()); + + // The passed collection was not modified + $this->assertEquals($axesMergedCopy, $axesMerged); } public function testSetValueCallsAdderAndRemoverForNestedCollections() diff --git a/src/Symfony/Component/Form/Util/PropertyPath.php b/src/Symfony/Component/Form/Util/PropertyPath.php index ec8ed267ca..03dc423e24 100644 --- a/src/Symfony/Component/Form/Util/PropertyPath.php +++ b/src/Symfony/Component/Form/Util/PropertyPath.php @@ -485,7 +485,9 @@ class PropertyPath implements \IteratorAggregate, PropertyPathInterface $methods = $this->findAdderAndRemover($reflClass, $singulars); if (null !== $methods) { // At this point the add and remove methods have been found - $itemsToAdd = is_object($value) ? clone $value : $value; + // Use iterator_to_array() instead of clone in order to prevent side effects + // see https://github.com/symfony/symfony/issues/4670 + $itemsToAdd = is_object($value) ? iterator_to_array($value) : $value; $itemToRemove = array(); $propertyValue = $this->readProperty($objectOrArray, $property, $isIndex); $previousValue = $propertyValue[self::VALUE];