[php-src] master: Fix error paths in dom_xml_serializing_a_document_node()

Author: ndossche (ndossche) Date: 2026-10-24T19:14:15+02:00 Commit: https://github.com/php/php-src/commit/6454e102d816e115b083af539792a4cb20246ce4 Raw diff: https://github.com/php/php-src/commit/6454e102d816e115b083af539792a4cb20246ce4.diff Fix error paths in dom_xml_serializing_a_document_node() They should restore the child pointers even in case of a write failure. Closes GH-24564. Changed paths: A ext/dom/tests/modern/xml/serialize_document_write_failure.phpt A ext/dom/tests/modern/xml/serialize_document_write_mutation.phpt M NEWS M ext/dom/xml_serializer.c Diff: diff --git a/NEWS b/NEWS index 38476ae63111..952acfa5d8e9 100644 --- a/NEWS +++ b/NEWS @@ -53,6 +53,7 @@ PHP NEWS from a foreign document). (David Carlier) . Fixed DOMXPath crash wrapping a foreign node after a nodeset callback. (Ilia Alshanetsky) + . Fix error paths in dom_xml_serializing_a_document_node(). (ndossche) - FFI: . Fixed bug GH-24582 (Memory leaks in ext/ffi parser when finalize_type() diff --git a/ext/dom/tests/modern/xml/serialize_document_write_failure.phpt b/ext/dom/tests/modern/xml/serialize_document_write_failure.phpt new file mode 100644 index 000000000000..2e33cb5420cf --- /dev/null +++ b/ext/dom/tests/modern/xml/serialize_document_write_failure.phpt @@ -0,0 +1,39 @@ +--TEST-- +Dom\XMLDocument: failed write while serializing the document node must not detach its children +--EXTENSIONS-- +dom +--FILE-- +<?php + +class FailingStream { + public $context; + + public function stream_open(string $path, string $mode, int $options, ?string &$opened_path): bool { + return true; + } + + public function stream_write(string $data): int|false { + return false; + } + + public function stream_tell(): int { + return 0; + } +} + +stream_wrapper_register('failing', FailingStream::class); + +$doc = Dom\XMLDocument::createFromString('<root><child/></root>'); +var_dump($doc->saveXmlFile('failing://out')); + +var_dump($doc->documentElement?->nodeName); +var_dump($doc->childNodes->length); +echo $doc->saveXml(); + +?> +--EXPECT-- +bool(false) +string(4) "root" +int(1) +<?xml version="1.0" encoding="UTF-8"?> +<root><child/></root> diff --git a/ext/dom/tests/modern/xml/serialize_document_write_mutation.phpt b/ext/dom/tests/modern/xml/serialize_document_write_mutation.phpt new file mode 100644 index 000000000000..66127130ecff --- /dev/null +++ b/ext/dom/tests/modern/xml/serialize_document_write_mutation.phpt @@ -0,0 +1,40 @@ +--TEST-- +Dom\XMLDocument: mutating the document from a stream write during document serialization +--EXTENSIONS-- +dom +--FILE-- +<?php + +class MutatingStream { + public $context; + public static $doc; + + public function stream_open(string $path, string $mode, int $options, ?string &$opened_path): bool { + return true; + } + + public function stream_write(string $data): int { + if (self::$doc !== null) { + $doc = self::$doc; + self::$doc = null; + $doc->append($doc->createComment('injected')); + } + return strlen($data); + } +} + +stream_wrapper_register('mutating', MutatingStream::class); + +$doc = Dom\XMLDocument::createFromString('<root><child/></root>'); +MutatingStream::$doc = $doc; +$doc->saveXmlFile('mutating://out'); + +$doc->append($doc->createComment('after')); +var_dump($doc->childNodes->length); +echo $doc->saveXml(), "\n"; + +?> +--EXPECT-- +int(3) +<?xml version="1.0" encoding="UTF-8"?> +<root><child/></root><!--injected--><!--after--> diff --git a/ext/dom/xml_serializer.c b/ext/dom/xml_serializer.c index 2d9e55a7b706..bd7e8bf6aa8d 100644 --- a/ext/dom/xml_serializer.c +++ b/ext/dom/xml_serializer.c @@ -1228,18 +1228,18 @@ static int dom_xml_serializing_a_document_node( /* 1. Let serialized document be an empty string. * => We use the output buffer instead. */ - xmlNodePtr child = node->children; - node->children = NULL; - /* https://github.com/w3c/DOM-Parsing/issues/50 */ TRY(xmlOutputBufferFlush(ctx->out)); - TRY(xmlSaveDoc(ctx->ctxt, (xmlDocPtr) node)); + /* To gain the xml declaration, use a shallow copy of the document with the children NULLed out. + * Stream wrappers can access the real document, so we cannot modify that. */ + xmlDoc declaration_doc = *(xmlDocPtr) node; + declaration_doc.children = declaration_doc.last = NULL; + TRY(xmlSaveDoc(ctx->ctxt, &declaration_doc)); TRY(xmlSaveFlush(ctx->ctxt)); - node->children = child; - /* 2. For each child child of node, in tree order, run the XML serialization algorithm on the child passing along the provided arguments, * and append the result to serialized document. */ + xmlNodePtr child = node->children; while (child != NULL) { TRY(dom_xml_serialization_algorithm(ctx, namespace_prefix_map, child, namespace, prefix_index, indent, require_well_formed)); child = child->next;