Skip to content

Commit f420ea8

Browse files
committed
Merge branch 'PHP-8.3'
* PHP-8.3: Fix crashes with entity references and predefined entities Fix crash in adoptNode with attribute references
2 parents 20dfc41 + 3fa5af8 commit f420ea8

File tree

5 files changed

+127
-19
lines changed

5 files changed

+127
-19
lines changed

ext/dom/document.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,21 +1030,33 @@ PHP_METHOD(DOM_Document, getElementById)
10301030
}
10311031
/* }}} end dom_document_get_element_by_id */
10321032

1033-
static void php_dom_transfer_document_ref(xmlNodePtr node, dom_object *dom_object_document, xmlDocPtr document)
1033+
static zend_always_inline void php_dom_transfer_document_ref_single_node(xmlNodePtr node, php_libxml_ref_obj *new_document)
1034+
{
1035+
php_libxml_node_ptr *iteration_object_ptr = node->_private;
1036+
if (iteration_object_ptr) {
1037+
php_libxml_node_object *iteration_object = iteration_object_ptr->_private;
1038+
ZEND_ASSERT(iteration_object != NULL);
1039+
/* Must increase refcount first because we could be the last reference holder, and the document may be equal. */
1040+
new_document->refcount++;
1041+
php_libxml_decrement_doc_ref(iteration_object);
1042+
iteration_object->document = new_document;
1043+
}
1044+
}
1045+
1046+
static void php_dom_transfer_document_ref(xmlNodePtr node, php_libxml_ref_obj *new_document)
10341047
{
10351048
if (node->children) {
1036-
php_dom_transfer_document_ref(node->children, dom_object_document, document);
1049+
php_dom_transfer_document_ref(node->children, new_document);
10371050
}
1051+
10381052
while (node) {
1039-
php_libxml_node_ptr *iteration_object_ptr = node->_private;
1040-
if (iteration_object_ptr) {
1041-
php_libxml_node_object *iteration_object = iteration_object_ptr->_private;
1042-
ZEND_ASSERT(iteration_object != NULL);
1043-
/* Must increase refcount first because we could be the last reference holder, and the document may be equal. */
1044-
dom_object_document->document->refcount++;
1045-
php_libxml_decrement_doc_ref(iteration_object);
1046-
iteration_object->document = dom_object_document->document;
1053+
if (node->type == XML_ELEMENT_NODE) {
1054+
for (xmlAttrPtr attr = node->properties; attr != NULL; attr = attr->next) {
1055+
php_dom_transfer_document_ref_single_node((xmlNodePtr) attr, new_document);
1056+
}
10471057
}
1058+
1059+
php_dom_transfer_document_ref_single_node(node, new_document);
10481060
node = node->next;
10491061
}
10501062
}
@@ -1062,7 +1074,7 @@ bool php_dom_adopt_node(xmlNodePtr nodep, dom_object *dom_object_new_document, x
10621074
return false;
10631075
}
10641076

1065-
php_dom_transfer_document_ref(nodep, dom_object_new_document, new_document);
1077+
php_dom_transfer_document_ref(nodep, dom_object_new_document->document);
10661078
} else {
10671079
xmlUnlinkNode(nodep);
10681080
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
DOMDocument::adoptNode() with attribute references
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = new DOMDocument;
9+
$root = $dom->appendChild($dom->createElement('root'));
10+
$root->setAttributeNS("urn:a", "a:root1", "bar");
11+
$root1 = $root->getAttributeNodeNS("urn:a", "root1");
12+
echo $dom->saveXML();
13+
14+
$dom = new DOMDocument;
15+
$dom->appendChild($dom->adoptNode($root));
16+
foreach ($dom->documentElement->attributes as $attr) {
17+
var_dump($attr->namespaceURI, $attr->prefix, $attr->localName, $attr->nodeValue);
18+
}
19+
20+
?>
21+
--EXPECT--
22+
<?xml version="1.0"?>
23+
<root xmlns:a="urn:a" a:root1="bar"/>
24+
string(5) "urn:a"
25+
string(1) "a"
26+
string(5) "root1"
27+
string(3) "bar"
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
Freeing of a predefined DOMEntityReference
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$ref = new DOMEntityReference("amp");
8+
var_dump($ref);
9+
?>
10+
--EXPECT--
11+
object(DOMEntityReference)#1 (17) {
12+
["nodeName"]=>
13+
string(3) "amp"
14+
["nodeValue"]=>
15+
NULL
16+
["nodeType"]=>
17+
int(5)
18+
["parentNode"]=>
19+
NULL
20+
["parentElement"]=>
21+
NULL
22+
["childNodes"]=>
23+
string(22) "(object value omitted)"
24+
["firstChild"]=>
25+
string(22) "(object value omitted)"
26+
["lastChild"]=>
27+
string(22) "(object value omitted)"
28+
["previousSibling"]=>
29+
NULL
30+
["nextSibling"]=>
31+
NULL
32+
["attributes"]=>
33+
NULL
34+
["isConnected"]=>
35+
bool(false)
36+
["namespaceURI"]=>
37+
NULL
38+
["prefix"]=>
39+
string(0) ""
40+
["localName"]=>
41+
NULL
42+
["baseURI"]=>
43+
NULL
44+
["textContent"]=>
45+
string(0) ""
46+
}

