On 2019-Mar-08, Alvaro Herrera wrote:
> > Maybe we can call explicitly xmlFreeDoc instead xmlFreeNode
> >
> > some like
> >
> > if (cur_copy->type == XML_DOCUMENT_NODE)
> > xmlFreeDoc((xmlDocPtr) cur_copy);
> > else
> > xmlFreeNode(cur_copy);
> >
> > This looks most correct fix for me. What do you think?
>
> Seems like that should work, yeah ...
Something like this perhaps? Less repetitive ...
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 28b3eaaa201..9204d6e9cf6 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -3720,35 +3720,57 @@ xml_xmlnodetoxmltype(xmlNodePtr cur, PgXmlErrorContext *xmlerrcxt)
if (cur->type != XML_ATTRIBUTE_NODE && cur->type != XML_TEXT_NODE)
{
- xmlBufferPtr buf;
- xmlNodePtr cur_copy;
-
- buf = xmlBufferCreate();
-
- /*
- * The result of xmlNodeDump() won't contain namespace definitions
- * from parent nodes, but xmlCopyNode() duplicates a node along with
- * its required namespace definitions.
- */
- cur_copy = xmlCopyNode(cur, 1);
-
- if (cur_copy == NULL)
- xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
- "could not copy node");
+ volatile void (*nodefree) (xmlNodePtr) = NULL;
+ volatile xmlBufferPtr buf = NULL;
+ volatile xmlNodePtr cur_copy = NULL;
PG_TRY();
{
- xmlNodeDump(buf, NULL, cur_copy, 0, 1);
+ int bytes;
+
+ buf = xmlBufferCreate();
+ if (buf == NULL || xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not allocate xmlBuffer");
+
+ /*
+ * Produce a dump of the node that we can serialize. xmlNodeDump
+ * does that, but the result of that function won't contain
+ * namespace definitions from ancestor nodes, so we first do a
+ * xmlCopyNode() which duplicates the node along with its required
+ * namespace definitions.
+ *
+ * Some old libxml2 versions such as 2.7.6 produce partially
+ * broken XML_DOCUMENT_NODE nodes (unset content field) when
+ * copying them. xmlNodeDump of such a node works fine, but
+ * xmlFreeNode crashes; set us up to call xmlFreeDoc instead.
+ */
+ cur_copy = xmlCopyNode(cur, 1);
+ if (cur_copy == NULL || xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not copy node");
+ nodefree = cur_copy->type == XML_DOCUMENT_NODE ?
+ (void (*) (xmlNodePtr)) xmlFreeDoc : xmlFreeNode;
+
+ bytes = xmlNodeDump(buf, NULL, cur_copy, 0, 1);
+ if (bytes == -1 || xmlerrcxt->err_occurred)
+ xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
+ "could not dump node");
+
result = xmlBuffer_to_xmltype(buf);
}
PG_CATCH();
{
- xmlFreeNode(cur_copy);
- xmlBufferFree(buf);
+ if (nodefree)
+ nodefree(cur_copy);
+ if (buf)
+ xmlBufferFree(buf);
PG_RE_THROW();
}
PG_END_TRY();
- xmlFreeNode(cur_copy);
+
+ if (nodefree)
+ nodefree(cur_copy);
xmlBufferFree(buf);
}
else
