Skip to content

Commit 04fd5f1

Browse files
committed
[Form] Fixed PropertyPath to not modify Collection instances (not even their clones)
1 parent c0673d7 commit 04fd5f1

File tree

2 files changed

+7
-1
lines changed

2 files changed

+7
-1
lines changed

src/Symfony/Component/Form/Tests/Util/PropertyPathCollectionTest.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ public function testSetValueCallsAdderAndRemoverForCollections()
168168
$axesBefore = $this->getCollection(array(1 => 'second', 3 => 'fourth', 4 => 'fifth'));
169169
$axesMerged = $this->getCollection(array(1 => 'first', 2 => 'second', 3 => 'third'));
170170
$axesAfter = $this->getCollection(array(1 => 'second', 5 => 'first', 6 => 'third'));
171+
$axesMergedCopy = is_object($axesMerged) ? clone $axesMerged : $axesMerged;
171172

172173
// Don't use a mock in order to test whether the collections are
173174
// modified while iterating them
@@ -178,6 +179,9 @@ public function testSetValueCallsAdderAndRemoverForCollections()
178179
$path->setValue($car, $axesMerged);
179180

180181
$this->assertEquals($axesAfter, $car->getAxes());
182+
183+
// The passed collection was not modified
184+
$this->assertEquals($axesMergedCopy, $axesMerged);
181185
}
182186

183187
public function testSetValueCallsAdderAndRemoverForNestedCollections()

src/Symfony/Component/Form/Util/PropertyPath.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,9 @@ private function writeProperty(&$objectOrArray, $property, $singular, $isIndex,
485485
$methods = $this->findAdderAndRemover($reflClass, $singulars);
486486
if (null !== $methods) {
487487
// At this point the add and remove methods have been found
488-
$itemsToAdd = is_object($value) ? clone $value : $value;
488+
// Use iterator_to_array() instead of clone in order to prevent side effects
489+
// see https://github.com/symfony/symfony/issues/4670
490+
$itemsToAdd = is_object($value) ? iterator_to_array($value) : $value;
489491
$itemToRemove = array();
490492
$propertyValue = $this->readProperty($objectOrArray, $property, $isIndex);
491493
$previousValue = $propertyValue[self::VALUE];

0 commit comments

Comments
 (0)