Skip to content

Commit

Permalink
Revert "Fix crashes with entity references and predefined entities"
Browse files Browse the repository at this point in the history
This reverts commit 3fa5af8.
  • Loading branch information
nielsdos committed Dec 23, 2023
1 parent bb00743 commit 5f69232
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 78 deletions.
1 change: 0 additions & 1 deletion NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
46 changes: 0 additions & 46 deletions ext/dom/tests/DOMEntityReference_predefined_free.phpt

This file was deleted.

22 changes: 3 additions & 19 deletions ext/dom/tests/delayed_freeing/entity_declaration.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -9,32 +9,16 @@ $doc->loadXML(<<<'XML'
<?xml version="1.0"?>
<!DOCTYPE books [
<!ENTITY test "entity is only for test purposes">
<!ENTITY myimage PUBLIC "-" "mypicture.gif" NDATA GIF>
]>
<container/>
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
17 changes: 5 additions & 12 deletions ext/libxml/libxml.c
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 5f69232

Please sign in to comment.