From c11820007c02bcf5b3a1ae1ba765ba3382e0d448 Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Thu, 22 Oct 2026 19:31:52 +0200 Subject: [PATCH 1/2] Fix error paths in dom_xml_serializing_a_document_node() They should restore the child pointers even in case of a write failure. --- .../xml/serialize_document_write_failure.phpt | 39 +++++++++++++++++++ ext/dom/xml_serializer.c | 10 +++-- 2 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 ext/dom/tests/modern/xml/serialize_document_write_failure.phpt 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-- +'); +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) + + diff --git a/ext/dom/xml_serializer.c b/ext/dom/xml_serializer.c index 2d9e55a7b706..ff6dfc74dfe6 100644 --- a/ext/dom/xml_serializer.c +++ b/ext/dom/xml_serializer.c @@ -1232,9 +1232,9 @@ static int dom_xml_serializing_a_document_node( node->children = NULL; /* https://github.com/w3c/DOM-Parsing/issues/50 */ - TRY(xmlOutputBufferFlush(ctx->out)); - TRY(xmlSaveDoc(ctx->ctxt, (xmlDocPtr) node)); - TRY(xmlSaveFlush(ctx->ctxt)); + TRY_OR_CLEANUP(xmlOutputBufferFlush(ctx->out)); + TRY_OR_CLEANUP(xmlSaveDoc(ctx->ctxt, (xmlDocPtr) node)); + TRY_OR_CLEANUP(xmlSaveFlush(ctx->ctxt)); node->children = child; @@ -1248,6 +1248,10 @@ static int dom_xml_serializing_a_document_node( /* 3. Return the value of serialized document. * => We use the output buffer instead. */ return 0; + +cleanup: + node->children = child; + return -1; } static zend_always_inline bool dom_xml_serialize_check_stack_limit(void) From 37fded17055dcc32ebaaf3ea858e21d97c9a723c Mon Sep 17 00:00:00 2001 From: ndossche <7771979+ndossche@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:35:42 +0200 Subject: [PATCH 2/2] Fix Ilia's remark --- .../serialize_document_write_mutation.phpt | 40 +++++++++++++++++++ ext/dom/xml_serializer.c | 20 ++++------ 2 files changed, 48 insertions(+), 12 deletions(-) create mode 100644 ext/dom/tests/modern/xml/serialize_document_write_mutation.phpt 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-- +append($doc->createComment('injected')); + } + return strlen($data); + } +} + +stream_wrapper_register('mutating', MutatingStream::class); + +$doc = Dom\XMLDocument::createFromString(''); +MutatingStream::$doc = $doc; +$doc->saveXmlFile('mutating://out'); + +$doc->append($doc->createComment('after')); +var_dump($doc->childNodes->length); +echo $doc->saveXml(), "\n"; + +?> +--EXPECT-- +int(3) + + diff --git a/ext/dom/xml_serializer.c b/ext/dom/xml_serializer.c index ff6dfc74dfe6..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_OR_CLEANUP(xmlOutputBufferFlush(ctx->out)); - TRY_OR_CLEANUP(xmlSaveDoc(ctx->ctxt, (xmlDocPtr) node)); - TRY_OR_CLEANUP(xmlSaveFlush(ctx->ctxt)); - - node->children = child; + TRY(xmlOutputBufferFlush(ctx->out)); + /* 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)); /* 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; @@ -1248,10 +1248,6 @@ static int dom_xml_serializing_a_document_node( /* 3. Return the value of serialized document. * => We use the output buffer instead. */ return 0; - -cleanup: - node->children = child; - return -1; } static zend_always_inline bool dom_xml_serialize_check_stack_limit(void)