From 380b19a6d30f3cc17b0c21dab490f98f766b8fa2 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 | 22 +++++++++- tests/DeepCopyTest/DeepCopyTest.php | 29 ++++++++++++++ .../TypeFilter/Spl/ArrayObjectFilterTest.php | 40 ++++++++++++------- 3 files changed, 76 insertions(+), 15 deletions(-) diff --git a/src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php b/src/DeepCopy/TypeFilter/Spl/ArrayObjectFilter.php index c271509..ca8da5b 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 through the recursive entry point rather than {@see DeepCopy::copy()}, + * which would reset the map of already copied objects: shared objects would + * be duplicated and a cycle running through the ArrayObject would not + * terminate. 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']); + } }