From 28551ce84602a6963d1c4d74741022fa06c2b6d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Thu, 26 Sep 2024 23:57:10 +0200 Subject: [PATCH 1/3] Fix property_exists() for XMLReader --- ext/xmlreader/php_xmlreader.c | 20 +++++++++ ext/xmlreader/tests/virtual_properties2.phpt | 46 ++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 ext/xmlreader/tests/virtual_properties2.phpt diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c index 3f834b7155133..552c451d77e2f 100644 --- a/ext/xmlreader/php_xmlreader.c +++ b/ext/xmlreader/php_xmlreader.c @@ -123,6 +123,25 @@ zval *xmlreader_get_property_ptr_ptr(zend_object *object, zend_string *name, int } /* }}} */ +static int xmlreader_has_property(zend_object *object, zend_string *name, int type, void **cache_slot) +{ + xmlreader_object *obj = php_xmlreader_fetch_object(object); + xmlreader_prop_handler *hnd = zend_hash_find_ptr(&xmlreader_prop_handlers, name); + + if (hnd != NULL) { + zval rv; + if (xmlreader_property_reader(obj, hnd, &rv) == FAILURE) { + return 0; + } else { + zval_ptr_dtor(&rv); + return 1; + } + } + + return zend_std_has_property(object, name, type, cache_slot); +} + + /* {{{ xmlreader_read_property */ zval *xmlreader_read_property(zend_object *object, zend_string *name, int type, void **cache_slot, zval *rv) { @@ -1293,6 +1312,7 @@ PHP_MINIT_FUNCTION(xmlreader) memcpy(&xmlreader_object_handlers, &std_object_handlers, sizeof(zend_object_handlers)); xmlreader_object_handlers.offset = XtOffsetOf(xmlreader_object, std); xmlreader_object_handlers.free_obj = xmlreader_objects_free_storage; + xmlreader_object_handlers.has_property = xmlreader_has_property; xmlreader_object_handlers.read_property = xmlreader_read_property; xmlreader_object_handlers.write_property = xmlreader_write_property; xmlreader_object_handlers.get_property_ptr_ptr = xmlreader_get_property_ptr_ptr; diff --git a/ext/xmlreader/tests/virtual_properties2.phpt b/ext/xmlreader/tests/virtual_properties2.phpt new file mode 100644 index 0000000000000..8f879891c51b4 --- /dev/null +++ b/ext/xmlreader/tests/virtual_properties2.phpt @@ -0,0 +1,46 @@ +--TEST-- +Virtual property existence tests +--EXTENSIONS-- +xmlreader +--FILE-- +attributeCount)); +var_dump(empty($di->attributeCount)); +var_dump(property_exists($di, "attributeCount")); + +var_dump(isset($di->baseURI)); +var_dump(empty($di->baseURI)); +var_dump(property_exists($di, "baseURI")); + +var_dump(isset($di->depth)); +var_dump(empty($di->depth)); +var_dump(property_exists($di, "depth")); + +var_dump(isset($di->hasAttributes)); +var_dump(empty($di->hasAttributes)); +var_dump(property_exists($di, "hasAttributes")); + +var_dump(isset($di->hasValue)); +var_dump(empty($di->hasValue)); +var_dump(property_exists($di, "hasValue")); + +?> +--EXPECTF-- +bool(true) +bool(false) +bool(true) +bool(true) +bool(false) +bool(true) +bool(true) +bool(false) +bool(true) +bool(true) +bool(false) +bool(true) +bool(true) +bool(false) +bool(true) From 5f76e86427cac46f6bc4234b55fc1baaae649192 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Fri, 27 Sep 2024 08:50:29 +0200 Subject: [PATCH 2/3] Fix has_property handler + add unset handler --- ext/xmlreader/php_xmlreader.c | 27 ++++++++-- ext/xmlreader/tests/virtual_properties2.phpt | 44 ++++++++-------- ext/xmlreader/tests/virtual_properties3.phpt | 55 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 25 deletions(-) create mode 100644 ext/xmlreader/tests/virtual_properties3.phpt diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c index 552c451d77e2f..f02d58f724a9b 100644 --- a/ext/xmlreader/php_xmlreader.c +++ b/ext/xmlreader/php_xmlreader.c @@ -129,13 +129,21 @@ static int xmlreader_has_property(zend_object *object, zend_string *name, int ty xmlreader_prop_handler *hnd = zend_hash_find_ptr(&xmlreader_prop_handlers, name); if (hnd != NULL) { + if (type == ZEND_PROPERTY_EXISTS) { + return 1; + } + zval rv; if (xmlreader_property_reader(obj, hnd, &rv) == FAILURE) { return 0; - } else { - zval_ptr_dtor(&rv); - return 1; } + + if (type == ZEND_PROPERTY_NOT_EMPTY) { + return zend_is_true(&rv); + } + + ZEND_ASSERT(type == ZEND_PROPERTY_ISSET); + return (Z_TYPE(rv) != IS_NULL); } return zend_std_has_property(object, name, type, cache_slot); @@ -178,6 +186,18 @@ zval *xmlreader_write_property(zend_object *object, zend_string *name, zval *val } /* }}} */ +void xmlreader_unset_property(zend_object *object, zend_string *name, void **cache_slot) +{ + xmlreader_prop_handler *hnd = zend_hash_find_ptr(&xmlreader_prop_handlers, name); + + if (hnd != NULL) { + zend_throw_error(NULL, "Cannot unset %s::$%s", ZSTR_VAL(object->ce->name), ZSTR_VAL(name)); + return; + } + + zend_std_unset_property(object, name, cache_slot); +} + /* {{{ */ static zend_function *xmlreader_get_method(zend_object **obj, zend_string *name, const zval *key) { @@ -1315,6 +1335,7 @@ PHP_MINIT_FUNCTION(xmlreader) xmlreader_object_handlers.has_property = xmlreader_has_property; xmlreader_object_handlers.read_property = xmlreader_read_property; xmlreader_object_handlers.write_property = xmlreader_write_property; + xmlreader_object_handlers.unset_property = xmlreader_unset_property; xmlreader_object_handlers.get_property_ptr_ptr = xmlreader_get_property_ptr_ptr; xmlreader_object_handlers.get_method = xmlreader_get_method; xmlreader_object_handlers.clone_obj = NULL; diff --git a/ext/xmlreader/tests/virtual_properties2.phpt b/ext/xmlreader/tests/virtual_properties2.phpt index 8f879891c51b4..c748f1fc59ca0 100644 --- a/ext/xmlreader/tests/virtual_properties2.phpt +++ b/ext/xmlreader/tests/virtual_properties2.phpt @@ -5,42 +5,42 @@ xmlreader --FILE-- attributeCount)); -var_dump(empty($di->attributeCount)); -var_dump(property_exists($di, "attributeCount")); +var_dump(isset($reader->attributeCount)); +var_dump(empty($reader->attributeCount)); +var_dump(property_exists($reader, "attributeCount")); -var_dump(isset($di->baseURI)); -var_dump(empty($di->baseURI)); -var_dump(property_exists($di, "baseURI")); +var_dump(isset($reader->baseURI)); +var_dump(empty($reader->baseURI)); +var_dump(property_exists($reader, "baseURI")); -var_dump(isset($di->depth)); -var_dump(empty($di->depth)); -var_dump(property_exists($di, "depth")); +var_dump(isset($reader->depth)); +var_dump(empty($reader->depth)); +var_dump(property_exists($reader, "depth")); -var_dump(isset($di->hasAttributes)); -var_dump(empty($di->hasAttributes)); -var_dump(property_exists($di, "hasAttributes")); +var_dump(isset($reader->hasAttributes)); +var_dump(empty($reader->hasAttributes)); +var_dump(property_exists($reader, "hasAttributes")); -var_dump(isset($di->hasValue)); -var_dump(empty($di->hasValue)); -var_dump(property_exists($di, "hasValue")); +var_dump(isset($reader->hasValue)); +var_dump(empty($reader->hasValue)); +var_dump(property_exists($reader, "hasValue")); ?> ---EXPECTF-- +--EXPECT-- +bool(true) +bool(true) +bool(true) +bool(true) +bool(true) bool(true) -bool(false) bool(true) bool(true) -bool(false) bool(true) bool(true) -bool(false) bool(true) bool(true) -bool(false) bool(true) bool(true) -bool(false) bool(true) diff --git a/ext/xmlreader/tests/virtual_properties3.phpt b/ext/xmlreader/tests/virtual_properties3.phpt new file mode 100644 index 0000000000000..ad016c4db7647 --- /dev/null +++ b/ext/xmlreader/tests/virtual_properties3.phpt @@ -0,0 +1,55 @@ +--TEST-- +Virtual property unset tests +--EXTENSIONS-- +xmlreader +--FILE-- +attributeCount); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +try { + unset($reader->baseURI); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +try { + unset($reader->depth); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +try { + unset($reader->hasAttributes); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +try { + unset($reader->hasValue); +} catch (Error $e) { + echo $e->getMessage() . "\n"; +} + +unset($reader->x); +var_dump(isset($reader->x)); + +?> +--EXPECT-- +Cannot unset MyXMLReader::$attributeCount +Cannot unset MyXMLReader::$baseURI +Cannot unset MyXMLReader::$depth +Cannot unset MyXMLReader::$hasAttributes +Cannot unset MyXMLReader::$hasValue +bool(false) From ffb41eaab6cb225dc7782ad45267dc6171ff9c68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Fri, 27 Sep 2024 20:55:42 +0200 Subject: [PATCH 3/3] Fix possible memory leak --- ext/xmlreader/php_xmlreader.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c index f02d58f724a9b..fb05339522760 100644 --- a/ext/xmlreader/php_xmlreader.c +++ b/ext/xmlreader/php_xmlreader.c @@ -138,12 +138,19 @@ static int xmlreader_has_property(zend_object *object, zend_string *name, int ty return 0; } + bool result; + if (type == ZEND_PROPERTY_NOT_EMPTY) { - return zend_is_true(&rv); + result = zend_is_true(&rv); + } else if (type == ZEND_PROPERTY_ISSET) { + result = (Z_TYPE(rv) != IS_NULL); + } else { + ZEND_UNREACHABLE(); } - ZEND_ASSERT(type == ZEND_PROPERTY_ISSET); - return (Z_TYPE(rv) != IS_NULL); + zval_ptr_dtor(&rv); + + return result; } return zend_std_has_property(object, name, type, cache_slot);