Modernize our parsing of XML data in CONTENT mode.

The main goal of this patch is to get rid of our use of
xmlKeepBlanksDefault, which libxml2 has deprecated.
To do that, we want to use xmlParseInNodeContext instead of
xmlParseBalancedChunkMemory, so that we can pass an options
mask that includes XML_PARSE_NOBLANKS.

xmlParseInNodeContext wants an xmlNode rather than an xmlDoc as
context.  This gives us an opportunity for simplification though:
we can make that node be a dummy root node for xml_parse's result
document, and then attach the content node(s) as children of the root,
essentially moving the data structure hack in xmltotext_with_options
into xml_parse.  The main benefit of this is that callers no longer
need to free the content node list separately; freeing the document is
sufficient.  (There were error paths in xmltotext_with_options that
I think would have leaked that list, though they could only be
reached in OOM or similar conditions.)

We also do some fooling with doc->encoding, to be bug-compatible
with our previous behavior on as many libxml2 versions as practical.
Some small discrepancies with previous behavior are known with older
libxml2 versions.  Therefore, it seems inadvisable to back-patch this,
at least not before it's gotten a good deal of field experience.

Author: Tom Lane <tgl@sss.pgh.pa.us>
Fix-suggested-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Rui Zhao <zhaorui126@gmail.com>
Discussion: https://postgr.es/m/ZR2PPFF1830B8772632FA23847D7482168FD2C62@ZR2PPFF1830B877.CHEP278.PROD.OUTLOOK.COM

src/backend/utils/adt/xml.c

@@ -157,7 +157,6 @@ static bool xml_doctype_in_content(const xmlChar *str);
 static xmlDocPtr xml_parse(text *data, XmlOptionType xmloption_arg,
 						   bool preserve_whitespace, int encoding,
 						   XmlOptionType *parsed_xmloptiontype,
-						   xmlNodePtr *parsed_nodes,
 						   Node *escontext);
 static text *xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext *xmlerrcxt);
 static int	xml_xpathobjtoxmlarray(xmlXPathObjectPtr xpathobj,
@@ -285,7 +284,7 @@ xml_in(PG_FUNCTION_ARGS)
 	 * Note: we don't need to worry about whether a soft error is detected.
 	 */
 	doc = xml_parse(vardata, xmloption, true, GetDatabaseEncoding(),
-					NULL, NULL, fcinfo->context);
+					NULL, fcinfo->context);
 	if (doc != NULL)
 		xmlFreeDoc(doc);
 
@@ -413,7 +412,7 @@ xml_recv(PG_FUNCTION_ARGS)
 	 * Parse the data to check if it is well-formed XML data.  Assume that
 	 * xml_parse will throw ERROR if not.
 	 */
-	doc = xml_parse(result, xmloption, true, encoding, NULL, NULL, NULL);
+	doc = xml_parse(result, xmloption, true, encoding, NULL, NULL);
 	xmlFreeDoc(doc);
 
 	/* Now that we know what we're dealing with, convert to server encoding */
@@ -681,7 +680,6 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
 	text	   *volatile result;
 	xmlDocPtr	doc;
 	XmlOptionType parsed_xmloptiontype;
-	xmlNodePtr	content_nodes;
 	volatile xmlBufferPtr buf = NULL;
 	volatile xmlSaveCtxtPtr ctxt = NULL;
 	ErrorSaveContext escontext = {T_ErrorSaveContext};
@@ -707,7 +705,7 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
 	 * libxml2 will fail to indent elements that have whitespace between them.
 	 */
 	doc = xml_parse(data, xmloption_arg, !indent, GetDatabaseEncoding(),
-					&parsed_xmloptiontype, &content_nodes,
+					&parsed_xmloptiontype,
 					(Node *) &escontext);
 	if (doc == NULL || escontext.error_occurred)
 	{
@@ -772,35 +770,16 @@ xmltotext_with_options(xmltype *data, XmlOptionType xmloption_arg, bool indent)
 				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
 							"could not save document to xmlBuffer");
 		}