ext/dom/tests/delayed_freeing/entity_declaration.phpt

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,32 @@ $doc->loadXML(<<<'XML'
99
<?xml version="1.0"?>
1010
<!DOCTYPE books [
1111
<!ENTITY test "entity is only for test purposes">
12+
<!ENTITY myimage PUBLIC "-" "mypicture.gif" NDATA GIF>
1213
]>
1314
<container/>
1415
XML);
15-
$entity = $doc->doctype->entities[0];
16-
var_dump($entity->nodeName, $entity->parentNode->nodeName);
16+
$ref1 = $doc->createEntityReference("test");
17+
$ref2 = $doc->createEntityReference("myimage");
18+
$entity1 = $doc->doctype->entities[0];
19+
$entity2 = $doc->doctype->entities[1];
20+
21+
// Entity order depends on addresses
22+
if ($entity1->nodeName !== "test") {
23+
[$entity1, $entity2] = [$entity2, $entity1];
24+
}
25+
26+
var_dump($entity1->nodeName, $entity1->parentNode->nodeName);
27+
var_dump($entity2->nodeName, $entity2->parentNode->nodeName);
1728
$doc->removeChild($doc->doctype);
18-
var_dump($entity->nodeName, $entity->parentNode);
29+
var_dump($entity1->nodeName, $entity1->parentNode);
30+
var_dump($entity2->nodeName, $entity2->parentNode);
1931
?>
2032
--EXPECT--
2133
string(4) "test"
2234
string(5) "books"
35+
string(7) "myimage"
36+
string(5) "books"
2337
string(4) "test"
2438
NULL
39+
string(7) "myimage"
40+
NULL

ext/libxml/libxml.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -207,12 +207,10 @@ static void php_libxml_node_free(xmlNodePtr node)
207207
* dtd is attached to the document. This works around the issue by inspecting the parent directly. */
208208
case XML_ENTITY_DECL: {
209209
xmlEntityPtr entity = (xmlEntityPtr) node;
210-
php_libxml_unlink_entity_decl(entity);
211-
if (entity->orig != NULL) {
212-
xmlFree((char *) entity->orig);
213-
entity->orig = NULL;
210+
if (entity->etype != XML_INTERNAL_PREDEFINED_ENTITY) {
211+
php_libxml_unlink_entity_decl(entity);
212+
xmlFreeEntity(entity);
214213
}
215-
xmlFreeNode(node);
216214
break;
217215
}
218216
case XML_NOTATION_NODE: {
@@ -1383,6 +1381,15 @@ PHP_LIBXML_API void php_libxml_node_free_resource(xmlNodePtr node)
13831381
case XML_DOCUMENT_NODE:
13841382
case XML_HTML_DOCUMENT_NODE:
13851383
break;
1384+
case XML_ENTITY_REF_NODE:
1385+
/* Entity reference nodes are special: their children point to entity declarations,
1386+
* but they don't own the declarations and therefore shouldn't free the children.
1387+
* Moreover, there can be N>1 reference nodes for a single entity declarations. */
1388+
php_libxml_unregister_node(node);
1389+
if (node->parent == NULL) {
1390+
php_libxml_node_free(node);
1391+
}
1392+
break;
13861393
default:
13871394
if (node->parent == NULL || node->type == XML_NAMESPACE_DECL) {
13881395
php_libxml_node_free_list((xmlNodePtr) node->children);

0 commit comments

Comments
 (0)