On 28.07.25 22:16, Tom Lane wrote:
> Erik's v2 is slightly wrong as to the save-and-restore logic for
> the KeepBlanks setting: we need to restore in the error path too,
> and we'd better mark the save variable volatile since it's modified
> inside the PG_TRY.  I made some other cosmetic changes, mainly to
> avoid calculating "options" when it won't be used.  I tested the
> attached v3 against RHEL8's libxml2-2.9.7, as well as against today's
> libxml2 git master, and it accepts the problematic input on both.

Out of curiosity, what's the reasoning behind keeping node_list instead
of directly using parsed_nodes in the xmlParseBalancedChunkMemory call?

Example:

if (*(utf8string + count))
{
    res_code = xmlParseBalancedChunkMemory(doc, NULL, NULL, 0,
                                           utf8string + count,
                                           parsed_nodes);
    if (res_code != 0 || xmlerrcxt->err_occurred)
    {
        xml_errsave(escontext, xmlerrcxt,
                    ERRCODE_INVALID_XML_CONTENT,
                    "invalid XML content");
        goto fail;
    }
}

I was also wondering if we should add to PG 19 a GUC to enable
XML_MAX_HUGE_LENGTH if so needed. If we go down that route, we'd likely
need to revisit xmlParseBalancedChunkMemory (again!) since it appears to
be hardcoded to XML_MAX_TEXT_LENGTH. Any thoughts?

Best regards, Jim
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 3379d39226..182e8f75db 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -1922,24 +1922,16 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 			/* allow empty content */
 			if (*(utf8string + count))
 			{
-				xmlNodePtr	node_list = NULL;
-
 				res_code = xmlParseBalancedChunkMemory(doc, NULL, NULL, 0,
 													   utf8string + count,
-													   &node_list);
+													   parsed_nodes);
 				if (res_code != 0 || xmlerrcxt->err_occurred)
 				{
 					xml_errsave(escontext, xmlerrcxt,
 								ERRCODE_INVALID_XML_CONTENT,
 								"invalid XML content");
-					xmlFreeNodeList(node_list);
 					goto fail;
 				}
-
-				if (parsed_nodes != NULL)
-					*parsed_nodes = node_list;
-				else
-					xmlFreeNodeList(node_list);
 			}
 		}
 

Reply via email to