-		else if (content_nodes != NULL)
+		else
 		{
 			/*
-			 * Deal with the case where we have non-singly-rooted XML.
+			 * When we have non-singly-rooted XML, we need to insert newlines
-			 * libxml's dump functions don't work well for that without help.
+			 * between the content nodes, else the output is ugly.
-			 * We build a fake root node that serves as a container for the
-			 * content nodes, and then iterate over the nodes.
 			 */
-			xmlNodePtr	root;
+			xmlNodePtr	root = xmlDocGetRootElement(doc);
-			xmlNodePtr	oldroot;
 			xmlNodePtr	newline;
 
-			root = xmlNewNode(NULL, (const xmlChar *) "content-root");
+			Assert(root != NULL);
-			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...
-			 * but instead, we have to free the old root if there was one.
-			 */
-			oldroot = xmlDocSetRootElement(doc, root);
-			if (oldroot != NULL)
-				xmlFreeNode(oldroot);
-
-			if (xmlAddChildList(root, content_nodes) == NULL ||
-				xmlerrcxt->err_occurred)
-				xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
-							"could not append xml node list");
 
 			/*
 			 * We use this node to insert newlines in the dump.  Note: in at
@@ -1035,7 +1014,7 @@ xmlparse(text *data, XmlOptionType xmloption_arg, bool preserve_whitespace, Node
 	xmlDocPtr	doc;
 
 	doc = xml_parse(data, xmloption_arg, preserve_whitespace,
-					GetDatabaseEncoding(), NULL, NULL, escontext);
+					GetDatabaseEncoding(), NULL, escontext);
 	if (doc)
 		xmlFreeDoc(doc);
 
@@ -1179,7 +1158,7 @@ xml_is_document(xmltype *arg)
 	 * We'll report "true" if no soft error is reported by xml_parse().
 	 */
 	doc = xml_parse((text *) arg, XMLOPTION_DOCUMENT, true,
-					GetDatabaseEncoding(), NULL, NULL, (Node *) &escontext);
+					GetDatabaseEncoding(), NULL, (Node *) &escontext);
 	if (doc)
 		xmlFreeDoc(doc);
 
@@ -1772,9 +1751,8 @@ xml_doctype_in_content(const xmlChar *str)
  * XmlOptionType actually used to parse the input (typically the same as
  * xmloption_arg, but a DOCTYPE node in the input can force DOCUMENT mode).
  *
- * If parsed_nodes isn't NULL and we parse in CONTENT mode, the list
+ * When we parse in CONTENT mode, the resulting document has a dummy root
- * of parsed nodes from the xmlParseBalancedChunkMemory call will be returned
+ * node with zero or more child nodes, reflecting the contents found.
- * to *parsed_nodes.  (It is caller's responsibility to free that.)
  *
  * Errors normally result in ereport(ERROR), but if escontext is an
  * ErrorSaveContext, then "safe" errors are reported there instead, and the
@@ -1790,7 +1768,7 @@ xml_doctype_in_content(const xmlChar *str)
 static xmlDocPtr
 xml_parse(text *data, XmlOptionType xmloption_arg,
 		  bool preserve_whitespace, int encoding,
-		  XmlOptionType *parsed_xmloptiontype, xmlNodePtr *parsed_nodes,
+		  XmlOptionType *parsed_xmloptiontype,
 		  Node *escontext)
 {
 	int32		len;
@@ -1799,7 +1777,6 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 	PgXmlErrorContext *xmlerrcxt;
 	volatile xmlParserCtxtPtr ctxt = NULL;
 	volatile xmlDocPtr doc = NULL;
-	volatile int save_keep_blanks = -1;
 
 	/*
 	 * This step looks annoyingly redundant, but we must do it to have a
@@ -1827,10 +1804,10 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 	PG_TRY();
 	{
 		bool		parse_as_document = false;
-		int			res_code;
 		size_t		count = 0;
 		xmlChar    *version = NULL;
 		int			standalone = 0;
+		int			options;
 
 		/* Any errors here are reported as hard ereport's */
 		xmlInitParser();
@@ -1841,6 +1818,8 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 		else
 		{
 			/* Parse and skip over the XML declaration, if any */
+			int			res_code;
+
 			res_code = parse_xml_decl(utf8string,
 									  &count, &version, NULL, &standalone);
 			if (res_code != 0)
@@ -1857,35 +1836,31 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 				parse_as_document = true;
 		}
 
-		/* initialize output parameters */
+		/*
+		 * Select parse options.
+		 *
+		 * Note that here we try to apply DTD defaults (XML_PARSE_DTDATTR)
+		 * according to SQL/XML:2008 GR 10.16.7.d: 'Default values defined by
+		 * internal DTD are applied'.  As for external DTDs, we try to support
+		 * them too (see SQL/XML:2008 GR 10.16.7.e), but that doesn't really
+		 * happen because xmlPgEntityLoader prevents it.
+		 */
+		options = XML_PARSE_NOENT | XML_PARSE_DTDATTR
+			| (preserve_whitespace ? 0 : XML_PARSE_NOBLANKS);
+
+		/* initialize output parameter */
 		if (parsed_xmloptiontype != NULL)
 			*parsed_xmloptiontype = parse_as_document ? XMLOPTION_DOCUMENT :
 				XMLOPTION_CONTENT;
-		if (parsed_nodes != NULL)
-			*parsed_nodes = NULL;
 
 		if (parse_as_document)
 		{
-			int			options;
-
 			/* set up parser context used by xmlCtxtReadDoc */
 			ctxt = xmlNewParserCtxt();
 			if (ctxt == NULL || xmlerrcxt->err_occurred)
 				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
 							"could not allocate parser context");
 
-			/*
-			 * Select parse options.
-			 *
-			 * Note that here we try to apply DTD defaults (XML_PARSE_DTDATTR)
-			 * according to SQL/XML:2008 GR 10.16.7.d: 'Default values defined
-			 * by internal DTD are applied'.  As for external DTDs, we try to
-			 * support them too (see SQL/XML:2008 GR 10.16.7.e), but that
-			 * doesn't really happen because xmlPgEntityLoader prevents it.
-			 */
-			options = XML_PARSE_NOENT | XML_PARSE_DTDATTR
-				| (preserve_whitespace ? 0 : XML_PARSE_NOBLANKS);
-
 			doc = xmlCtxtReadDoc(ctxt, utf8string,
 								 NULL,	/* no URL */
 								 "UTF-8",
@@ -1907,29 +1882,61 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 		}
 		else
 		{
-			/* set up document that xmlParseBalancedChunkMemory will add to */
+			xmlNodePtr	root;
+
+			/*
+			 * Create our result document and set up its dummy root node,
+			 * which we'll use as the context node for xmlParseInNodeContext.
+			 */
 			doc = xmlNewDoc(version);
 			if (doc == NULL || xmlerrcxt->err_occurred)
 				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
 							"could not allocate XML document");
 
-			Assert(doc->encoding == NULL);
+			root = xmlNewNode(NULL, (const xmlChar *) "content-root");
-			doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
+			if (root == NULL || xmlerrcxt->err_occurred)
-			if (doc->encoding == NULL || xmlerrcxt->err_occurred)
 				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
-							"could not allocate XML document");
+							"could not allocate xml node");
-			doc->standalone = standalone;
+			xmlDocSetRootElement(doc, root);
 
