From b842ea4fa820536068604c689a5b36374532fd22 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:53:01 +0200 Subject: [PATCH] Apply SimpleXML iterator fixes only on master Many methods in SimpleXML reset the iterator when called. This has the consequence that mixing these operations with loops can cause infinite loops, or the loss of iteration data. Some people may however rely on the resetting behaviour. To prevent unintended breaks in stable branches, let's only apply the fix to master. This reverts GH-12193, GH-12229, GG-12247 for stable branches while keeping them on master, adding a note in UPGRADING as well. --- NEWS | 5 -- ext/simplexml/simplexml.c | 62 ++++++++++----------- ext/simplexml/tests/bug55098.phpt | 92 ------------------------------- ext/simplexml/tests/bug62639.phpt | 4 +- ext/simplexml/tests/gh12192.phpt | 37 ------------- ext/simplexml/tests/gh12208.phpt | 26 --------- 6 files changed, 33 insertions(+), 193 deletions(-) delete mode 100644 ext/simplexml/tests/bug55098.phpt delete mode 100644 ext/simplexml/tests/gh12192.phpt delete mode 100644 ext/simplexml/tests/gh12208.phpt diff --git a/NEWS b/NEWS index a6107b1a566..a000e6c1b47 100644 --- a/NEWS +++ b/NEWS @@ -39,13 +39,8 @@ PHP NEWS - SimpleXML: . Fixed bug GH-12170 (Can't use xpath with comments in SimpleXML). (nielsdos) - . Fixed bug GH-12192 (SimpleXML infinite loop when getName() is called - within foreach). (nielsdos) . Fixed bug GH-12223 (Entity reference produces infinite loop in 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) . Fixed bug GH-12167 (Unable to get processing instruction contents in SimpleXML). (nielsdos) . Fixed bug GH-12169 (Unable to get comment contents in SimpleXML). diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 5622fbf47fd..9bcfe1e641b 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -78,10 +78,10 @@ static void _node_as_zval(php_sxe_object *sxe, xmlNodePtr node, zval *value, SXE } /* }}} */ -static xmlNodePtr php_sxe_get_first_node_non_destructive(php_sxe_object *sxe, xmlNodePtr node) +static xmlNodePtr php_sxe_get_first_node(php_sxe_object *sxe, xmlNodePtr node) { if (sxe && sxe->iter.type != SXE_ITER_NONE) { - return php_sxe_reset_iterator_no_clear_iter_data(sxe, false); + return php_sxe_reset_iterator(sxe, 1); } else { return node; } @@ -165,7 +165,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); sxe->iter.type = orgtype; } @@ -251,11 +251,11 @@ long_dim: if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = node ? node->properties : NULL; test = 0; if (!member && node && node->parent && @@ -303,7 +303,7 @@ long_dim: xmlNodePtr mynode = node; if (sxe->iter.type == SXE_ITER_CHILD) { - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } if (sxe->iter.type == SXE_ITER_NONE) { if (member && Z_LVAL_P(member) > 0) { @@ -437,12 +437,12 @@ long_dim: if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = node ? node->properties : NULL; test = 0; if (!member && node && node->parent && @@ -688,7 +688,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } } } @@ -696,11 +696,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = node ? node->properties : NULL; test = 0; } @@ -740,7 +740,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL); } else { @@ -810,7 +810,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } } } @@ -818,11 +818,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = (xmlAttrPtr)node; test = sxe->iter.name != NULL; } else if (sxe->iter.type != SXE_ITER_CHILD) { - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); attr = node ? node->properties : NULL; test = 0; } @@ -859,7 +859,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL); if (node) { @@ -992,7 +992,7 @@ static int sxe_prop_is_empty(zend_object *object) /* {{{ */ } if (sxe->iter.type == SXE_ITER_ELEMENT) { - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } if (!node || node->type != XML_ENTITY_DECL) { attr = node ? (xmlAttrPtr)node->properties : NULL; @@ -1006,7 +1006,7 @@ static int sxe_prop_is_empty(zend_object *object) /* {{{ */ } GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); is_empty = 1; ZVAL_UNDEF(&iter_data); if (node && sxe->iter.type != SXE_ITER_ATTRLIST) { @@ -1101,7 +1101,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_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); } if (!node || node->type != XML_ENTITY_DECL) { attr = node ? (xmlAttrPtr)node->properties : NULL; @@ -1123,7 +1123,7 @@ static HashTable *sxe_get_prop_hash(zend_object *object, int is_debug) /* {{{ */ } GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (node && sxe->iter.type != SXE_ITER_ATTRLIST) { if (node->type == XML_ATTRIBUTE_NODE) { @@ -1282,7 +1282,7 @@ PHP_METHOD(SimpleXMLElement, xpath) } GET_NODE(sxe, nodeptr); - nodeptr = php_sxe_get_first_node_non_destructive(sxe, nodeptr); + nodeptr = php_sxe_get_first_node(sxe, nodeptr); if (!nodeptr) { return; } @@ -1391,7 +1391,7 @@ PHP_METHOD(SimpleXMLElement, asXML) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (!node) { RETURN_FALSE; @@ -1514,7 +1514,7 @@ PHP_METHOD(SimpleXMLElement, getNamespaces) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (node) { if (node->type == XML_ELEMENT_NODE) { @@ -1599,7 +1599,7 @@ PHP_METHOD(SimpleXMLElement, children) } GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (!node) { return; } @@ -1623,7 +1623,7 @@ PHP_METHOD(SimpleXMLElement, getName) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (node) { namelen = xmlStrlen(node->name); RETURN_STRINGL((char*)node->name, namelen); @@ -1648,7 +1648,7 @@ PHP_METHOD(SimpleXMLElement, attributes) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (!node) { return; } @@ -1689,7 +1689,7 @@ PHP_METHOD(SimpleXMLElement, addChild) return; } - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (node == NULL) { php_error_docref(NULL, E_WARNING, "Cannot add child. Parent is not a permanent member of the XML tree"); @@ -1749,7 +1749,7 @@ PHP_METHOD(SimpleXMLElement, addAttribute) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - node = php_sxe_get_first_node_non_destructive(sxe, node); + node = php_sxe_get_first_node(sxe, node); if (node && node->type != XML_ELEMENT_NODE) { node = node->parent; @@ -1842,7 +1842,7 @@ static int sxe_object_cast_ex(zend_object *readobj, zval *writeobj, int type) sxe = php_sxe_fetch_object(readobj); if (type == _IS_BOOL) { - node = php_sxe_get_first_node_non_destructive(sxe, NULL); + node = php_sxe_get_first_node(sxe, NULL); if (node) { ZVAL_TRUE(writeobj); } else { @@ -1852,7 +1852,7 @@ static int sxe_object_cast_ex(zend_object *readobj, zval *writeobj, int type) } if (sxe->iter.type != SXE_ITER_NONE) { - node = php_sxe_get_first_node_non_destructive(sxe, NULL); + node = php_sxe_get_first_node(sxe, NULL); if (node) { contents = xmlNodeListGetString((xmlDocPtr) sxe->document->ptr, node->children, 1); } @@ -2600,7 +2600,7 @@ void *simplexml_export_node(zval *object) /* {{{ */ sxe = Z_SXEOBJ_P(object); GET_NODE(sxe, node); - return php_sxe_get_first_node_non_destructive(sxe, node); + return php_sxe_get_first_node(sxe, node); } /* }}} */ diff --git a/ext/simplexml/tests/bug55098.phpt b/ext/simplexml/tests/bug55098.phpt deleted file mode 100644 index 71c8b424ecd..00000000000 --- a/ext/simplexml/tests/bug55098.phpt +++ /dev/null @@ -1,92 +0,0 @@ ---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 517162d84eb..286e6c5d8fd 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)#4 (2) { +object(A)#2 (2) { ["@attributes"]=> array(1) { ["attr"]=> @@ -50,7 +50,7 @@ object(A)#4 (2) { [0]=> string(10) "Some Value" } -object(A)#6 (2) { +object(A)#3 (2) { ["@attributes"]=> array(1) { ["attr"]=> diff --git a/ext/simplexml/tests/gh12192.phpt b/ext/simplexml/tests/gh12192.phpt deleted file mode 100644 index 4d648495a10..00000000000 --- a/ext/simplexml/tests/gh12192.phpt +++ /dev/null @@ -1,37 +0,0 @@ ---TEST-- -GH-12192 (SimpleXML infinite loop when getName() is called within foreach) ---EXTENSIONS-- -simplexml ---FILE-- -12"; -$xml = simplexml_load_string($xml); - -$a = $xml->a; - -foreach ($a as $test) { - echo "Iteration\n"; - var_dump($a->key()); - var_dump($a->getName()); - var_dump((string) $test); -} - -var_dump($a); - -?> ---EXPECT-- -Iteration -string(1) "a" -string(1) "a" -string(1) "1" -Iteration -string(1) "a" -string(1) "a" -string(1) "2" -object(SimpleXMLElement)#2 (2) { - [0]=> - string(1) "1" - [1]=> - string(1) "2" -} diff --git a/ext/simplexml/tests/gh12208.phpt b/ext/simplexml/tests/gh12208.phpt deleted file mode 100644 index da3a997a504..00000000000 --- a/ext/simplexml/tests/gh12208.phpt +++ /dev/null @@ -1,26 +0,0 @@ ---TEST-- -GH-12208 (SimpleXML infinite loop when a cast is used inside a foreach) ---EXTENSIONS-- -simplexml ---FILE-- -12"; -$xml = simplexml_load_string($xml); - -$a = $xml->a; - -foreach ($a as $test) { - var_dump((string) $a->current()); - var_dump((string) $a); - var_dump((bool) $a); -} - -?> ---EXPECT-- -string(1) "1" -string(1) "1" -bool(true) -string(1) "2" -string(1) "1" -bool(true)