From 8464f0d9602702534b32fb5dbfc1fc681f56b734 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 09:37:53 -0400 Subject: [PATCH] dom: normalize null namespace in getNamedItemNS The named-item refactor moved getNamedItem and getNamedItemNS onto a shared lookup where a null namespace fell back to qualified-name matching, letting getNamedItemNS(null, name) return attributes that do have a namespace. Split the two entry points into dedicated handler methods: the null-namespace lookup now only matches attributes without a namespace (with HTML adjusted-local-name handling), while getNamedItem() and array/property access keep qualified-name matching. Sibling audit found no other affected paths; getAttributeNS(), hasAttributeNS() and getAttributeNodeNS() already use xmlHasNsProp. --- NEWS | 2 + ext/dom/namednodemap.c | 2 +- ext/dom/obj_map.c | 104 ++++++++++++++++-- ext/dom/obj_map.h | 4 + ext/dom/php_dom.c | 8 +- .../xml/getnamednodemap_null_ns_lookup.phpt | 41 +++++++ 6 files changed, 148 insertions(+), 13 deletions(-) create mode 100644 ext/dom/tests/modern/xml/getnamednodemap_null_ns_lookup.phpt diff --git a/NEWS b/NEWS index 283c90bad870..717fa5b06739 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,8 @@ PHP NEWS middle generator delegates again). (Lazizbek Ergashev) - DOM: + . Fixed DOMNamedNodeMap::getNamedItemNS() with a null namespace incorrectly + matching namespaced attributes. (iliaal) . Fixed a use-after-free when cloning a DOMNameSpaceNode after DOMDocument::xinclude(). (iliaal) . Fixed a crash in DOMXPath when a php:function callback receives a nodeset diff --git a/ext/dom/namednodemap.c b/ext/dom/namednodemap.c index 4964f836407c..9116eefc84ab 100644 --- a/ext/dom/namednodemap.c +++ b/ext/dom/namednodemap.c @@ -63,7 +63,7 @@ PHP_METHOD(DOMNamedNodeMap, getNamedItem) } dom_nnodemap_object *objmap = Z_DOMOBJ_P(ZEND_THIS)->ptr; - php_dom_obj_map_get_ns_named_item_into_zval(objmap, named, NULL, return_value); + php_dom_obj_map_get_named_item_into_zval(objmap, named, return_value); } /* }}} end dom_namednodemap_get_named_item */ diff --git a/ext/dom/obj_map.c b/ext/dom/obj_map.c index 800dadbfb422..9a147f61ad6e 100644 --- a/ext/dom/obj_map.c +++ b/ext/dom/obj_map.c @@ -486,6 +486,21 @@ void php_dom_obj_map_get_ns_named_item_into_zval(dom_nnodemap_object *objmap, co } } +void php_dom_obj_map_get_named_item_into_zval(dom_nnodemap_object *objmap, const zend_string *named, zval *return_value) +{ + xmlNodePtr itemnode = objmap->handler->get_named_item(objmap, named); + if (itemnode) { + DOM_RET_OBJ(itemnode, objmap->baseobj); + } else { + RETURN_NULL(); + } +} + +bool php_dom_obj_map_has_named_item(dom_nnodemap_object *objmap, const zend_string *named) +{ + return objmap->handler->has_named_item(objmap, named); +} + /********************** * === Named item === * **********************/ @@ -509,23 +524,78 @@ static xmlNodePtr dom_map_get_ns_named_item_notation(dom_nnodemap_object *map, c return NULL; } -static xmlNodePtr dom_map_get_ns_named_item_prop(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static xmlNodePtr dom_map_get_named_item_prop(dom_nnodemap_object *map, const zend_string *named) { xmlNodePtr nodep = dom_object_get_node(map->baseobj); if (nodep) { - if (ns) { - return (xmlNodePtr) xmlHasNsProp(nodep, BAD_CAST ZSTR_VAL(named), BAD_CAST ns); + if (php_dom_follow_spec_intern(map->baseobj)) { + return (xmlNodePtr) php_dom_get_attribute_node(nodep, BAD_CAST ZSTR_VAL(named), ZSTR_LEN(named)); } else { - if (php_dom_follow_spec_intern(map->baseobj)) { - return (xmlNodePtr) php_dom_get_attribute_node(nodep, BAD_CAST ZSTR_VAL(named), ZSTR_LEN(named)); - } else { - return (xmlNodePtr) xmlHasProp(nodep, BAD_CAST ZSTR_VAL(named)); - } + return (xmlNodePtr) xmlHasProp(nodep, BAD_CAST ZSTR_VAL(named)); } } return NULL; } +static bool dom_map_has_named_item_prop(dom_nnodemap_object *map, const zend_string *named) +{ + return dom_map_get_named_item_prop(map, named) != NULL; +} + +static bool dom_map_has_named_item_entity_fn(dom_nnodemap_object *map, const zend_string *named) +{ + return dom_map_has_ns_named_item_xmlht(map, named, NULL); +} + +static bool dom_map_has_named_item_null(dom_nnodemap_object *map, const zend_string *named) +{ + return false; +} + +static xmlNodePtr dom_map_get_named_item_entity_fn(dom_nnodemap_object *map, const zend_string *named) +{ + return dom_map_get_ns_named_item_entity(map, named, NULL); +} + +static xmlNodePtr dom_map_get_named_item_notation_fn(dom_nnodemap_object *map, const zend_string *named) +{ + return dom_map_get_ns_named_item_notation(map, named, NULL); +} + +static xmlNodePtr dom_map_get_ns_named_item_prop(dom_nnodemap_object *map, const zend_string *named, const char *ns) +{ + xmlNodePtr nodep = dom_object_get_node(map->baseobj); + if (nodep == NULL) { + return NULL; + } + if (ns != NULL) { + return (xmlNodePtr) xmlHasNsProp(nodep, BAD_CAST ZSTR_VAL(named), BAD_CAST ns); + } + if (!php_dom_follow_spec_intern(map->baseobj)) { + return (xmlNodePtr) xmlHasNsProp(nodep, BAD_CAST ZSTR_VAL(named), NULL); + } + const xmlChar *name = BAD_CAST ZSTR_VAL(named); + bool must_free_name = false; + if (php_dom_ns_is_html_and_document_is_html(nodep)) { + char *lowercase_copy = zend_str_tolower_dup_ex((char *) name, ZSTR_LEN(named)); + if (lowercase_copy != NULL) { + name = BAD_CAST lowercase_copy; + must_free_name = true; + } + } + xmlAttrPtr ret = NULL; + for (xmlAttrPtr attr = nodep->properties; attr != NULL; attr = attr->next) { + if (attr->ns == NULL && xmlStrEqual(attr->name, name)) { + ret = attr; + break; + } + } + if (must_free_name) { + efree((char *) name); + } + return (xmlNodePtr) ret; +} + static bool dom_map_has_ns_named_item_prop(dom_nnodemap_object *map, const zend_string *named, const char *ns) { return dom_map_get_ns_named_item_prop(map, named, ns) != NULL; @@ -546,6 +616,8 @@ static bool dom_map_has_ns_named_item_null(dom_nnodemap_object *map, const zend_ **************************/ const php_dom_obj_map_handler php_dom_obj_map_attributes = { + .get_named_item = dom_map_get_named_item_prop, + .has_named_item = dom_map_has_named_item_prop, .length = dom_map_get_prop_length, .get_item = dom_map_get_attributes_item, .get_ns_named_item = dom_map_get_ns_named_item_prop, @@ -556,6 +628,8 @@ const php_dom_obj_map_handler php_dom_obj_map_attributes = { }; const php_dom_obj_map_handler php_dom_obj_map_by_tag_name = { + .get_named_item = dom_map_get_ns_named_item_null, + .has_named_item = dom_map_has_named_item_null, .length = dom_map_get_by_tag_name_length, .get_item = dom_map_get_by_tag_name_item, .get_ns_named_item = dom_map_get_ns_named_item_null, @@ -566,6 +640,8 @@ const php_dom_obj_map_handler php_dom_obj_map_by_tag_name = { }; const php_dom_obj_map_handler php_dom_obj_map_by_class_name = { + .get_named_item = dom_map_get_ns_named_item_null, + .has_named_item = dom_map_has_named_item_null, .length = dom_map_get_by_class_name_length, .get_item = dom_map_get_by_class_name_item, .get_ns_named_item = dom_map_get_ns_named_item_null, @@ -576,6 +652,8 @@ const php_dom_obj_map_handler php_dom_obj_map_by_class_name = { }; const php_dom_obj_map_handler php_dom_obj_map_child_nodes = { + .get_named_item = dom_map_get_ns_named_item_null, + .has_named_item = dom_map_has_named_item_null, .length = dom_map_get_nodes_length, .get_item = dom_map_get_nodes_item, .get_ns_named_item = dom_map_get_ns_named_item_null, @@ -586,6 +664,8 @@ const php_dom_obj_map_handler php_dom_obj_map_child_nodes = { }; const php_dom_obj_map_handler php_dom_obj_map_nodeset = { + .get_named_item = dom_map_get_ns_named_item_null, + .has_named_item = dom_map_has_named_item_null, .length = dom_map_get_nodeset_length, .get_item = dom_map_get_nodeset_item, .get_ns_named_item = dom_map_get_ns_named_item_null, @@ -596,6 +676,8 @@ const php_dom_obj_map_handler php_dom_obj_map_nodeset = { }; const php_dom_obj_map_handler php_dom_obj_map_entities = { + .get_named_item = dom_map_get_named_item_entity_fn, + .has_named_item = dom_map_has_named_item_entity_fn, .length = dom_map_get_xmlht_length, .get_item = dom_map_get_entity_item, .get_ns_named_item = dom_map_get_ns_named_item_entity, @@ -606,6 +688,8 @@ const php_dom_obj_map_handler php_dom_obj_map_entities = { }; const php_dom_obj_map_handler php_dom_obj_map_notations = { + .get_named_item = dom_map_get_named_item_notation_fn, + .has_named_item = dom_map_has_named_item_entity_fn, .length = dom_map_get_xmlht_length, .get_item = dom_map_get_notation_item, .get_ns_named_item = dom_map_get_ns_named_item_notation, @@ -616,6 +700,8 @@ const php_dom_obj_map_handler php_dom_obj_map_notations = { }; const php_dom_obj_map_handler php_dom_obj_map_child_elements = { + .get_named_item = dom_map_get_ns_named_item_null, + .has_named_item = dom_map_has_named_item_null, .length = dom_map_get_elements_length, .get_item = dom_map_get_elements_item, .get_ns_named_item = dom_map_get_ns_named_item_null, @@ -626,6 +712,8 @@ const php_dom_obj_map_handler php_dom_obj_map_child_elements = { }; const php_dom_obj_map_handler php_dom_obj_map_noop = { + .get_named_item = dom_map_get_ns_named_item_null, + .has_named_item = dom_map_has_named_item_null, .length = dom_map_get_zero_length, .get_item = dom_map_get_null_item, .get_ns_named_item = dom_map_get_ns_named_item_null, diff --git a/ext/dom/obj_map.h b/ext/dom/obj_map.h index beed08bbb164..8990bd613bbd 100644 --- a/ext/dom/obj_map.h +++ b/ext/dom/obj_map.h @@ -27,6 +27,8 @@ typedef struct php_dom_obj_map_collection_iter { typedef struct php_dom_obj_map_handler { zend_long (*length)(dom_nnodemap_object *); void (*get_item)(dom_nnodemap_object *, zend_long, zval *); + xmlNodePtr (*get_named_item)(dom_nnodemap_object *, const zend_string *); + bool (*has_named_item)(dom_nnodemap_object *, const zend_string *); xmlNodePtr (*get_ns_named_item)(dom_nnodemap_object *, const zend_string *, const char *); bool (*has_ns_named_item)(dom_nnodemap_object *, const zend_string *, const char *); void (*collection_named_item_iter)(dom_nnodemap_object *, php_dom_obj_map_collection_iter *); @@ -58,6 +60,8 @@ typedef struct dom_nnodemap_object { void php_dom_create_obj_map(dom_object *basenode, dom_object *intern, xmlHashTablePtr ht, zend_string *local, zend_string *ns, const php_dom_obj_map_handler *handler); void php_dom_obj_map_get_ns_named_item_into_zval(dom_nnodemap_object *objmap, const zend_string *named, const char *ns, zval *return_value); +void php_dom_obj_map_get_named_item_into_zval(dom_nnodemap_object *objmap, const zend_string *named, zval *return_value); +bool php_dom_obj_map_has_named_item(dom_nnodemap_object *objmap, const zend_string *named); void php_dom_obj_map_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value); zend_long php_dom_get_nodelist_length(dom_object *obj); diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index f034976839a7..41f4fe7c71a3 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -2403,7 +2403,7 @@ static zval *dom_nodemap_read_dimension(zend_object *object, zval *offset, int t zend_long lval; if (dom_nodemap_or_nodelist_process_offset_as_named(offset, &lval)) { /* exceptional case, switch to named lookup */ - php_dom_obj_map_get_ns_named_item_into_zval(php_dom_obj_from_obj(object)->ptr, Z_STR_P(offset), NULL, rv); + php_dom_obj_map_get_named_item_into_zval(php_dom_obj_from_obj(object)->ptr, Z_STR_P(offset), rv); return rv; } @@ -2429,7 +2429,7 @@ static int dom_nodemap_has_dimension(zend_object *object, zval *member, int chec if (dom_nodemap_or_nodelist_process_offset_as_named(member, &offset)) { /* exceptional case, switch to named lookup */ dom_nnodemap_object *map = php_dom_obj_from_obj(object)->ptr; - return map->handler->has_ns_named_item(map, Z_STR_P(member), NULL); + return php_dom_obj_map_has_named_item(map, Z_STR_P(member)); } return offset >= 0 && offset < php_dom_get_namednodemap_length(php_dom_obj_from_obj(object)); @@ -2450,7 +2450,7 @@ static zval *dom_modern_nodemap_read_dimension(zend_object *object, zval *offset if (ZEND_HANDLE_NUMERIC(Z_STR_P(offset), lval)) { map->handler->get_item(map, (zend_long) lval, rv); } else { - php_dom_obj_map_get_ns_named_item_into_zval(map, Z_STR_P(offset), NULL, rv); + php_dom_obj_map_get_named_item_into_zval(map, Z_STR_P(offset), rv); } } else if (Z_TYPE_P(offset) == IS_LONG) { map->handler->get_item(map, Z_LVAL_P(offset), rv); @@ -2478,7 +2478,7 @@ static int dom_modern_nodemap_has_dimension(zend_object *object, zval *member, i if (ZEND_HANDLE_NUMERIC(Z_STR_P(member), lval)) { return (zend_long) lval >= 0 && (zend_long) lval < php_dom_get_namednodemap_length(obj); } else { - return map->handler->has_ns_named_item(map, Z_STR_P(member), NULL); + return php_dom_obj_map_has_named_item(map, Z_STR_P(member)); } } else if (Z_TYPE_P(member) == IS_LONG) { zend_long offset = Z_LVAL_P(member); diff --git a/ext/dom/tests/modern/xml/getnamednodemap_null_ns_lookup.phpt b/ext/dom/tests/modern/xml/getnamednodemap_null_ns_lookup.phpt new file mode 100644 index 000000000000..e6b2039fbe23 --- /dev/null +++ b/ext/dom/tests/modern/xml/getnamednodemap_null_ns_lookup.phpt @@ -0,0 +1,41 @@ +--TEST-- +DOMNamedNodeMap::getNamedItemNS() with null namespace must not match namespaced attributes +--EXTENSIONS-- +dom +--FILE-- +'); +$root = $dom->documentElement; +$root->setAttributeNS('urn:x', 'bar', 'ns'); +var_dump($root->attributes->getNamedItemNS(null, 'bar')); +var_dump($root->attributes->getNamedItemNS('urn:x', 'bar')->value); +var_dump(isset($root->attributes['xmlns:foo'])); + +$other = $dom->createElement('other'); +$attrNs = $dom->createAttributeNS('urn:y', 'x:baz'); +$attrNs->value = 'nsval'; +$other->setAttributeNode($attrNs); +$attrPlain = $dom->createAttribute('baz'); +$attrPlain->value = 'plain'; +$other->setAttributeNode($attrPlain); +$attrs = $other->attributes; +var_dump($attrs->getNamedItemNS(null, 'baz')->value); +var_dump($attrs->getNamedItemNS('urn:y', 'baz')->value); +var_dump($attrs->getNamedItem('baz')->value); +var_dump($attrs['x:baz']->value); + +$html = Dom\HTMLDocument::createFromString('

'); +$p = $html->getElementsByTagName('p')->item(0); +var_dump($p->attributes->getNamedItemNS(null, 'ALIGN')->value); + +?> +--EXPECT-- +NULL +string(2) "ns" +bool(true) +string(5) "plain" +string(5) "nsval" +string(5) "plain" +string(5) "nsval" +string(6) "center"