Fix intratransaction memory leaks in xml_recv, xmlconcat, xmlroot, and

xml_parse, all arising from the same sloppy usage of parse_xml_decl.
The original coding had that function returning its output string
parameters in the libxml context, which is long-lived, and all but one
of its callers neglected to free the strings afterwards.  The easiest
and most bulletproof fix is to return the strings in the local palloc
context instead, since that's short-lived.  This was only costing a
dozen or two bytes per function call, but that adds up fast if the
function is called repeatedly ...

Noted while poking at the more general problem of what to do with our
libxml memory allocation hooks.  Back-patch to 8.3, which has the
identical coding.

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 5cb761773d921e3069cb1131eab773b86cebc0e5..7cfca13feaf65174e20df30b6aca02761e2569f2 100644 (file)
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/xml.c,v 1.86 2009/04/08 21:51:38 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/xml.c,v 1.87 2009/05/12 20:17:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -236,8 +236,6 @@ xml_out_internal(xmltype *x, pg_enc target_encoding)
                }
                appendStringInfoString(&buf, str + len);
 
-               if (version)
-                       xmlFree(version);
                pfree(str);
 
                return buf.data;
@@ -455,7 +453,7 @@ xmlconcat(List *args)
                if (!version)
                        global_version_no_value = true;
                else if (!global_version)
-                       global_version = xmlStrdup(version);
+                       global_version = version;
                else if (xmlStrcmp(version, global_version) != 0)
                        global_version_no_value = true;
 
@@ -918,6 +916,24 @@ xml_init(void)
                        || xmlIsCombiningQ(c) \
                        || xmlIsExtender_ch(c))
 
+/* pnstrdup, but deal with xmlChar not char; len is measured in xmlChars */
+static xmlChar *
+xml_pnstrdup(const xmlChar *str, size_t len)
+{
+       xmlChar    *result;
+
+       result = (xmlChar *) palloc((len + 1) * sizeof(xmlChar));
+       memcpy(result, str, len * sizeof(xmlChar));
+       result[len] = 0;
+       return result;
+}
+
+/*
+ * str is the null-terminated input string.  Remaining arguments are
+ * output arguments; each can be NULL if value is not wanted.
+ * version and encoding are returned as locally-palloc'd strings.
+ * Result is 0 if OK, an error code if not.
+ */
 static int
 parse_xml_decl(const xmlChar * str, size_t *lenp,
                           xmlChar ** version, xmlChar ** encoding, int *standalone)
@@ -930,6 +946,7 @@ parse_xml_decl(const xmlChar * str, size_t *lenp,
 
        xml_init();
 
+       /* Initialize output arguments to "not present" */
        if (version)
                *version = NULL;
        if (encoding)
@@ -971,7 +988,7 @@ parse_xml_decl(const xmlChar * str, size_t *lenp,
                        return XML_ERR_VERSION_MISSING;
 
                if (version)
-                       *version = xmlStrndup(p + 1, q - p - 1);
+                       *version = xml_pnstrdup(p + 1, q - p - 1);
                p = q + 1;
        }
        else
@@ -999,7 +1016,7 @@ parse_xml_decl(const xmlChar * str, size_t *lenp,
                                return XML_ERR_MISSING_ENCODING;
 
                        if (encoding)
-                               *encoding = xmlStrndup(p + 1, q - p - 1);
+                               *encoding = xml_pnstrdup(p + 1, q - p - 1);
                        p = q + 1;
                }
                else
@@ -1172,8 +1189,6 @@ xml_parse(text *data, XmlOptionType xmloption_arg, bool preserve_whitespace,
                xmlChar    *version = NULL;
                int                     standalone = -1;
 
-               doc = xmlNewDoc(NULL);
-
                res_code = parse_xml_decl(utf8string,
                                                                  &count, &version, NULL, &standalone);
                if (res_code != 0)
@@ -1181,15 +1196,16 @@ xml_parse(text *data, XmlOptionType xmloption_arg, bool preserve_whitespace,
                                                                "invalid XML content: invalid XML declaration",
                                                                res_code);
 
+               doc = xmlNewDoc(version);
+               Assert(doc->encoding == NULL);
+               doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
+               doc->standalone = standalone;
+
                res_code = xmlParseBalancedChunkMemory(doc, NULL, NULL, 0,
                                                                                           utf8string + count, NULL);
                if (res_code != 0)
                        xml_ereport(ERROR, ERRCODE_INVALID_XML_CONTENT,
                                                "invalid XML content");
-
-               doc->version = xmlStrdup(version);
-               doc->encoding = xmlStrdup((xmlChar *) "UTF-8");
-               doc->standalone = standalone;
        }
 
        xmlFreeParserCtxt(ctxt);