-			/* set parse options --- have to do this the ugly way */
+			/*
-			save_keep_blanks = xmlKeepBlanksDefault(preserve_whitespace ? 1 : 0);
+			 * Leave doc->encoding unset while parsing the UTF-8 content.
+			 * Older libxml2 versions otherwise strip a leading U+FEFF as a
+			 * BOM, even when it followed the XML declaration in the original
+			 * input.  libxml2 will treat the data as UTF-8 anyway, and there
+			 * do not seem to be any negative side-effects during parsing.
+			 */
+			Assert(doc->encoding == NULL);
+			doc->standalone = standalone;
 
 			/* allow empty content */
 			if (*(utf8string + count))
 			{
-				res_code = xmlParseBalancedChunkMemory(doc, NULL, NULL, 0,
+				/* for some reason we need "char *" not "xmlChar *" here */
-													   utf8string + count,
+				const char *content = (const char *) (utf8string + count);
-													   parsed_nodes);
+				xmlNodePtr	nodelist = NULL;
-				if (res_code != 0 || xmlerrcxt->err_occurred)
+				xmlParserErrors res_code;
+
+				res_code = xmlParseInNodeContext(root,
+												 content,
+												 strlen(content),
+												 options,
+												 &nodelist);
+
+				/* immediately attach the resulting nodelist to the root */
+				if (nodelist)
+				{
+					if (xmlAddChildList(root, nodelist) == NULL)
+					{
+						/* don't leak the nodelist if we can't attach it */
+						xmlFreeNodeList(nodelist);
+						xml_ereport(xmlerrcxt, ERROR, ERRCODE_INTERNAL_ERROR,
+									"could not append xml node list");
+					}
+					/* now, freeing the document will release the nodelist */
+				}
+
+				if (res_code != XML_ERR_OK || xmlerrcxt->err_occurred)
 				{
 					xml_errsave(escontext, xmlerrcxt,
 								ERRCODE_INVALID_XML_CONTENT,
@@ -1937,6 +1944,16 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 					goto fail;
 				}
 			}
+
+			/*
+			 * Now install the correct encoding setting.  If we don't, some
+			 * aspects of document dumping don't do what we want.
+			 */
+			Assert(doc->encoding == NULL);
+			doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
+			if (doc->encoding == NULL || xmlerrcxt->err_occurred)
+				xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+							"could not allocate XML document");
 		}
 
 fail:
@@ -1944,8 +1961,6 @@ fail:
 	}
 	PG_CATCH();
 	{
-		if (save_keep_blanks != -1)
-			xmlKeepBlanksDefault(save_keep_blanks);
 		if (doc != NULL)
 			xmlFreeDoc(doc);
 		if (ctxt != NULL)
@@ -1957,9 +1972,6 @@ fail:
 	}
 	PG_END_TRY();
 
