diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2025-05-22 13:52:46 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2025-05-22 13:52:46 -0400 |
| commit | f24605e2dc1687917766f43775f0dcde2cf678a0 (patch) | |
| tree | 215d04b8affbc0c141d0828b5169c3a9dcd42ff6 /src/backend/utils/adt/xml.c | |
| parent | 5d6eac80cdce7aa7c5f4ec74208ddc1feea9eef3 (diff) | |
| download | postgresql-f24605e2dc1687917766f43775f0dcde2cf678a0.tar.gz postgresql-f24605e2dc1687917766f43775f0dcde2cf678a0.zip | |
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
Diffstat (limited to 'src/backend/utils/adt/xml.c')
| -rw-r--r-- | src/backend/utils/adt/xml.c | 21 |
1 files changed, 17 insertions, 4 deletions
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c index db8d0d6a7e8..a4150bff2ea 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -754,6 +754,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"); @@ -761,8 +762,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); /* @@ -1850,6 +1857,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); @@ -1868,8 +1876,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)) |