From f032199075b5b7b5be23538f2ff60b3f3aa0d632 Mon Sep 17 00:00:00 2001 From: dualfroz Date: Fri, 4 Sep 2026 07:47:46 +0200 Subject: [PATCH] fix: copy ArrayObject contents through recursiveCopy ArrayObjectFilter copied each element through the public copy(), whose first statement resets the object map. That map is what keeps one copy per source object and what guards against cycles, so an object shared inside an ArrayObject was duplicated and a graph whose cycle ran through an ArrayObject recursed until memory ran out. Reach recursiveCopy() through a bound closure, the way the sibling SplDoublyLinkedListFilter already does. DeepCopy::copy() is unchanged. ArrayObjectFilterTest stubbed the copy() call being removed, so it is rewritten against a real DeepCopy like SplDoublyLinkedListFilterTest, keeping its flags and iterator-class assertions. --- .../TypeFilter/Spl/ArrayObjectFilter.php | 24 ++++++++++- tests/DeepCopyTest/DeepCopyTest.php | 29 ++++++++++++++ .../TypeFilter/Spl/ArrayObjectFilterTest.php | 40 ++++++++++++------- 3 files changed, 78 insertions(+), 15 deletions(-) diff --git a/src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php b/src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php index c271509..5ca571c 100644 --- a/src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php +++ b/src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php @@ -1,6 +1,7 @@ createCopyClosure(); + foreach ($arrayObject->getArrayCopy() as $k => $v) { - $clone->offsetSet($k, $this->copier->copy($v)); + $clone->offsetSet($k, $copy($v)); } return $clone; } + + /** + * Copies a single value through the copier's recursive entry point rather than through + * {@see DeepCopy::copy()}, which starts a new copy and so resets the map of already copied + * objects. Going through `copy()` here would duplicate objects shared inside the ArrayObject + * and would never terminate on a graph whose cycle runs through one. + * + * Same approach as {@see SplDoublyLinkedListFilter}. + */ + private function createCopyClosure(): Closure + { + $copier = $this->copier; + + $copy = function (mixed $value) use ($copier): mixed { + return $copier->recursiveCopy($value); + }; + + return Closure::bind($copy, null, DeepCopy::class); + } } diff --git a/tests/DeepCopyTest/DeepCopyTest.php b/tests/DeepCopyTest/DeepCopyTest.php index ae35cbc..7429e3d 100644 --- a/tests/DeepCopyTest/DeepCopyTest.php +++ b/tests/DeepCopyTest/DeepCopyTest.php @@ -247,6 +247,35 @@ class DeepCopyTest extends TestCase $this->assertSame($copy->getProp1()->c, $copy->getProp2()); } + public function test_it_keeps_reference_of_objects_shared_through_an_array_object() + { + $shared = new stdClass(); + + $object = new stdClass(); + $object->collection = new ArrayObject(['a' => $shared, 'b' => $shared]); + $object->direct = $shared; + + $copy = deep_copy($object); + + $this->assertEqualButNotSame($shared, $copy->collection['a']); + $this->assertSame($copy->collection['a'], $copy->collection['b']); + $this->assertSame($copy->collection['a'], $copy->direct); + } + + public function test_it_can_copy_graphs_whose_circular_reference_runs_through_an_array_object() + { + $parent = new stdClass(); + $child = new stdClass(); + + $child->parent = $parent; + $parent->children = new ArrayObject([$child]); + + $copy = deep_copy($parent); + + $this->assertEqualButNotSame($child, $copy->children[0]); + $this->assertSame($copy, $copy->children[0]->parent); + } + public function test_it_can_copy_graphs_with_circular_references() { $a = new stdClass(); diff --git a/tests/DeepCopyTest/TypeFilter/Spl/ArrayObjectFilterTest.php b/tests/DeepCopyTest/TypeFilter/Spl/ArrayObjectFilterTest.php index 12cf81d..bfdec24 100644 --- a/tests/DeepCopyTest/TypeFilter/Spl/ArrayObjectFilterTest.php +++ b/tests/DeepCopyTest/TypeFilter/Spl/ArrayObjectFilterTest.php @@ -6,8 +6,8 @@ use ArrayObject; use DeepCopy\DeepCopy; use DeepCopy\TypeFilter\Spl\ArrayObjectFilter; use PHPUnit\Framework\TestCase; -use Prophecy\Prophecy\ObjectProphecy; use RecursiveArrayIterator; +use stdClass; /** * @author Dominic Tubach @@ -21,28 +21,40 @@ final class ArrayObjectFilterTest extends TestCase */ private $arrayObjectFilter; - /** - * @var DeepCopy|ObjectProphecy - */ - private $copierProphecy; - protected function setUp(): void { - $this->copierProphecy = $this->prophesize(DeepCopy::class); - $this->arrayObjectFilter = new ArrayObjectFilter( - $this->copierProphecy->reveal() - ); + $this->arrayObjectFilter = new ArrayObjectFilter(new DeepCopy()); } public function test_it_deep_copies_an_array_object(): void { - $arrayObject = new ArrayObject(['foo' => 'bar'], ArrayObject::ARRAY_AS_PROPS, RecursiveArrayIterator::class); - $this->copierProphecy->copy('bar')->willReturn('baz'); + $foo = new stdClass(); + $foo->bar = 'baz'; - /** @var \ArrayObject $newArrayObject */ + $arrayObject = new ArrayObject( + ['foo' => $foo], + ArrayObject::ARRAY_AS_PROPS, + RecursiveArrayIterator::class + ); + + /** @var ArrayObject $newArrayObject */ $newArrayObject = $this->arrayObjectFilter->apply($arrayObject); - $this->assertSame(['foo' => 'baz'], $newArrayObject->getArrayCopy()); + + $this->assertNotSame($foo, $newArrayObject['foo']); + $this->assertEquals($foo, $newArrayObject['foo']); $this->assertSame(ArrayObject::ARRAY_AS_PROPS, $newArrayObject->getFlags()); $this->assertSame(RecursiveArrayIterator::class, $newArrayObject->getIteratorClass()); } + + public function test_it_keeps_objects_shared_inside_the_array_object_shared(): void + { + $shared = new stdClass(); + $arrayObject = new ArrayObject(['a' => $shared, 'b' => $shared]); + + /** @var ArrayObject $newArrayObject */ + $newArrayObject = $this->arrayObjectFilter->apply($arrayObject); + + $this->assertNotSame($shared, $newArrayObject['a']); + $this->assertSame($newArrayObject['a'], $newArrayObject['b']); + } }