-	if (save_keep_blanks != -1)
-		xmlKeepBlanksDefault(save_keep_blanks);
-
 	if (ctxt != NULL)
 		xmlFreeParserCtxt(ctxt);
 
@@ -4648,7 +4660,7 @@ wellformed_xml(text *data, XmlOptionType xmloption_arg)
 	 * We'll report "true" if no soft error is reported by xml_parse().
 	 */
 	doc = xml_parse(data, xmloption_arg, true,
-					GetDatabaseEncoding(), NULL, NULL, (Node *) &escontext);
+					GetDatabaseEncoding(), NULL, (Node *) &escontext);
 	if (doc)
 		xmlFreeDoc(doc);
 

src/test/regress/expected/xml.out

@@ -682,6 +682,35 @@ SELECT xmlserialize(CONTENT  'text node<foo>    <bar></bar>   </foo>' AS text IN
  </foo>
 (1 row)
 
+-- check some encoding-dependent behaviors
+-- (this is mostly checking for libxml2 bugs, not our bugs...)
+DO $$
+DECLARE
+  result text;
+BEGIN
+  IF getdatabaseencoding() = 'UTF8' THEN
+    -- U+FEFF after an XML declaration is content, not a byte-order mark
+    result := xmlserialize(CONTENT xmlparse(CONTENT
+              '<?xml version="1.0"?>' || chr(65279) || 'abc') AS text INDENT);
+    IF result <> '&#xFEFF;abc' THEN
+      RAISE WARNING 'U+FEFF was not preserved: %', result;
+    END IF;
+    -- non-ASCII characters are converted to hex entities if in data
+    result := xmlserialize(CONTENT xmlparse(CONTENT
+              'ni' || chr(241) || 'o') AS text INDENT);
+    IF result <> 'ni&#xF1;o' THEN
+      RAISE WARNING 'U+F1 was not converted to a hex entity: %', result;
+    END IF;
+    -- non-ASCII characters are preserved in tag attributes
+    result := xmlserialize(CONTENT xmlparse(CONTENT
+              '<a attr="ni' || chr(241) || 'o"/>') AS text INDENT);
+    IF result <> '<a attr="ni' || chr(241) || 'o"/>' THEN
+      RAISE WARNING 'U+F1 was not preserved in a tag: %', result;
+    END IF;
+  END IF;
+END
+$$;
+-- IS DOCUMENT
 SELECT xml '<foo>bar</foo>' IS DOCUMENT;
  ?column? 
 ----------

src/test/regress/sql/xml.sql

@@ -179,6 +179,36 @@ SELECT xmlserialize(CONTENT  '<foo><bar><val x="y">42</val></bar></foo>' AS text
 SELECT xmlserialize(DOCUMENT '<foo>   <bar></bar>    </foo>' AS text INDENT);
 SELECT xmlserialize(CONTENT  'text node<foo>    <bar></bar>   </foo>' AS text INDENT);
 
+-- check some encoding-dependent behaviors
+-- (this is mostly checking for libxml2 bugs, not our bugs...)
+DO $$
+DECLARE
+  result text;
+BEGIN
+  IF getdatabaseencoding() = 'UTF8' THEN
+    -- U+FEFF after an XML declaration is content, not a byte-order mark
+    result := xmlserialize(CONTENT xmlparse(CONTENT
+              '<?xml version="1.0"?>' || chr(65279) || 'abc') AS text INDENT);
+    IF result <> '&#xFEFF;abc' THEN
+      RAISE WARNING 'U+FEFF was not preserved: %', result;
+    END IF;
+    -- non-ASCII characters are converted to hex entities if in data
+    result := xmlserialize(CONTENT xmlparse(CONTENT
+              'ni' || chr(241) || 'o') AS text INDENT);
+    IF result <> 'ni&#xF1;o' THEN
+      RAISE WARNING 'U+F1 was not converted to a hex entity: %', result;
+    END IF;
+    -- non-ASCII characters are preserved in tag attributes
+    result := xmlserialize(CONTENT xmlparse(CONTENT
+              '<a attr="ni' || chr(241) || 'o"/>') AS text INDENT);
+    IF result <> '<a attr="ni' || chr(241) || 'o"/>' THEN
+      RAISE WARNING 'U+F1 was not preserved in a tag: %', result;
+    END IF;
+  END IF;
+END
+$$;
+
+-- IS DOCUMENT
 SELECT xml '<foo>bar</foo>' IS DOCUMENT;
 SELECT xml '<foo>bar</foo><bar>foo</bar>' IS DOCUMENT;
 SELECT xml '<abc/>' IS NOT DOCUMENT;