From 4546f487ed1581795c3b7ee8921faba1e219e7b8 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 22 Jun 2025 00:17:17 +0200 Subject: [PATCH 1/3] Don't use the obj_map cache for attributes Doing so will cause inconsistencies. This was revealed to be inconsistent after decoupling the nodemap and nodelist code. --- ext/dom/obj_map.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/ext/dom/obj_map.c b/ext/dom/obj_map.c index f2a93fe3ef870..f562058eeb797 100644 --- a/ext/dom/obj_map.c +++ b/ext/dom/obj_map.c @@ -204,13 +204,24 @@ static xmlNodePtr dom_map_get_attr_start(xmlNodePtr node) return (xmlNodePtr) node->properties; } -static void dom_map_get_chain_item(dom_nnodemap_object *map, zend_long index, zval *return_value, xmlNodePtr (*get_start)(xmlNodePtr)) +static void dom_map_get_attributes_item(dom_nnodemap_object *map, zend_long index, zval *return_value) +{ + xmlNodePtr nodep = dom_object_get_node(map->baseobj); + xmlNodePtr itemnode = NULL; + if (nodep && index >= 0) { + itemnode = dom_map_get_attr_start(nodep); + for (; index > 0 && itemnode; itemnode = itemnode->next, index--); + } + dom_ret_node_to_zobj(map, itemnode, return_value); +} + +static void dom_map_get_nodes_item(dom_nnodemap_object *map, zend_long index, zval *return_value) { xmlNodePtr nodep = dom_object_get_node(map->baseobj); xmlNodePtr itemnode = NULL; if (nodep && index >= 0) { dom_node_idx_pair start_point = dom_obj_map_get_start_point(map, nodep, index); - itemnode = start_point.node ? start_point.node : get_start(nodep); + itemnode = start_point.node ? start_point.node : dom_nodelist_iter_start_first_child(nodep); for (; start_point.index > 0 && itemnode; itemnode = itemnode->next, start_point.index--); } dom_ret_node_to_zobj(map, itemnode, return_value); @@ -219,16 +230,6 @@ static void dom_map_get_chain_item(dom_nnodemap_object *map, zend_long index, zv } } -static void dom_map_get_attributes_item(dom_nnodemap_object *map, zend_long index, zval *return_value) -{ - dom_map_get_chain_item(map, index, return_value, dom_map_get_attr_start); -} - -static void dom_map_get_nodes_item(dom_nnodemap_object *map, zend_long index, zval *return_value) -{ - dom_map_get_chain_item(map, index, return_value, dom_nodelist_iter_start_first_child); -} - static void dom_map_get_by_tag_name_item(dom_nnodemap_object *map, zend_long index, zval *return_value) { xmlNodePtr nodep = dom_object_get_node(map->baseobj); @@ -352,7 +353,7 @@ const php_dom_obj_map_handler php_dom_obj_map_attributes = { .get_item = dom_map_get_attributes_item, .get_named_item = dom_map_get_named_item_prop, .has_named_item = dom_map_has_named_item_prop, - .use_cache = true, + .use_cache = false, .nameless = false, }; From 4e6e9cf37f728ee8dc5adada287c04b251b5adf6 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 22 Jun 2025 00:30:26 +0200 Subject: [PATCH 2/3] merge functions --- ext/dom/obj_map.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/ext/dom/obj_map.c b/ext/dom/obj_map.c index f562058eeb797..1455eeaad54a2 100644 --- a/ext/dom/obj_map.c +++ b/ext/dom/obj_map.c @@ -198,18 +198,13 @@ static void dom_map_cache_obj(dom_nnodemap_object *map, xmlNodePtr itemnode, zen map->cached_obj = cached_obj; } -static xmlNodePtr dom_map_get_attr_start(xmlNodePtr node) -{ - ZEND_ASSERT(node->type == XML_ELEMENT_NODE); - return (xmlNodePtr) node->properties; -} - static void dom_map_get_attributes_item(dom_nnodemap_object *map, zend_long index, zval *return_value) { xmlNodePtr nodep = dom_object_get_node(map->baseobj); xmlNodePtr itemnode = NULL; if (nodep && index >= 0) { - itemnode = dom_map_get_attr_start(nodep); + ZEND_ASSERT(nodep->type == XML_ELEMENT_NODE); + itemnode = (xmlNodePtr) nodep->properties; for (; index > 0 && itemnode; itemnode = itemnode->next, index--); } dom_ret_node_to_zobj(map, itemnode, return_value); From 6d22243e7ea548eb3dca00869a30f130f9a8cbed Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 22 Jun 2025 00:57:35 +0200 Subject: [PATCH 3/3] add forgotten test --- .../common/attr_named_node_map_cache.phpt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 ext/dom/tests/modern/common/attr_named_node_map_cache.phpt diff --git a/ext/dom/tests/modern/common/attr_named_node_map_cache.phpt b/ext/dom/tests/modern/common/attr_named_node_map_cache.phpt new file mode 100644 index 0000000000000..18dedaddb9a9e --- /dev/null +++ b/ext/dom/tests/modern/common/attr_named_node_map_cache.phpt @@ -0,0 +1,17 @@ +--TEST-- +Attribute named node map cache +--EXTENSIONS-- +dom +--FILE-- +'); +$attrs = $dom->documentElement->attributes; +var_dump($attrs[1]->nodeName); +$dom->documentElement->removeAttribute('b'); +var_dump($attrs[1]->nodeName); + +?> +--EXPECT-- +string(1) "b" +string(1) "c"