Replace xmlroot with a properly functioning version that parses the value,

sets the items, and serializes the value back (rather than adding an
arbitrary number of XML preambles as before).

The libxml memory management via palloc had to be disabled because it
crashes when libxml tries to access memory that was helpfully freed
earlier by PostgreSQL.  This needs further thought.

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 6771360c48abff8acabcb1e0daca8bed388f9e36..ab929eed64720308428a971193373308268170b9 100644 (file)
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/utils/adt/xml.c,v 1.10 2007/01/05 22:19:42 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/adt/xml.c,v 1.11 2007/01/06 19:18:36 petere Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -31,6 +31,7 @@
 #include <libxml/tree.h>
 #include <libxml/uri.h>
 #include <libxml/xmlerror.h>
+#include <libxml/xmlsave.h>
 #endif /* USE_LIBXML */
 
 #include "fmgr.h"
@@ -49,10 +50,12 @@
 static StringInfo xml_err_buf = NULL;
 
 static void    xml_init(void);
+#ifdef NOT_USED
 static void    *xml_palloc(size_t size);
 static void    *xml_repalloc(void *ptr, size_t size);
 static void    xml_pfree(void *ptr);
 static char    *xml_pstrdup(const char *string);
+#endif
 static void    xml_ereport(int level, int sqlcode,
                                                        const char *msg, void *ctxt);
 static void    xml_errorHandler(void *ctxt, const char *msg, ...);
@@ -76,6 +79,7 @@ xml_in(PG_FUNCTION_ARGS)
        char            *s = PG_GETARG_CSTRING(0);
        size_t          len;
        xmltype         *vardata;
+       xmlDocPtr        doc;
 
        len = strlen(s);
        vardata = palloc(len + VARHDRSZ);
@@ -86,7 +90,8 @@ xml_in(PG_FUNCTION_ARGS)
         * Parse the data to check if it is well-formed XML data.  Assume
         * that ERROR occurred if parsing failed.
         */
-       xml_parse(vardata, false, true);
+       doc = xml_parse(vardata, false, true);
+       xmlFreeDoc(doc);
 
        PG_RETURN_XML_P(vardata);
 #else
@@ -120,6 +125,7 @@ xml_recv(PG_FUNCTION_ARGS)
        xmltype    *result;
        char       *str;
        int                     nbytes;
+       xmlDocPtr       doc;
 
        str = pq_getmsgtext(buf, buf->len - buf->cursor, &nbytes);
 
@@ -132,7 +138,8 @@ xml_recv(PG_FUNCTION_ARGS)
         * Parse the data to check if it is well-formed XML data.  Assume
         * that ERROR occurred if parsing failed.
         */
-       xml_parse(result, false, true);
+       doc = xml_parse(result, false, true);
+       xmlFreeDoc(doc);
 
        PG_RETURN_XML_P(result);
 #else
@@ -175,6 +182,21 @@ stringinfo_to_xmltype(StringInfo buf)
 
        return result;
 }
+
+
+static xmltype *
+xmlBuffer_to_xmltype(xmlBufferPtr buf)
+{
+       int32           len;
+       xmltype    *result;
+
+       len = xmlBufferLength(buf) + VARHDRSZ;
+       result = palloc(len);
+       VARATT_SIZEP(result) = len;
+       memcpy(VARDATA(result), xmlBufferContent(buf), len - VARHDRSZ);
+
+       return result;
+}
 #endif
 
 
@@ -221,7 +243,10 @@ xmltype *
 xmlparse(text *data, bool is_document, bool preserve_whitespace)
 {
 #ifdef USE_LIBXML
-       xml_parse(data, is_document, preserve_whitespace);
+       xmlDocPtr       doc;
+
+       doc = xml_parse(data, is_document, preserve_whitespace);
+       xmlFreeDoc(doc);
 
        return (xmltype *) data;
 #else
@@ -280,31 +305,38 @@ xmltype *
 xmlroot(xmltype *data, text *version, int standalone)
 {
 #ifdef USE_LIBXML
-       xmltype *result;
-       StringInfoData buf;
+       xmlDocPtr       doc;
+       xmlBufferPtr buffer;
+       xmlSaveCtxtPtr save;
 
-       initStringInfo(&buf);
+       doc = xml_parse((text *) data, true, true);
 
-       /*
-        * FIXME: This is probably supposed to be cleverer if there
-        * already is an XML preamble.
-        */
-       appendStringInfo(&buf,"<?xml");
        if (version)
+               doc->version = xmlStrdup(xml_text2xmlChar(version));
+       else
+               doc->version = NULL;
+
+       switch (standalone)
        {
-               appendStringInfo(&buf, " version=\"");
-               appendStringInfoText(&buf, version);
-               appendStringInfo(&buf, "\"");
+               case 1:
+                       doc->standalone = 1;
+                       break;
+               case -1:
+                       doc->standalone = 0;
+                       break;
+               default:
+                       doc->standalone = -1;
+                       break;
        }
-       if (standalone)
-               appendStringInfo(&buf, " standalone=\"%s\"",
-                                                (standalone == 1 ? "yes" : "no"));
-       appendStringInfo(&buf, "?>");
-       appendStringInfoText(&buf, (text *) data);
 
-       result = stringinfo_to_xmltype(&buf);
-       pfree(buf.data);
-       return result;
+       buffer = xmlBufferCreate();
+       save = xmlSaveToBuffer(buffer, NULL, 0);
+       xmlSaveDoc(save, doc);
+       xmlSaveClose(save);
+
+       xmlFreeDoc(doc);
+
+       return xmlBuffer_to_xmltype(buffer);
 #else
        NO_XML_SUPPORT();
        return NULL;
@@ -444,7 +476,14 @@ xml_init(void)
        /* Now that xml_err_buf exists, safe to call xml_errorHandler */
        xmlSetGenericErrorFunc(NULL, xml_errorHandler);
 
+#ifdef NOT_USED
+       /*
+        * FIXME: This doesn't work because libxml assumes that whatever
+        * libxml allocates, only libxml will free, so we can't just drop
+        * memory contexts behind it.  This needs to be refined.
+        */
        xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup);
+#endif
        xmlInitParser();
        LIBXML_TEST_VERSION;
 }
