Casting an object to array does not unwrap refcount=1 references

[nicolas-grekas]

Description

The following code produces the expected output on PHP < 8.1, but breaks on PHP >=8.1 see https://3v4l.org/L9WpO

class Foo
{
    public $foo;
}

function hydrate($properties, $object)
{
    foreach ($properties as $name => &$value) {
        $object->$name = &$value;
    }
};

$object = new Foo;

hydrate(['foo' => 123], $object);

var_dump(ReflectionReference::fromArrayElement((array) $object, 'foo'));

Resulted in this output:

object(ReflectionReference)#2 (0) {
}

But I expected this output instead:

NULL

PHP Version

8.1

Operating System

No response

[bwoebi]

The difference is that previously this code was routed through zend_array_dup(), duplicating the properties hashtable, with respect to RC=1 references prior to PHP 8.1.
With PHP 8.1 there's now a dedicated function for that zend_std_build_object_properties_array, which does not take care of RC=1 references.

[cmb69]

FWIW, introduced by commit 9da66e6.

[nicolas-grekas]

/cc @dstogov maybe then?

[dstogov]

I'm not sure which behaviour is right. Reference in PHP may automatically turn into regular zval when their reference-counter is decremented to 1, but the point where this conversion occurs is not defined (and cannot be defined). Relaying on reference/non-reference type is a bad idea. In case ReflectionReference::fromArrayElement would need a reference, it should create it.

It's possible to fix this case with the following patch

diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c
index 746897f641..43b54f915c 100644
--- a/Zend/zend_object_handlers.c
+++ b/Zend/zend_object_handlers.c
@@ -111,6 +111,9 @@ ZEND_API HashTable *zend_std_build_object_properties_array(zend_object *zobj) /*
 				continue;
 			}
 
+			if (Z_ISREF_P(prop) && Z_REFCOUNT_P(prop) == 1) {
+				prop = Z_REFVAL_P(prop);
+			}
 			Z_TRY_ADDREF_P(prop);
 			_zend_hash_append(ht, prop_info->name, prop);
 		}

[nicolas-grekas]

Thanks for the patch! I submitted it as #8737 with a test case.
The current behavior is definitely a bug, ReflectionReference::fromArrayElement is just one way to highlight it.
Casting an object to array and seeing the cast + the object being bound by reference is another way to see the issue (I added this situation in the test case also.)

[dstogov]

I wouldn't say this is a bug. I would say ReflectionReference::fromArrayElement relays on undefined behavior.

[nicolas-grekas]

Here is the bug in action without using ReflectionReference:

https://3v4l.org/IQHI1

class Foo
{
    public $foo;
}

function hydrate($properties, $object)
{
    foreach ($properties as $name => &$value) {
        $object->$name = &$value;
    }
};

$object = new Foo;

hydrate(['foo' => 123], $object);

$cast = (array) $object;

$object->foo = 234;

echo $cast['foo'];

[dstogov]

OK. Agree :)