postgres.git / summary / log / commit / refs

commit    ee58de10084b42a24aaa3ab4af7713bb2fe02ab4
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Thu May 22 17:52:46 2025 +0000
First released in: 18.0, 16.10, 17.6

    Fix memory leak in XMLSERIALIZE(... INDENT).
    
    xmltotext_with_options sometimes tries to replace the existing
    root node of a libxml2 document.  In that case xmlDocSetRootElement
    will unlink and return the old root node; if we fail to free it,
    it's leaked for the remainder of the session.  The amount of memory
    at stake is not large, a couple hundred bytes per occurrence, but
    that could still become annoying in heavy usage.
    
    Our only other xmlDocSetRootElement call is not at risk because
    it's working on a just-created document, but let's modify that
    code too to make it clear that it's dependent on that.
    
    Author: Tom Lane <tgl@sss.pgh.pa.us>
    Reviewed-by: Jim Jones <jim.jones@uni-muenster.de>
    Discussion: https://postgr.es/m/1358967.1747858817@sss.pgh.pa.us
    Backpatch-through: 16


 src/backend/utils/adt/xml.c | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index cbab8308d1c..55154e0261e 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -733,6 +733,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
 			 * content nodes, and then iterate over the nodes.
 			 */
 			xmlNodePtr	root;
+			xmlNodePtr	oldroot;
 			xmlNodePtr	newline;
 
 			root = xmlNewNode(NULL, (const xmlChar *) "content-root");
@@ -740,8 +741,14 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
 				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
 							"could not allocate xml node");
 
-			/* This attaches root to doc, so we need not free it separately. */
-			xmlDocSetRootElement(doc, root);
+			/*
+			 * This attaches root to doc, so we need not free it separately...
+			 * but instead, we have to free the old root if there was one.
+			 */
+			oldroot = xmlDocSetRootElement(doc, root);
+			if (oldroot != NULL)
+				xmlFreeNode(oldroot);
+
 			xmlAddChildList(root, content_nodes);
 
 			/*
@@ -1829,6 +1836,7 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 		else
 		{
 			xmlNodePtr	root;
+			xmlNodePtr	oldroot PG_USED_FOR_ASSERTS_ONLY;
 
 			/* set up document with empty root node to be the context node */
 			doc = xmlNewDoc(version);
@@ -1847,8 +1855,13 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 			if (root == NULL || xmlerrcxt->err_occurred)
 				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
 							"could not allocate xml node");
-			/* This attaches root to doc, so we need not free it separately. */
-			xmlDocSetRootElement(doc, root);
+
+			/*
+			 * This attaches root to doc, so we need not free it separately;
+			 * and there can't yet be any old root to free.
+			 */
+			oldroot = xmlDocSetRootElement(doc, root);
+			Assert(oldroot == NULL);
 
 			/* allow empty content */
 			if (*(utf8string + count))


[parent: 0d2063585876]