diff --git a/NEWS b/NEWS index 3473112c34a..e74147772c2 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ PHP NEWS parent class implementations). (timwolla) . Fixed OSS Fuzz #62294 (Unsetting variable after ++/-- on string variable warning). (Girgias) + . Fixed bug GH-12215 (Module entry being overwritten causes type errors in + ext/dom). (nielsdos) - Core: . Fixed bug GH-12207 (memory leak when class using trait with doc block). @@ -27,6 +29,7 @@ PHP NEWS var_dump/print_r). (nielsdos) . Fixed bug GH-12208 (SimpleXML infinite loop when a cast is used inside a foreach). (nielsdos) + . Fixed bug #55098 (SimpleXML iteration produces infinite loop). (nielsdos) 14 Sep 2023, PHP 8.3.0RC2 diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 06d04e9a69b..b045b1ec2e9 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -77,25 +77,6 @@ static void _node_as_zval(php_sxe_object *sxe, xmlNodePtr node, zval *value, SXE } /* }}} */ -/* Important: this overwrites the iterator data, if you wish to keep it use php_sxe_get_first_node_non_destructive() instead! */ -static xmlNodePtr php_sxe_get_first_node(php_sxe_object *sxe, xmlNodePtr node) /* {{{ */ -{ - php_sxe_object *intern; - xmlNodePtr retnode = NULL; - - if (sxe && sxe->iter.type != SXE_ITER_NONE) { - php_sxe_reset_iterator(sxe, 1); - if (!Z_ISUNDEF(sxe->iter.data)) { - intern = Z_SXEOBJ_P(&sxe->iter.data); - GET_NODE(intern, retnode) - } - return retnode; - } else { - return node; - } -} -/* }}} */ - static xmlNodePtr php_sxe_get_first_node_non_destructive(php_sxe_object *sxe, xmlNodePtr node) { if (sxe && sxe->iter.type != SXE_ITER_NONE) { @@ -182,7 +163,7 @@ static xmlNodePtr sxe_get_element_by_name(php_sxe_object *sxe, xmlNodePtr node, if (sxe->iter.type == SXE_ITER_NONE) { sxe->iter.type = SXE_ITER_CHILD; } - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); sxe->iter.type = orgtype; } @@ -257,11 +238,11 @@ long_dim: if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = node ? node->properties : NULL; test = 0; if (!member && node && node->parent && @@ -309,7 +290,7 @@ long_dim: xmlNodePtr mynode = node; if (sxe->iter.type == SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } if (sxe->iter.type == SXE_ITER_NONE) { if (member && Z_LVAL_P(member) > 0) { @@ -445,12 +426,12 @@ long_dim: if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { mynode = node; - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = node ? node->properties : NULL; test = 0; if (!member && node && node->parent && @@ -696,7 +677,7 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt attribs = 0; elements = 1; if (sxe->iter.type == SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } } } @@ -704,11 +685,11 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = node ? node->properties : NULL; test = 0; } @@ -748,7 +729,7 @@ static int sxe_prop_dim_exists(zend_object *object, zval *member, int check_empt if (elements) { if (Z_TYPE_P(member) == IS_LONG) { if (sxe->iter.type == SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL); } else { @@ -820,7 +801,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements attribs = 0; elements = 1; if (sxe->iter.type == SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } } } @@ -828,11 +809,11 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); attr = node ? node->properties : NULL; test = 0; } @@ -869,7 +850,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements if (elements) { if (Z_TYPE_P(member) == IS_LONG) { if (sxe->iter.type == SXE_ITER_CHILD) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL); if (node) { @@ -1002,7 +983,7 @@ static int sxe_prop_is_empty(zend_object *object) /* {{{ */ } if (sxe->iter.type == SXE_ITER_ELEMENT) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } if (!node || node->type != XML_ENTITY_DECL) { attr = node ? (xmlAttrPtr)node->properties : NULL; @@ -1016,7 +997,7 @@ static int sxe_prop_is_empty(zend_object *object) /* {{{ */ } GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); is_empty = 1; ZVAL_UNDEF(&iter_data); if (node && sxe->iter.type != SXE_ITER_ATTRLIST) { @@ -1111,7 +1092,7 @@ static HashTable *sxe_get_prop_hash(zend_object *object, int is_debug) /* {{{ */ } if (is_debug || sxe->iter.type != SXE_ITER_CHILD) { if (sxe->iter.type == SXE_ITER_ELEMENT) { - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); } if (!node || node->type != XML_ENTITY_DECL) { attr = node ? (xmlAttrPtr)node->properties : NULL; @@ -1133,7 +1114,7 @@ static HashTable *sxe_get_prop_hash(zend_object *object, int is_debug) /* {{{ */ } GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (node && sxe->iter.type != SXE_ITER_ATTRLIST) { if (node->type == XML_ATTRIBUTE_NODE) { @@ -1292,7 +1273,7 @@ PHP_METHOD(SimpleXMLElement, xpath) } GET_NODE(sxe, nodeptr); - nodeptr = php_sxe_get_first_node(sxe, nodeptr); + nodeptr = php_sxe_get_first_node_non_destructive(sxe, nodeptr); if (!nodeptr) { return; } @@ -1401,7 +1382,7 @@ PHP_METHOD(SimpleXMLElement, asXML) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (!node) { RETURN_FALSE; @@ -1524,7 +1505,7 @@ PHP_METHOD(SimpleXMLElement, getNamespaces) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (node) { if (node->type == XML_ELEMENT_NODE) { @@ -1609,7 +1590,7 @@ PHP_METHOD(SimpleXMLElement, children) } GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (!node) { return; } @@ -1658,7 +1639,7 @@ PHP_METHOD(SimpleXMLElement, attributes) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (!node) { return; } @@ -1701,7 +1682,7 @@ PHP_METHOD(SimpleXMLElement, addChild) return; } - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (node == NULL) { php_error_docref(NULL, E_WARNING, "Cannot add child. Parent is not a permanent member of the XML tree"); @@ -1761,7 +1742,7 @@ PHP_METHOD(SimpleXMLElement, addAttribute) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node(sxe, node); + node = php_sxe_get_first_node_non_destructive(sxe, node); if (node && node->type != XML_ELEMENT_NODE) { node = node->parent; @@ -2608,7 +2589,7 @@ void *simplexml_export_node(zval *object) /* {{{ */ sxe = Z_SXEOBJ_P(object); GET_NODE(sxe, node); - return php_sxe_get_first_node(sxe, node); + return php_sxe_get_first_node_non_destructive(sxe, node); } /* }}} */ diff --git a/ext/simplexml/tests/bug55098.phpt b/ext/simplexml/tests/bug55098.phpt new file mode 100644 index 00000000000..71c8b424ecd --- /dev/null +++ b/ext/simplexml/tests/bug55098.phpt @@ -0,0 +1,92 @@ +--TEST-- +Bug #55098 (SimpleXML iteration produces infinite loop) +--EXTENSIONS-- +simplexml +--FILE-- +123"; +$xml = simplexml_load_string($xmlString); + +$nodes = $xml->a->b; + +function test($nodes, $name, $callable) { + echo "--- $name ---\n"; + foreach ($nodes as $nodeData) { + echo "nodeData: " . $nodeData . "\n"; + $callable($nodes); + } +} + +test($nodes, "asXml", fn ($n) => $n->asXml()); +test($nodes, "attributes", fn ($n) => $n->attributes()); +test($nodes, "children", fn ($n) => $n->children()); +test($nodes, "getNamespaces", fn ($n) => $n->getNamespaces()); +test($nodes, "xpath", fn ($n) => $n->xpath("/root/a/b")); +test($nodes, "var_dump", fn ($n) => var_dump($n)); +test($nodes, "manipulation combined with querying", function ($n) { + $n->addAttribute("attr", "value"); + (bool) $n["attr"]; + $n->addChild("child", "value"); + $n->outer[]->inner = "foo"; + (bool) $n->outer; + (bool) $n; + isset($n->outer); + isset($n["attr"]); + unset($n->outer); + unset($n["attr"]); + unset($n->child); +}); +?> +--EXPECT-- +--- asXml --- +nodeData: 1 +nodeData: 2 +nodeData: 3 +--- attributes --- +nodeData: 1 +nodeData: 2 +nodeData: 3 +--- children --- +nodeData: 1 +nodeData: 2 +nodeData: 3 +--- getNamespaces --- +nodeData: 1 +nodeData: 2 +nodeData: 3 +--- xpath --- +nodeData: 1 +nodeData: 2 +nodeData: 3 +--- var_dump --- +nodeData: 1 +object(SimpleXMLElement)#3 (3) { + [0]=> + string(1) "1" + [1]=> + string(1) "2" + [2]=> + string(1) "3" +} +nodeData: 2 +object(SimpleXMLElement)#3 (3) { + [0]=> + string(1) "1" + [1]=> + string(1) "2" + [2]=> + string(1) "3" +} +nodeData: 3 +object(SimpleXMLElement)#3 (3) { + [0]=> + string(1) "1" + [1]=> + string(1) "2" + [2]=> + string(1) "3" +} +--- manipulation combined with querying --- +nodeData: 1 +nodeData: 2 +nodeData: 3 diff --git a/ext/simplexml/tests/bug62639.phpt b/ext/simplexml/tests/bug62639.phpt index 286e6c5d8fd..517162d84eb 100644 --- a/ext/simplexml/tests/bug62639.phpt +++ b/ext/simplexml/tests/bug62639.phpt @@ -41,7 +41,7 @@ foreach ($a2->b->c->children() as $key => $value) { var_dump($value); }?> --EXPECT-- -object(A)#2 (2) { +object(A)#4 (2) { ["@attributes"]=> array(1) { ["attr"]=> @@ -50,7 +50,7 @@ object(A)#2 (2) { [0]=> string(10) "Some Value" } -object(A)#3 (2) { +object(A)#6 (2) { ["@attributes"]=> array(1) { ["attr"]=> diff --git a/ext/standard/dl.c b/ext/standard/dl.c index ebf8a3a507e..04670fc0adf 100644 --- a/ext/standard/dl.c +++ b/ext/standard/dl.c @@ -230,15 +230,30 @@ PHPAPI int php_load_extension(const char *filename, int type, int start_now) DL_UNLOAD(handle); return FAILURE; } + + int old_type = module_entry->type; + int old_module_number = module_entry->module_number; + void *old_handle = module_entry->handle; + module_entry->type = type; module_entry->module_number = zend_next_free_module(); module_entry->handle = handle; - if ((module_entry = zend_register_module_ex(module_entry)) == NULL) { + zend_module_entry *added_module_entry; + if ((added_module_entry = zend_register_module_ex(module_entry)) == NULL) { + /* Module loading failed, potentially because the module was already loaded. + * It is especially important in that case to restore the old type, module_number, and handle. + * Overwriting the values for an already-loaded module causes problem when these fields are used + * to uniquely identify module boundaries (e.g. in dom and reflection). */ + module_entry->type = old_type; + module_entry->module_number = old_module_number; + module_entry->handle = old_handle; DL_UNLOAD(handle); return FAILURE; } + module_entry = added_module_entry; + if ((type == MODULE_TEMPORARY || start_now) && zend_startup_module_ex(module_entry) == FAILURE) { DL_UNLOAD(handle); return FAILURE;