From 5f69232b5374d9dbd4d8da65a493eaa81e7c5a06 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 23 Dec 2023 17:31:18 +0100 Subject: [PATCH] Revert "Fix crashes with entity references and predefined entities" This reverts commit 3fa5af8496bdbc74e24828b790e4acfd6a26f0d4. --- NEWS | 1 - .../DOMEntityReference_predefined_free.phpt | 46 ------------------- .../delayed_freeing/entity_declaration.phpt | 22 ++------- ext/libxml/libxml.c | 17 ++----- 4 files changed, 8 insertions(+), 78 deletions(-) delete mode 100644 ext/dom/tests/DOMEntityReference_predefined_free.phpt diff --git a/NEWS b/NEWS index 4cda13adfe4..7f08765f436 100644 --- a/NEWS +++ b/NEWS @@ -22,7 +22,6 @@ PHP NEWS (nielsdos) . Fix crash when toggleAttribute() is used without a document. (nielsdos) . Fix crash in adoptNode with attribute references. (nielsdos) - . Fix crashes with entity references and predefined entities. (nielsdos) - FFI: . Fixed bug GH-9698 (stream_wrapper_register crashes with FFI\CData). diff --git a/ext/dom/tests/DOMEntityReference_predefined_free.phpt b/ext/dom/tests/DOMEntityReference_predefined_free.phpt deleted file mode 100644 index 4b971d83703..00000000000 --- a/ext/dom/tests/DOMEntityReference_predefined_free.phpt +++ /dev/null @@ -1,46 +0,0 @@ ---TEST-- -Freeing of a predefined DOMEntityReference ---EXTENSIONS-- -dom ---FILE-- - ---EXPECT-- -object(DOMEntityReference)#1 (17) { - ["nodeName"]=> - string(3) "amp" - ["nodeValue"]=> - NULL - ["nodeType"]=> - int(5) - ["parentNode"]=> - NULL - ["parentElement"]=> - NULL - ["childNodes"]=> - string(22) "(object value omitted)" - ["firstChild"]=> - string(22) "(object value omitted)" - ["lastChild"]=> - string(22) "(object value omitted)" - ["previousSibling"]=> - NULL - ["nextSibling"]=> - NULL - ["attributes"]=> - NULL - ["isConnected"]=> - bool(false) - ["namespaceURI"]=> - NULL - ["prefix"]=> - string(0) "" - ["localName"]=> - NULL - ["baseURI"]=> - NULL - ["textContent"]=> - string(0) "" -} diff --git a/ext/dom/tests/delayed_freeing/entity_declaration.phpt b/ext/dom/tests/delayed_freeing/entity_declaration.phpt index 5caf29eedad..3e082611c35 100644 --- a/ext/dom/tests/delayed_freeing/entity_declaration.phpt +++ b/ext/dom/tests/delayed_freeing/entity_declaration.phpt @@ -9,32 +9,16 @@ $doc->loadXML(<<<'XML' - ]> XML); -$ref1 = $doc->createEntityReference("test"); -$ref2 = $doc->createEntityReference("myimage"); -$entity1 = $doc->doctype->entities[0]; -$entity2 = $doc->doctype->entities[1]; - -// Entity order depends on addresses -if ($entity1->nodeName !== "test") { - [$entity1, $entity2] = [$entity2, $entity1]; -} - -var_dump($entity1->nodeName, $entity1->parentNode->nodeName); -var_dump($entity2->nodeName, $entity2->parentNode->nodeName); +$entity = $doc->doctype->entities[0]; +var_dump($entity->nodeName, $entity->parentNode->nodeName); $doc->removeChild($doc->doctype); -var_dump($entity1->nodeName, $entity1->parentNode); -var_dump($entity2->nodeName, $entity2->parentNode); +var_dump($entity->nodeName, $entity->parentNode); ?> --EXPECT-- string(4) "test" string(5) "books" -string(7) "myimage" -string(5) "books" string(4) "test" NULL -string(7) "myimage" -NULL diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c index b95800a3c12..e8654abeed7 100644 --- a/ext/libxml/libxml.c +++ b/ext/libxml/libxml.c @@ -207,10 +207,12 @@ static void php_libxml_node_free(xmlNodePtr node) * dtd is attached to the document. This works around the issue by inspecting the parent directly. */ case XML_ENTITY_DECL: { xmlEntityPtr entity = (xmlEntityPtr) node; - if (entity->etype != XML_INTERNAL_PREDEFINED_ENTITY) { - php_libxml_unlink_entity_decl(entity); - xmlFreeEntity(entity); + php_libxml_unlink_entity_decl(entity); + if (entity->orig != NULL) { + xmlFree((char *) entity->orig); + entity->orig = NULL; } + xmlFreeNode(node); break; } case XML_NOTATION_NODE: { @@ -1384,15 +1386,6 @@ PHP_LIBXML_API void php_libxml_node_free_resource(xmlNodePtr node) case XML_DOCUMENT_NODE: case XML_HTML_DOCUMENT_NODE: break; - case XML_ENTITY_REF_NODE: - /* Entity reference nodes are special: their children point to entity declarations, - * but they don't own the declarations and therefore shouldn't free the children. - * Moreover, there can be N>1 reference nodes for a single entity declarations. */ - php_libxml_unregister_node(node); - if (node->parent == NULL) { - php_libxml_node_free(node); - } - break; default: if (node->parent == NULL || node->type == XML_NAMESPACE_DECL) { php_libxml_node_free_list((xmlNodePtr) node->children);