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

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index cbab8308d1ce03310a2e48d1c1519c1ad153442b..55154e0261ef9aefa7a74455e7c8fad85538ae75 100644 (file)
--- 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))