@@ -528,8 +567,6 @@ xml_parse(text *data, bool is_document, bool preserve_whitespace)
                 * ) */
                /* ... */
 
-               if (doc)
-                       xmlFreeDoc(doc);
                if (ctxt)
                        xmlFreeParserCtxt(ctxt);
                xmlCleanupParser();
@@ -538,6 +575,7 @@ xml_parse(text *data, bool is_document, bool preserve_whitespace)
        {
                if (doc)
                        xmlFreeDoc(doc);
+               doc = NULL;
                if (ctxt)
                        xmlFreeParserCtxt(ctxt);
                xmlCleanupParser();
@@ -567,6 +605,7 @@ xml_text2xmlChar(text *in)
 }
 
 
+#ifdef NOT_USED
 /*
  * Wrappers for memory management functions
  */
@@ -596,6 +635,7 @@ xml_pstrdup(const char *string)
 {
        return pstrdup(string);
 }
+#endif /* NOT_USED */
 
 
 /*

diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 14744656124008b0a7f24f3d5dff2d295a1bc18a..9ebef1404b91cf32506508b586004d367432a4cc 100644 (file)
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -124,6 +124,30 @@ SELECT xmlpi(name foo, 'bar');
 SELECT xmlpi(name foo, 'in?>valid');
 ERROR:  invalid XML processing instruction
 DETAIL:  XML processing instruction cannot contain "?>".
+SELECT xmlroot(xml '<foo/>', version no value, standalone no value);
+        xmlroot        
+-----------------------
+ <?xml version="1.0"?>
+ <foo/>
+ 
+(1 row)
+
+SELECT xmlroot(xml '<foo/>', version '2.0');
+        xmlroot        
+-----------------------
+ <?xml version="2.0"?>
+ <foo/>
+ 
+(1 row)
+
+SELECT xmlroot(xmlroot(xml '<foo/>', version '1.0'), version '1.1', standalone no);
+                xmlroot                
+---------------------------------------
+ <?xml version="1.1" standalone="no"?>
+ <foo/>
+ 
+(1 row)
+
 SELECT xmlroot (
   xmlelement (
     name gazonk,
@@ -139,9 +163,11 @@ SELECT xmlroot (
   version '1.0',
   standalone yes
 );
-                                         xmlroot                                          
-------------------------------------------------------------------------------------------
- <?xml version="1.0" standalone="yes"?><gazonk name="val" num="2"><qux>foo</qux></gazonk>
+                      xmlroot                       
+----------------------------------------------------
+ <?xml version="1.0" standalone="yes"?>
+ <gazonk name="val" num="2"><qux>foo</qux></gazonk>
+ 
 (1 row)
 
 SELECT xmlserialize(content data as character varying) FROM xmltest;

diff --git a/src/test/regress/expected/xml_1.out b/src/test/regress/expected/xml_1.out
index 259bccbf4ba45aacee75354a9be5ff29e9db35eb..662c0e236a8113bd3003a648680ee33563696781 100644 (file)
--- a/src/test/regress/expected/xml_1.out
+++ b/src/test/regress/expected/xml_1.out
@@ -62,6 +62,12 @@ SELECT xmlpi(name foo, 'bar');
 ERROR:  no XML support in this installation
 SELECT xmlpi(name foo, 'in?>valid');
 ERROR:  no XML support in this installation
+SELECT xmlroot(xml '<foo/>', version no value, standalone no value);
+ERROR:  no XML support in this installation
+SELECT xmlroot(xml '<foo/>', version '2.0');
+ERROR:  no XML support in this installation
+SELECT xmlroot(xmlroot(xml '<foo/>', version '1.0'), version '1.1', standalone no);
+ERROR:  no XML support in this installation
 SELECT xmlroot (
   xmlelement (
     name gazonk,

diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index 11db4905c7674b1c756f9edde7a09eebdfaedb33..cf96beab060729ee8b18f5e71778428d45d5561d 100644 (file)
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -52,6 +52,9 @@ SELECT xmlpi(name xmlstuff);
 SELECT xmlpi(name foo, 'bar');
 SELECT xmlpi(name foo, 'in?>valid');
 
+SELECT xmlroot(xml '<foo/>', version no value, standalone no value);
+SELECT xmlroot(xml '<foo/>', version '2.0');
+SELECT xmlroot(xmlroot(xml '<foo/>', version '1.0'), version '1.1', standalone no);
 
 SELECT xmlroot (
   xmlelement (