Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 24 Jul 2025 12:12:28 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Erik Wienhold <ewie@ewie.name>
Subject: Regression with large XML data input
Message-ID: <aIGknLuc8b8ega2X@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Q/jGJq3HXFkGRKAv"
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/aIGknLuc8b8ega2X%40paquier.xyz>
Precedence: bulk


--Q/jGJq3HXFkGRKAv
Content-Type: multipart/mixed; boundary="klYMml4gv14Oa3H8"
Content-Disposition: inline


--klYMml4gv14Oa3H8
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hi all,
(Adding in CC Tom and Eric, as committer and  author.)

A customer has reported a regression with the parsing of rather large
XML data, introduced by the set of backpatches done with f68d6aabb7e2
& friends.

The problem is introduced by the change from
xmlParseBalancedChunkMemory() to xmlNewNode() +
xmlParseInNodeContext() in xml_parse(), to avoid an issue in
xmlParseBalancedChunkMemory() in the range of libxml2 2.13.0-2.13.2
for a bug that has already been fixed upstream, where we use a
temporary root node for the case where parse_as_document is false.

If the input XML data is large enough, one gets a failure at the top
of the latest branches, and it worked properly before.  Here is a
short test case (courtesy of a colleague, case that I've modified
slightly):
CREATE TABLE xmldata (id BIGINT PRIMARY KEY, message XML );
DO $$ DECLARE size_40mb TEXT := repeat('X', 40000000);
BEGIN
 BEGIN
   INSERT INTO xmldata (id, message) VALUES
     ( 1, (('<Root><Item><Name>Test40MB</Name><Content>' || size_40mb || '</Content></Item></Root>')::xml) );
   RAISE NOTICE 'insert 40MB successful';
   EXCEPTION WHEN OTHERS THEN RAISE NOTICE 'Error insert 40MB: %', SQLERRM;
 END;
END $$;

Switching back to the previous code, where we rely on
xmlParseBalancedChunkMemory() fixes the issue.  A quick POC is
attached.  It fails one case in check-world with SERIALIZE because I
am not sure it is possible to pass down some options through
xmlParseBalancedChunkMemory(), still the regression is gone, and I am
wondering if there is not a better solution to be able to dodge the
original problem and still accept this case.  One good thing is that
xmlParseBalancedChunkMemory() is able to return a list of nodes, that
we need for this code path of xml_parse().  So perhaps one solution
would be the addition of a code path with
xmlParseBalancedChunkMemory() depending on the options we want to
process, keeping the code path with the fake "content-root" for the
XML SERIALIZE case.

The patch in question has been applied first to 6082b3d5d3d1 on HEAD
impacting v18~, then it has been backpatched down to all stable
branches, like f68d6aabb7e2, introducing the regression in all the
stable branches since the minor releases done in August 2024, as of:
12.20, 13.16, 14.13, 15.8, 16.4.

Thoughts or comments?
--
Michael

--klYMml4gv14Oa3H8
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0001-Fix-xml2-regression.patch"
Content-Transfer-Encoding: quoted-printable

=46rom 1834a83ec4e013fcc762e07372ca3fa365e2541e Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Thu, 24 Jul 2025 12:01:52 +0900
Subject: [PATCH] Fix xml2 regression

---
 src/backend/utils/adt/xml.c | 27 ++++-----------------------
 1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index f7b731825fca..3a44794a2393 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -1900,9 +1900,6 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 		}
 		else
 		{
-			xmlNodePtr	root;
-			xmlNodePtr	oldroot PG_USED_FOR_ASSERTS_ONLY;
-
 			/* set up document with empty root node to be the context node */
 			doc =3D xmlNewDoc(version);
 			if (doc =3D=3D NULL || xmlerrcxt->err_occurred)
@@ -1916,31 +1913,15 @@ xml_parse(text *data, XmlOptionType xmloption_arg,
 							"could not allocate XML document");
 			doc->standalone =3D standalone;
=20
-			root =3D xmlNewNode(NULL, (const xmlChar *) "content-root");
-			if (root =3D=3D 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;
-			 * and there can't yet be any old root to free.
-			 */
-			oldroot =3D xmlDocSetRootElement(doc, root);
-			Assert(oldroot =3D=3D NULL);
-
 			/* allow empty content */
 			if (*(utf8string + count))
 			{
 				xmlNodePtr	node_list =3D NULL;
-				xmlParserErrors res;
=20
-				res =3D xmlParseInNodeContext(root,
-											(char *) utf8string + count,
-											strlen((char *) utf8string + count),
-											options,
-											&node_list);
-
-				if (res !=3D XML_ERR_OK || xmlerrcxt->err_occurred)
+				res_code =3D xmlParseBalancedChunkMemory(doc, NULL, NULL, 0,
+													   utf8string + count,
+													   &node_list);
+				if (res_code !=3D 0 || xmlerrcxt->err_occurred)
 				{
 					xmlFreeNodeList(node_list);
 					xml_errsave(escontext, xmlerrcxt,
--=20
2.50.0


--klYMml4gv14Oa3H8--

--Q/jGJq3HXFkGRKAv
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmiBpJwACgkQnvQgOdby
QH0hQQ//fQEkWXMNWuh1VdrtNkRcJ2fMpZ5VbO9dzhlmHvZUnGokXWwevOnkKx+X
srX2XYkMV93j+aJLx+jK1RGGYPzJWeTDJ+I8Zilk/eetEVQL0ZfWBnlP8jJlO2td
JNwnpSK23Gl3nJIFY4fLcvM3Bqj+bccbq7eLrBOSX5lksE4nkfRU3Cr9coSrIjXp
cbhPhfHjsvtkRoiaBE0P8l0NkXveLw0HV+4wK17f188+FpFJ8KNwsrlklYwtizjp
ipbpxyUjjVqpO2oitW2JpZTjQI1C7GKGmJm739UMY0u6cJSFLrNzHwhkytoG5s9F
jowp2P7gwFlR/n1f8U67uRVIUDGbJTLtcn20Qy+9sa3u9KAzQEwEMSe/UtOfxVoz
k77I83LoYYJMqxt8BcRhL8uTTWPxs7wfTib38cYvYDpBwRIj/FAAu+/pS7K2XSqv
wxQJN9l98r5Hv96lZ3QP0jbosD/bA+6apmZhk/+PWv71OSRG/NJBHH9TSQziGyLn
FBBAC40RhHZT0eQe0eDayvUCm8iFd9LXAgapWtWgwNKUBm63iuL21jslytkjuAra
B7FLjr6ZKp5bhLWqA4HPT1ycFPIz5qU6FQcDkeXyDfC5etDSLBIHAdpy9GnZmriD
u9Hne52zcL7GYZ+dqnWZyliLDRi00lbfUYDKYoBM0vlENNOquSU=
=eF2r
-----END PGP SIGNATURE-----

--Q/jGJq3HXFkGRKAv--