From 025ed70ce39072733bb30bd8b25d2e3d4591548c Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 10 Sep 2024 12:27:19 +0200 Subject: [PATCH] Fix ReflectionProperty::isInitialized() for hooked props In zend_std_has_property with ZEND_PROPERTY_EXISTS, we'd just return true when no get hook was present. However, this function is supposed to return false for uninitialized properties. PROPERTY_EXISTS is somewhat of a misnomer. Virtual properties continue to always return true, given there's no backing value to check. Fixes GH-15694 Closes GH-15822 --- NEWS | 2 + Zend/zend_object_handlers.c | 16 +++--- .../ReflectionProperty_isInitialized.phpt | 56 +++++++++++++++++++ 3 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 ext/reflection/tests/property_hooks/ReflectionProperty_isInitialized.phpt diff --git a/NEWS b/NEWS index 897304a0e79..ec6634da08e 100644 --- a/NEWS +++ b/NEWS @@ -49,6 +49,8 @@ PHP NEWS - Reflection: . Fixed bug GH-15718 (Segfault on ReflectionProperty::get{Hook,Hooks}() on dynamic properties). (DanielEScherzer) + . Fixed bug GH-15694 (ReflectionProperty::isInitialized() is incorrect for + hooked properties). (ilutov) - SOAP: . Fixed bug #61525 (SOAP functions require at least one space after HTTP diff --git a/Zend/zend_object_handlers.c b/Zend/zend_object_handlers.c index 1383c437b64..64e494200ee 100644 --- a/Zend/zend_object_handlers.c +++ b/Zend/zend_object_handlers.c @@ -2208,6 +2208,15 @@ found: } } else if (IS_HOOKED_PROPERTY_OFFSET(property_offset)) { zend_function *get = prop_info->hooks[ZEND_PROPERTY_HOOK_GET]; + + if (has_set_exists == ZEND_PROPERTY_EXISTS) { + if (prop_info->flags & ZEND_ACC_VIRTUAL) { + return true; + } + property_offset = prop_info->offset; + goto try_again; + } + if (!get) { if (prop_info->flags & ZEND_ACC_VIRTUAL) { zend_throw_error(NULL, "Property %s::$%s is write-only", @@ -2219,19 +2228,12 @@ found: } } - if (has_set_exists == ZEND_PROPERTY_EXISTS) { - return 1; - } - zval rv; if (!zend_call_get_hook(prop_info, name, get, zobj, &rv)) { if (EG(exception)) { return 0; } property_offset = prop_info->offset; - if (!ZEND_TYPE_IS_SET(prop_info->type)) { - prop_info = NULL; - } goto try_again; } diff --git a/ext/reflection/tests/property_hooks/ReflectionProperty_isInitialized.phpt b/ext/reflection/tests/property_hooks/ReflectionProperty_isInitialized.phpt new file mode 100644 index 00000000000..ed1c88b215f --- /dev/null +++ b/ext/reflection/tests/property_hooks/ReflectionProperty_isInitialized.phpt @@ -0,0 +1,56 @@ +--TEST-- +ReflectionProperty::isInitialized() on hooked properties +--FILE-- + throw new Exception(); } + public $v2 { set { throw new Exception(); } } + // Backed + public $b1 { get => throw new Exception($this->b1); } + public string $b2 { get => throw new Exception($this->b2); } + public $b3 { set => throw new Exception(); } + public string $b4 { set => throw new Exception(); } +} + +$test = new Test(); +$rc = new ReflectionClass(Test::class); +foreach ($rc->getProperties() as $rp) { + echo $rp->getName(), "\n"; + var_dump($rp->isInitialized($test)); + try { + $rp->setRawValue($test, 42); + } catch (Error $e) {} + var_dump($rp->isInitialized($test)); +} + +?> +--EXPECT-- +p1 +bool(true) +bool(true) +p2 +bool(false) +bool(true) +v1 +bool(true) +bool(true) +v2 +bool(true) +bool(true) +b1 +bool(true) +bool(true) +b2 +bool(false) +bool(true) +b3 +bool(true) +bool(true) +b4 +bool(false) +bool(true)