Commit 9498bc3ee13 for php.net
commit 9498bc3ee131616344370f59d4f1a6bb46375750
Author: Jordi Kroon <jkroon@onyourmarks.agency>
Date: Fri Apr 24 20:37:54 2026 +0200
fix Dom\Notation nodes missing tree connection
Fixes an existing TODO by @ndossche.
Notation nodes returned from $doctype->notations were not linked to their owning document or parent DocumentType. This caused several incorrect behaviour; including:
ownerDocument was missing
parentNode was NULL
isConnected returned false
baseURI fell back to "about:blank"
textContent returned an empty string instead of NULL
The last point is a violation of the DOM specification. Since Notation is not an Element, CharacterData, Attr, or DocumentFragment, its textContent must return NULL.
Spec reference: https://dom.spec.whatwg.org/#dom-node-textcontent
close GH-21868
diff --git a/NEWS b/NEWS
index a5393d50bfc..5be5ffd18ac 100644
--- a/NEWS
+++ b/NEWS
@@ -33,6 +33,10 @@ PHP NEWS
correctly, and external writes raise "Cannot modify private(set)
property" instead of the previous readonly modification error.
(David Carlier)
+ . Fixed Dom\Notation nodes missing tree connection, so that
+ ownerDocument, parentNode, isConnected and baseURI now return correct
+ values, and textContent returns NULL per the DOM specification.
+ (jordikroon)
- Fileinfo:
. Fixed bug GH-20679 (finfo_file() doesn't work on remote resources).
diff --git a/ext/dom/dom_iterators.c b/ext/dom/dom_iterators.c
index b71d188dcee..f97a9ec825d 100644
--- a/ext/dom/dom_iterators.c
+++ b/ext/dom/dom_iterators.c
@@ -54,7 +54,7 @@ static dom_nnodemap_object *php_dom_iterator_get_nnmap(const php_dom_iterator *i
return nnmap->ptr;
}
-xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID) /* {{{ */
+xmlNodePtr create_notation(xmlDtdPtr parent_dtd, const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID) /* {{{ */
{
xmlEntityPtr ret = xmlMalloc(sizeof(xmlEntity));
memset(ret, 0, sizeof(xmlEntity));
@@ -62,10 +62,23 @@ xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const
ret->name = xmlStrdup(name);
ret->ExternalID = xmlStrdup(ExternalID);
ret->SystemID = xmlStrdup(SystemID);
+ if (parent_dtd != NULL) {
+ ret->parent = parent_dtd;
+ ret->doc = parent_dtd->doc;
+ }
return (xmlNodePtr) ret;
}
/* }}} */
+void dom_free_notation(xmlEntityPtr entity) /* {{{ */
+{
+ xmlFree((xmlChar *) entity->name);
+ xmlFree((xmlChar *) entity->ExternalID);
+ xmlFree((xmlChar *) entity->SystemID);
+ xmlFree(entity);
+}
+/* }}} */
+
xmlNodePtr php_dom_libxml_hash_iter(xmlHashTable *ht, int index)
{
int htsize;
diff --git a/ext/dom/obj_map.c b/ext/dom/obj_map.c
index 60f8b28e6ca..eeef8345fc6 100644
--- a/ext/dom/obj_map.c
+++ b/ext/dom/obj_map.c
@@ -176,7 +176,8 @@ static void dom_map_get_notation_item(dom_nnodemap_object *map, zend_long index,
xmlNodePtr node = map->ht ? php_dom_libxml_hash_iter(map->ht, index) : NULL;
if (node) {
xmlNotation *notation = (xmlNotation *) node;
- node = create_notation(notation->name, notation->PublicID, notation->SystemID);
+ xmlDtdPtr dtd = (xmlDtdPtr) dom_object_get_node(map->baseobj);
+ node = create_notation(dtd, notation->name, notation->PublicID, notation->SystemID);
}
dom_ret_node_to_zobj(map, node, return_value);
}
@@ -504,7 +505,8 @@ static xmlNodePtr dom_map_get_ns_named_item_notation(dom_nnodemap_object *map, c
{
xmlNotationPtr notation = xmlHashLookup(map->ht, BAD_CAST ZSTR_VAL(named));
if (notation) {
- return create_notation(notation->name, notation->PublicID, notation->SystemID);
+ xmlDtdPtr dtd = (xmlDtdPtr) dom_object_get_node(map->baseobj);
+ return create_notation(dtd, notation->name, notation->PublicID, notation->SystemID);
}
return NULL;
}
diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c
index b188dea6eb4..ec1a2ea1c6c 100644
--- a/ext/dom/php_dom.c
+++ b/ext/dom/php_dom.c
@@ -1486,7 +1486,13 @@ void dom_objects_free_storage(zend_object *object)
if (ptr != NULL && ptr->node != NULL) {
xmlNodePtr node = ptr->node;
- if (node->type != XML_DOCUMENT_NODE && node->type != XML_HTML_DOCUMENT_NODE) {
+ if (node->type == XML_NOTATION_NODE) {
+ unsigned int refcount = php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
+ php_libxml_decrement_doc_ref((php_libxml_node_object *) intern);
+ if (refcount == 0) {
+ dom_free_notation((xmlEntityPtr) node);
+ }
+ } else if (node->type != XML_DOCUMENT_NODE && node->type != XML_HTML_DOCUMENT_NODE) {
php_libxml_node_decrement_resource((php_libxml_node_object *) intern);
} else {
php_libxml_decrement_node_ptr((php_libxml_node_object *) intern);
diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h
index bc414adaa2f..d424b26cc69 100644
--- a/ext/dom/php_dom.h
+++ b/ext/dom/php_dom.h
@@ -125,7 +125,8 @@ int dom_hierarchy(xmlNodePtr parent, xmlNodePtr child);
bool dom_has_feature(zend_string *feature, zend_string *version);
bool dom_node_is_read_only(const xmlNode *node);
bool dom_node_children_valid(const xmlNode *node);
-xmlNodePtr create_notation(const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID);
+xmlNodePtr create_notation(xmlDtdPtr parent_dtd, const xmlChar *name, const xmlChar *ExternalID, const xmlChar *SystemID);
+void dom_free_notation(xmlEntityPtr entity);
xmlNode *php_dom_libxml_hash_iter(xmlHashTable *ht, int index);
zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref);
void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce);
diff --git a/ext/dom/tests/modern/xml/DTDNamedNodeMap.phpt b/ext/dom/tests/modern/xml/DTDNamedNodeMap.phpt
index f9bb1f7a996..4ac15a029e3 100644
--- a/ext/dom/tests/modern/xml/DTDNamedNodeMap.phpt
+++ b/ext/dom/tests/modern/xml/DTDNamedNodeMap.phpt
@@ -21,7 +21,6 @@
var_dump($doctype->entities["test"]);
var_dump($doctype->entities["myimage"]);
-// TODO: isConnected returning false is a bug
var_dump($doctype->notations["GIF"]);
?>
@@ -142,17 +141,19 @@
["textContent"]=>
NULL
}
-object(Dom\Notation)#4 (13) {
+object(Dom\Notation)#4 (14) {
["nodeType"]=>
int(12)
["nodeName"]=>
string(3) "GIF"
["baseURI"]=>
- string(11) "about:blank"
+ string(%d) "%s"
["isConnected"]=>
- bool(false)
+ bool(true)
+ ["ownerDocument"]=>
+ string(22) "(object value omitted)"
["parentNode"]=>
- NULL
+ string(22) "(object value omitted)"
["parentElement"]=>
NULL
["childNodes"]=>
@@ -168,5 +169,5 @@
["nodeValue"]=>
NULL
["textContent"]=>
- string(0) ""
+ NULL
}
diff --git a/ext/dom/tests/modern/xml/XMLDocument_node_notation_wiring.phpt b/ext/dom/tests/modern/xml/XMLDocument_node_notation_wiring.phpt
new file mode 100644
index 00000000000..dd74a273759
--- /dev/null
+++ b/ext/dom/tests/modern/xml/XMLDocument_node_notation_wiring.phpt
@@ -0,0 +1,101 @@
+--TEST--
+Dom\XMLDocument: Dom\Notation nodes are connected to their document and doctype
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$cases = [
+ 'GIF' => '<!NOTATION GIF SYSTEM "image/gif">',
+ 'JPEG' => '<!NOTATION JPEG PUBLIC "-//W3C//NOTATION JPEG//EN" "image/jpeg">',
+ 'HTML' => '<!NOTATION HTML PUBLIC "-//W3C//NOTATION HTML//EN">',
+];
+
+foreach ($cases as $name => $declaration) {
+ $xml = <<<XML
+<!DOCTYPE root [
+ $declaration
+]>
+<root/>
+XML;
+
+ $dom = Dom\XMLDocument::createFromString($xml);
+ $doctype = $dom->doctype;
+ $notations = $doctype->notations;
+
+ echo "=== $name ===\n";
+
+ $namedNotation = $notations->getNamedItem($name);
+ foreach ($notations as $iteratedNotation) {
+ // getNamedItem
+ var_dump($namedNotation->nodeName);
+ var_dump($namedNotation->textContent);
+ var_dump($namedNotation->nodeValue);
+ var_dump($namedNotation->isConnected);
+ var_dump($namedNotation->ownerDocument === $dom);
+ var_dump($namedNotation->parentNode === $doctype);
+ var_dump($namedNotation->parentElement);
+
+ // iteration
+ var_dump($iteratedNotation->nodeName);
+ var_dump($iteratedNotation->textContent);
+ var_dump($iteratedNotation->nodeValue);
+ var_dump($iteratedNotation->isConnected);
+ var_dump($iteratedNotation->ownerDocument === $dom);
+ var_dump($iteratedNotation->parentNode === $doctype);
+ var_dump($iteratedNotation->parentElement);
+
+ // wiring
+ // getNamedItem and iteration each allocate a fresh Notation instance
+ var_dump($namedNotation !== $iteratedNotation);
+ }
+}
+?>
+--EXPECT--
+=== GIF ===
+string(3) "GIF"
+NULL
+NULL
+bool(true)
+bool(true)
+bool(true)
+NULL
+string(3) "GIF"
+NULL
+NULL
+bool(true)
+bool(true)
+bool(true)
+NULL
+bool(true)
+=== JPEG ===
+string(4) "JPEG"
+NULL
+NULL
+bool(true)
+bool(true)
+bool(true)
+NULL
+string(4) "JPEG"
+NULL
+NULL
+bool(true)
+bool(true)
+bool(true)
+NULL
+bool(true)
+=== HTML ===
+string(4) "HTML"
+NULL
+NULL
+bool(true)
+bool(true)
+bool(true)
+NULL
+string(4) "HTML"
+NULL
+NULL
+bool(true)
+bool(true)
+bool(true)
+NULL
+bool(true)