From 0b8ed4181c8d6f82e0dee48d8a6f90e32fb5ea84 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Thu, 16 Apr 2026 06:08:16 +0100 Subject: [PATCH 1/4] Fixes GH-21768: ext/reflection: assertion failure on internal virtual properties. ReflectionProperty::isReadable() and isWritable() asserted prop->hooks being set whenever ZEND_ACC_VIRTUAL is set. Internal classes such as DOMDocument declare virtual properties backed by the object read/write handlers without hooks, tripping the assertion. Assertion introduced in e4f727d61eb. --- ext/reflection/php_reflection.c | 6 ++---- ext/reflection/tests/gh21768.phpt | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 ext/reflection/tests/gh21768.phpt diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 7552b00c1151..13e028f8a2ff 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -6743,8 +6743,7 @@ ZEND_METHOD(ReflectionProperty, isReadable) } if (prop->flags & ZEND_ACC_VIRTUAL) { - ZEND_ASSERT(prop->hooks); - if (!prop->hooks[ZEND_PROPERTY_HOOK_GET]) { + if (prop->hooks && !prop->hooks[ZEND_PROPERTY_HOOK_GET]) { RETURN_FALSE; } } else if (obj && (!prop->hooks || !prop->hooks[ZEND_PROPERTY_HOOK_GET])) { @@ -6836,8 +6835,7 @@ ZEND_METHOD(ReflectionProperty, isWritable) } if (prop->flags & ZEND_ACC_VIRTUAL) { - ZEND_ASSERT(prop->hooks); - if (!prop->hooks[ZEND_PROPERTY_HOOK_SET]) { + if (prop->hooks && !prop->hooks[ZEND_PROPERTY_HOOK_SET]) { RETURN_FALSE; } } else if (obj && (prop->flags & ZEND_ACC_READONLY)) { diff --git a/ext/reflection/tests/gh21768.phpt b/ext/reflection/tests/gh21768.phpt new file mode 100644 index 000000000000..422239120dbe --- /dev/null +++ b/ext/reflection/tests/gh21768.phpt @@ -0,0 +1,21 @@ +--TEST-- +GH-21768: Assertion failure in ReflectionProperty::is{Readable,Writable}() on internal virtual properties +--EXTENSIONS-- +dom +--FILE-- +getProperties() as $rp) { + if (!$rp->isVirtual()) + continue; + if (!$rp->isReadable(null)) + die("$rp should be readable"); + if (!$rp->isWritable(null)) + die("$rp should be writable"); +} +echo "done\n"; + +?> +--EXPECT-- +done From fbb0059ee542c9d6624d71770b586dd9742028f8 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 17 Apr 2026 19:56:19 +0100 Subject: [PATCH 2/4] ext/reflection: handle virtual+readonly in ReflectionProperty::isWritable(). Follow-up to review feedback from @iluuu1994 on GH-21769. Gate the ZEND_ACC_VIRTUAL branch on prop->hooks being set, and short-circuit virtual properties that also carry ZEND_ACC_READONLY to RETURN_FALSE (no backing slot, treat as always initialized). Covers legitimate virtual+readonly uses outside of DOM. --- ext/reflection/php_reflection.c | 40 +++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index 13e028f8a2ff..5bdd5e18bf22 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -6742,8 +6742,8 @@ ZEND_METHOD(ReflectionProperty, isReadable) RETURN_FALSE; } - if (prop->flags & ZEND_ACC_VIRTUAL) { - if (prop->hooks && !prop->hooks[ZEND_PROPERTY_HOOK_GET]) { + if ((prop->flags & ZEND_ACC_VIRTUAL) && prop->hooks) { + if (!prop->hooks[ZEND_PROPERTY_HOOK_GET]) { RETURN_FALSE; } } else if (obj && (!prop->hooks || !prop->hooks[ZEND_PROPERTY_HOOK_GET])) { @@ -6834,25 +6834,31 @@ ZEND_METHOD(ReflectionProperty, isWritable) RETURN_FALSE; } - if (prop->flags & ZEND_ACC_VIRTUAL) { - if (prop->hooks && !prop->hooks[ZEND_PROPERTY_HOOK_SET]) { + if ((prop->flags & ZEND_ACC_VIRTUAL) && prop->hooks) { + if (!prop->hooks[ZEND_PROPERTY_HOOK_SET]) { RETURN_FALSE; } - } else if (obj && (prop->flags & ZEND_ACC_READONLY)) { + } else if (prop->flags & ZEND_ACC_VIRTUAL) { + if (prop->flags & ZEND_ACC_READONLY) { + RETURN_FALSE; + } + } else if ((prop->flags & ZEND_ACC_READONLY)) { + if (obj) { retry:; - zval *prop_val = OBJ_PROP(obj, prop->offset); - if (Z_TYPE_P(prop_val) == IS_UNDEF - && zend_lazy_object_must_init(obj) - && (Z_PROP_FLAG_P(prop_val) & IS_PROP_LAZY) - ) { - obj = zend_lazy_object_init(obj); - if (!obj) { - RETURN_THROWS(); + zval *prop_val = OBJ_PROP(obj, prop->offset); + if (Z_TYPE_P(prop_val) == IS_UNDEF + && zend_lazy_object_must_init(obj) + && (Z_PROP_FLAG_P(prop_val) & IS_PROP_LAZY) + ) { + obj = zend_lazy_object_init(obj); + if (!obj) { + RETURN_THROWS(); + } + goto retry; + } + if (Z_TYPE_P(prop_val) != IS_UNDEF && !(Z_PROP_FLAG_P(prop_val) & IS_PROP_REINITABLE)) { + RETURN_FALSE; } - goto retry; - } - if (Z_TYPE_P(prop_val) != IS_UNDEF && !(Z_PROP_FLAG_P(prop_val) & IS_PROP_REINITABLE)) { - RETURN_FALSE; } } From 52b402cbf4294631f3cbe607156c6010d182e058 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sat, 18 Apr 2026 17:37:21 +0100 Subject: [PATCH 3/4] update test --- ext/reflection/tests/gh21768.phpt | 86 +++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 4 deletions(-) diff --git a/ext/reflection/tests/gh21768.phpt b/ext/reflection/tests/gh21768.phpt index 422239120dbe..69c50c5a4482 100644 --- a/ext/reflection/tests/gh21768.phpt +++ b/ext/reflection/tests/gh21768.phpt @@ -9,13 +9,91 @@ $rc = new ReflectionClass('DOMDocument'); foreach ($rc->getProperties() as $rp) { if (!$rp->isVirtual()) continue; - if (!$rp->isReadable(null)) - die("$rp should be readable"); - if (!$rp->isWritable(null)) - die("$rp should be writable"); + var_dump($rp->isReadable(null)); + var_dump($rp->isWritable(null)); } echo "done\n"; ?> --EXPECT-- +bool(true) +bool(false) +bool(true) +bool(true) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(true) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(true) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(true) +bool(true) +bool(false) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(false) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) +bool(false) +bool(true) +bool(false) +bool(true) +bool(false) done From 7e4998df2684540c1faa70b979cdaf33d20dfc70 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Fri, 30 Oct 2026 10:30:29 +0000 Subject: [PATCH 4/4] feedback --- ext/reflection/tests/gh21768.phpt | 131 +++++++++++------------------- 1 file changed, 46 insertions(+), 85 deletions(-) diff --git a/ext/reflection/tests/gh21768.phpt b/ext/reflection/tests/gh21768.phpt index 69c50c5a4482..eabc7f89f123 100644 --- a/ext/reflection/tests/gh21768.phpt +++ b/ext/reflection/tests/gh21768.phpt @@ -7,93 +7,54 @@ dom $rc = new ReflectionClass('DOMDocument'); foreach ($rc->getProperties() as $rp) { - if (!$rp->isVirtual()) - continue; - var_dump($rp->isReadable(null)); - var_dump($rp->isWritable(null)); + if (!$rp->isVirtual()) { + continue; + } + echo $rp->getName(), ': readable=', var_export($rp->isReadable(null), true), + ', writable=', var_export($rp->isWritable(null), true), PHP_EOL; } -echo "done\n"; +echo "done", PHP_EOL; ?> --EXPECT-- -bool(true) -bool(false) -bool(true) -bool(true) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(true) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(true) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(true) -bool(true) -bool(false) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(false) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(true) -bool(false) -bool(true) -bool(false) -bool(true) -bool(false) +nodeName: readable=true, writable=false +nodeValue: readable=true, writable=true +nodeType: readable=true, writable=false +parentNode: readable=true, writable=false +parentElement: readable=true, writable=false +childNodes: readable=true, writable=false +firstChild: readable=true, writable=false +lastChild: readable=true, writable=false +previousSibling: readable=true, writable=false +nextSibling: readable=true, writable=false +attributes: readable=true, writable=false +isConnected: readable=true, writable=false +ownerDocument: readable=true, writable=false +namespaceURI: readable=true, writable=false +prefix: readable=true, writable=true +localName: readable=true, writable=false +baseURI: readable=true, writable=false +textContent: readable=true, writable=true +doctype: readable=true, writable=false +implementation: readable=true, writable=false +documentElement: readable=true, writable=false +actualEncoding: readable=true, writable=false +encoding: readable=true, writable=true +xmlEncoding: readable=true, writable=false +standalone: readable=true, writable=true +xmlStandalone: readable=true, writable=true +version: readable=true, writable=true +xmlVersion: readable=true, writable=true +strictErrorChecking: readable=true, writable=true +documentURI: readable=true, writable=true +config: readable=true, writable=false +formatOutput: readable=true, writable=true +validateOnParse: readable=true, writable=true +resolveExternals: readable=true, writable=true +preserveWhiteSpace: readable=true, writable=true +recover: readable=true, writable=true +substituteEntities: readable=true, writable=true +firstElementChild: readable=true, writable=false +lastElementChild: readable=true, writable=false +childElementCount: readable=true, writable=false done