From d5347fbc350464cbbb42c850f935fb8b3672fe61 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Sun, 30 Aug 2026 19:13:12 +0100 Subject: [PATCH] ext/dom: getNamedItemNS() matching on the qualified name. The named item handlers took a single NULL ns to mean both "no namespace argument" and "the null namespace", so getNamedItemNS() fell back to the qualified name lookup used by getNamedItem(), and both lookups handed back DTD attribute declarations that DOM_RET_OBJ() cannot wrap. The two lookups are now told apart by a use_ns flag and declarations are rejected. --- ext/dom/namednodemap.c | 4 +- ext/dom/obj_map.c | 43 +++++++------- ext/dom/obj_map.h | 6 +- ext/dom/php_dom.c | 8 +-- ...NodeMap_getNamedItemNS_qualified_name.phpt | 57 +++++++++++++++++++ ...NamedNodeMap_getNamedItem_dtd_default.phpt | 43 ++++++++++++++ 6 files changed, 130 insertions(+), 31 deletions(-) create mode 100644 ext/dom/tests/modern/spec/NamedNodeMap_getNamedItemNS_qualified_name.phpt create mode 100644 ext/dom/tests/modern/spec/NamedNodeMap_getNamedItem_dtd_default.phpt diff --git a/ext/dom/namednodemap.c b/ext/dom/namednodemap.c index 5e0818e4a960..38e4c53e91ad 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_ns_named_item_into_zval(objmap, named, NULL, false, return_value); } /* }}} end dom_namednodemap_get_named_item */ @@ -111,7 +111,7 @@ PHP_METHOD(DOMNamedNodeMap, getNamedItemNS) if (urilen == 0 && objmap->baseobj != NULL && php_dom_follow_spec_intern(objmap->baseobj)) { uri = NULL; } - php_dom_obj_map_get_ns_named_item_into_zval(objmap, named, uri, return_value); + php_dom_obj_map_get_ns_named_item_into_zval(objmap, named, uri, true, return_value); } } /* }}} end dom_namednodemap_get_named_item_ns */ diff --git a/ext/dom/obj_map.c b/ext/dom/obj_map.c index 88d7075be531..e25ea71de2b0 100644 --- a/ext/dom/obj_map.c +++ b/ext/dom/obj_map.c @@ -476,9 +476,9 @@ void php_dom_obj_map_get_item_into_zval(dom_nnodemap_object *objmap, zend_long i } } -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_ns_named_item_into_zval(dom_nnodemap_object *objmap, const zend_string *named, const char *ns, bool use_ns, zval *return_value) { - xmlNodePtr itemnode = objmap->handler->get_ns_named_item(objmap, named, ns); + xmlNodePtr itemnode = objmap->handler->get_ns_named_item(objmap, named, ns, use_ns); if (itemnode) { DOM_RET_OBJ(itemnode, objmap->baseobj); } else { @@ -490,17 +490,17 @@ void php_dom_obj_map_get_ns_named_item_into_zval(dom_nnodemap_object *objmap, co * === Named item === * **********************/ -static xmlNodePtr dom_map_get_ns_named_item_entity(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static xmlNodePtr dom_map_get_ns_named_item_entity(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { return xmlHashLookup(map->ht, BAD_CAST ZSTR_VAL(named)); } -static bool dom_map_has_ns_named_item_xmlht(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static bool dom_map_has_ns_named_item_xmlht(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { - return dom_map_get_ns_named_item_entity(map, named, ns) != NULL; + return dom_map_get_ns_named_item_entity(map, named, ns, use_ns) != NULL; } -static xmlNodePtr dom_map_get_ns_named_item_notation(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static xmlNodePtr dom_map_get_ns_named_item_notation(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { xmlNotationPtr notation = xmlHashLookup(map->ht, BAD_CAST ZSTR_VAL(named)); if (notation) { @@ -509,38 +509,37 @@ 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_ns_named_item_prop(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { xmlNodePtr nodep = dom_object_get_node(map->baseobj); if (nodep) { - if (ns) { - xmlNodePtr itemnode = (xmlNodePtr) xmlHasNsProp(nodep, BAD_CAST ZSTR_VAL(named), BAD_CAST ns); - if (itemnode != NULL && itemnode->type == XML_ATTRIBUTE_DECL) { - return NULL; - } - return itemnode; + xmlNodePtr itemnode; + if (use_ns) { + itemnode = (xmlNodePtr) xmlHasNsProp(nodep, BAD_CAST ZSTR_VAL(named), BAD_CAST ns); + } else if (php_dom_follow_spec_intern(map->baseobj)) { + itemnode = (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)); - } + itemnode = (xmlNodePtr) xmlHasProp(nodep, BAD_CAST ZSTR_VAL(named)); + } + if (itemnode != NULL && itemnode->type == XML_ATTRIBUTE_DECL) { + return NULL; } + return itemnode; } return NULL; } -static bool dom_map_has_ns_named_item_prop(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static bool dom_map_has_ns_named_item_prop(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { - return dom_map_get_ns_named_item_prop(map, named, ns) != NULL; + return dom_map_get_ns_named_item_prop(map, named, ns, use_ns) != NULL; } -static xmlNodePtr dom_map_get_ns_named_item_null(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static xmlNodePtr dom_map_get_ns_named_item_null(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { return NULL; } -static bool dom_map_has_ns_named_item_null(dom_nnodemap_object *map, const zend_string *named, const char *ns) +static bool dom_map_has_ns_named_item_null(dom_nnodemap_object *map, const zend_string *named, const char *ns, bool use_ns) { return false; } diff --git a/ext/dom/obj_map.h b/ext/dom/obj_map.h index beed08bbb164..fe430e260b1d 100644 --- a/ext/dom/obj_map.h +++ b/ext/dom/obj_map.h @@ -27,8 +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_ns_named_item)(dom_nnodemap_object *, const zend_string *, const char *); - bool (*has_ns_named_item)(dom_nnodemap_object *, const zend_string *, const char *); + xmlNodePtr (*get_ns_named_item)(dom_nnodemap_object *, const zend_string *, const char *, bool); + bool (*has_ns_named_item)(dom_nnodemap_object *, const zend_string *, const char *, bool); void (*collection_named_item_iter)(dom_nnodemap_object *, php_dom_obj_map_collection_iter *); bool use_cache; bool nameless; @@ -57,7 +57,7 @@ typedef struct dom_nnodemap_object { } 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_ns_named_item_into_zval(dom_nnodemap_object *objmap, const zend_string *named, const char *ns, bool use_ns, zval *return_value); 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..b403db360dc0 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_ns_named_item_into_zval(php_dom_obj_from_obj(object)->ptr, Z_STR_P(offset), NULL, false, 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 map->handler->has_ns_named_item(map, Z_STR_P(member), NULL, false); } 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_ns_named_item_into_zval(map, Z_STR_P(offset), NULL, false, 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 map->handler->has_ns_named_item(map, Z_STR_P(member), NULL, false); } } else if (Z_TYPE_P(member) == IS_LONG) { zend_long offset = Z_LVAL_P(member); diff --git a/ext/dom/tests/modern/spec/NamedNodeMap_getNamedItemNS_qualified_name.phpt b/ext/dom/tests/modern/spec/NamedNodeMap_getNamedItemNS_qualified_name.phpt new file mode 100644 index 000000000000..1297c6b51e51 --- /dev/null +++ b/ext/dom/tests/modern/spec/NamedNodeMap_getNamedItemNS_qualified_name.phpt @@ -0,0 +1,57 @@ +--TEST-- +getNamedItemNS() must match on the local name in a namespace, not on the qualified name +--EXTENSIONS-- +dom +--FILE-- +'; +$dtd = << + + +]> + +XML; + +$legacy = new DOMDocument(); +$legacy->loadXML($src); +$legacyDtd = new DOMDocument(); +$legacyDtd->loadXML($dtd); + +foreach (['legacy' => $legacy, 'spec' => Dom\XMLDocument::createFromString($src)] as $label => $doc) { + echo $label, PHP_EOL; + $map = $doc->documentElement->attributes; + var_dump($map->getNamedItemNS(null, 'bar')?->nodeValue); + var_dump($map->getNamedItemNS('', 'bar')?->nodeValue); + var_dump($map->getNamedItemNS(null, 'q:bar')?->nodeValue); + var_dump($map->getNamedItemNS('', 'q:bar')?->nodeValue); + var_dump($map->getNamedItemNS(null, 'only')?->nodeValue); + var_dump($map->getNamedItemNS('urn:q', 'only')?->nodeValue); + var_dump($map->getNamedItem('q:bar')?->nodeValue); +} + +echo 'dtd default', PHP_EOL; +var_dump($legacyDtd->documentElement->attributes->getNamedItemNS(null, 'defaulted')?->nodeValue); +var_dump(Dom\XMLDocument::createFromString($dtd)->documentElement->attributes->getNamedItemNS(null, 'defaulted')?->nodeValue); +?> +--EXPECT-- +legacy +string(5) "no-ns" +NULL +NULL +NULL +NULL +string(8) "prefixed" +NULL +spec +string(5) "no-ns" +string(5) "no-ns" +NULL +NULL +NULL +string(8) "prefixed" +string(2) "ns" +dtd default +NULL +NULL diff --git a/ext/dom/tests/modern/spec/NamedNodeMap_getNamedItem_dtd_default.phpt b/ext/dom/tests/modern/spec/NamedNodeMap_getNamedItem_dtd_default.phpt new file mode 100644 index 000000000000..f2dae8538fa2 --- /dev/null +++ b/ext/dom/tests/modern/spec/NamedNodeMap_getNamedItem_dtd_default.phpt @@ -0,0 +1,43 @@ +--TEST-- +getNamedItem() must not expose DTD attribute declarations +--EXTENSIONS-- +dom +--FILE-- + + + +]> + +XML; + +$legacy = new DOMDocument(); +$legacy->loadXML($xml); + +foreach (['legacy' => $legacy, 'spec' => Dom\XMLDocument::createFromString($xml)] as $label => $doc) { + echo $label, PHP_EOL; + $map = $doc->documentElement->attributes; + foreach (['defaulted', 'real'] as $name) { + var_dump($map->getNamedItem($name)?->nodeValue); + var_dump(isset($map[$name])); + var_dump($map[$name]?->nodeValue); + } +} +?> +--EXPECT-- +legacy +NULL +bool(false) +NULL +string(7) "present" +bool(true) +string(7) "present" +spec +NULL +bool(false) +NULL +string(7) "present" +bool(true) +string(7) "present"