From b73a2016ff58dfdceae25013017e11786c1d05de Mon Sep 17 00:00:00 2001 From: acoulton Date: Thu, 5 Jun 2025 11:01:34 +0100 Subject: [PATCH] bug: Fix `ObjectPropertyRipper` to handle `stdClass` objects Currently, attempting to rip from a `stdClass` will fail with an error that it is not possible to bind a closure to the scope of an internal object. It should not normally be necessary to call any of the `rip` methods for a `stdClass`, because by definition all props are public so the result will be the same as `$vars = (array) $object`. However, it can cause a problem if e.g. calling the ObjectPropertyRipper recursively to dump an entire object graph where the tree may include some stdClass instances. Our methods are typed to accept any kind of object, so should work with `stdClass` rather than requiring the caller to check this externally. --- CHANGELOG.md | 4 ++ src/Object/ObjectPropertyRipper.php | 45 ++++++++++++------- test/unit/Object/ObjectPropertyRipperTest.php | 42 +++++++++++++++++ 3 files changed, 75 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 058f33a..bc07d26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ### Unreleased +### v2.4.0 (2025-06-05) + +* Fix `ObjectPropertyRipper` to handle `stdClass` objects + ### v2.3.1 (2025-03-12) * Support option to customize EOL character in CSVWriter diff --git a/src/Object/ObjectPropertyRipper.php b/src/Object/ObjectPropertyRipper.php index 35da329..e25aeed 100644 --- a/src/Object/ObjectPropertyRipper.php +++ b/src/Object/ObjectPropertyRipper.php @@ -49,10 +49,9 @@ public static function ripAll(object $object): array // We also shouldn't cache, as individual objects may have variable field names (e.g. with public vars) // that are not present on other instances of the same class - $props = (\Closure::bind( + $props = (self::bindScopedClosure( fn() => \get_object_vars($object), - NULL, - $object + $object, ))(); // Safety check - the method above is efficient but can't return private props from parent classes @@ -92,21 +91,35 @@ public static function ripOne($object, $property) */ protected static function getRipper($class) { - if ( ! isset(static::$rippers[$class])) { - static::$rippers[$class] = \Closure::bind( - function ($object, $properties) { - $values = []; - foreach ($properties as $property) { - $values[$property] = $object->$property; - } + static::$rippers[$class] ??= self::bindScopedClosure( + function ($object, $properties) { + $values = []; + foreach ($properties as $property) { + $values[$property] = $object->$property; + } - return $values; - }, - NULL, - $class - ); - } + return $values; + }, + $class, + ); return static::$rippers[$class]; } + + /** + * @param object|class-string $scope + */ + private static function bindScopedClosure(callable $callback, object|string $scope): \Closure + { + $scope_class = \is_object($scope) ? $scope::class : $scope; + if ($scope_class === \stdClass::class) { + // Cannot bind to the scope of an internal class (e.g. stdClass), and there is no need to do so since + // all stdClass properties are public. + // Note that this is the *not* the case for a user-defined class that extends stdClass, hence checking + // for the exact class name rather than `instanceof`. + $scope = null; + } + + return \Closure::bind($callback, newThis: null, newScope: $scope); + } } diff --git a/test/unit/Object/ObjectPropertyRipperTest.php b/test/unit/Object/ObjectPropertyRipperTest.php index 27bb53f..42431e2 100644 --- a/test/unit/Object/ObjectPropertyRipperTest.php +++ b/test/unit/Object/ObjectPropertyRipperTest.php @@ -9,6 +9,7 @@ use Ingenerator\PHPUtils\Object\ObjectPropertyRipper; use PHPUnit\Framework\TestCase; +use stdClass; class ObjectPropertyRipperTest extends TestCase { @@ -44,6 +45,47 @@ public function test_it_rips_all_properties() ); } + public function test_it_can_rip_from_stdclass() + { + $c = new stdClass(); + $c->data = 'something'; + $c->other = 1; + + $this->assertSame( + [ + 'data' => 'something', + 'other' => 1, + ], + ObjectPropertyRipper::ripAll($c), + ); + + $this->assertSame('something', ObjectPropertyRipper::ripOne($c, 'data')); + $this->assertSame(['other' => 1], ObjectPropertyRipper::rip($c, ['other'])); + } + + public function test_it_can_rip_from_class_that_inherits_from_stdclass() + { + // This is an edge case and should be fairly unlikely IRL, but it is valid. + + $c = new class extends stdClass { + private string $hidden = 'whatever'; + }; + $c->data = 'something'; + $c->other = 1; + + $this->assertSame( + [ + 'hidden' => 'whatever', + 'data' => 'something', + 'other' => 1, + ], + ObjectPropertyRipper::ripAll($c), + ); + + $this->assertSame('whatever', ObjectPropertyRipper::ripOne($c, 'hidden')); + $this->assertSame(['hidden' => 'whatever', 'other' => 1], ObjectPropertyRipper::rip($c, ['hidden', 'other'])); + } + public function test_it_throws_if_ripping_all_from_an_object_with_private_parent_properties() { $this->expectException(\DomainException::class);