Hi

While investigating a libxml2-related crash I noticed that xmlroot()
does not validate VERSION,so it can store xml values whose declaration
xml_in() rejects, e.g. a version containing a quote or a non-ASCII
character. xmlconcat(),xmlagg() and xmlroot() then silently drop the
content of such values:

== master ==

postgres=# create table t (x xml);
CREATE TABLE
postgres=# insert into t values (xmlroot('<doc>data</doc>'::xml, version
'1.0"x'));
INSERT 0 1
postgres=# select  xmlconcat('<a/>'::xml, x) from t;
 xmlconcat
-----------
 <a/>
(1 row)

Such values also cannot be restored from a dump, since xml_in() rejects
what xml_out() returns:

postgres=# select xml_in(xml_out(x)) from t;
WARNING:  could not parse XML declaration in stored value
DETAIL:  Parsing XML declaration: '?>' expected.
ERROR:  invalid XML content: invalid XML declaration
DETAIL:  Parsing XML declaration: '?>' expected.

v1 attached addresses these issues:

0001 makes xmlconcat() and xmlroot() raise an error for such values,
with the message xml_out() already warns with, instead of dropping their
content. Queries that currently return a wrong result for them will fail
instead, which IMHO is better. Affected rows can be found with ..

postgres=# select * from t where not xml_is_well_formed_content(x::text);
WARNING:  could not parse XML declaration in stored value
DETAIL:  Parsing XML declaration: '?>' expected.
                   x
----------------------------------------
 <?xml version="1.0"x"?><doc>data</doc>
(1 row)

0002 makes xmlroot() reject a VERSION that cannot be read back unchanged
from the declaration it builds, so no new values like this can be
created. It does not otherwise check whether the version is valid. Since
0001's tests can then no longer create a bad value, 0002 replaces them.

With the patches applied, xmlroot() rejects such a version...

postgres=# select xmlroot('<doc>data</doc>'::xml, version '1.0"x');
ERROR:  invalid XML version "1.0"x"

Thoughts?

Best, Jim
From 120495773349fc4fb0bca48e4b03ea22fc5777c7 Mon Sep 17 00:00:00 2001
From: Jim Jones <[email protected]>
Date: Sun, 8 Nov 2026 17:50:31 +0100
Subject: [PATCH v1 1/2] Raise an error for invalid XML declarations in
 xmlconcat() and xmlroot()

Both functions strip the XML declaration of their input using
parse_xml_decl(), but ignored its result.  When the declaration could
not be parsed, the declaration length was left at the full length of
the value, so its content was silently dropped.  This also affected
xmlagg(), which uses xmlconcat().

Such values could be created by xmlroot() with an unusual VERSION, e.g.
'1.0"x'.  Raise the same error that xml_out() already reports as a
warning for them, instead of returning a wrong result.
---
 src/backend/utils/adt/xml.c       | 18 ++++++++++++++++--
 src/test/regress/expected/xml.out | 10 ++++++++++
 src/test/regress/sql/xml.sql      |  4 ++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 44a3a21d135..b6e7e367723 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -588,11 +588,18 @@ xmlconcat(List *args)
 		xmlChar    *version;
 		int			standalone;
 		char	   *str;
+		int			res_code;
 
 		len = VARSIZE(x) - VARHDRSZ;
 		str = text_to_cstring((text *) x);
 
-		parse_xml_decl((xmlChar *) str, &len, &version, NULL, &standalone);
+		res_code = parse_xml_decl((xmlChar *) str, &len, &version, NULL,
+								  &standalone);
+		if (res_code != 0)
+			ereport(ERROR,
+					errcode(ERRCODE_DATA_CORRUPTED),
+					errmsg_internal("could not parse XML declaration in stored value"),
+					errdetail_for_xml_code(res_code));
 
 		if (standalone == 0 && global_standalone == 1)
 			global_standalone = 0;
@@ -1091,11 +1098,18 @@ xmlroot(xmltype *data, text *version, int standalone)
 	xmlChar    *orig_version;
 	int			orig_standalone;
 	StringInfoData buf;
+	int			res_code;
 
 	len = VARSIZE(data) - VARHDRSZ;
 	str = text_to_cstring((text *) data);
 
-	parse_xml_decl((xmlChar *) str, &len, &orig_version, NULL, &orig_standalone);
+	res_code = parse_xml_decl((xmlChar *) str, &len, &orig_version, NULL,
+							  &orig_standalone);
+	if (res_code != 0)
+		ereport(ERROR,
+				errcode(ERRCODE_DATA_CORRUPTED),
+				errmsg_internal("could not parse XML declaration in stored value"),
+				errdetail_for_xml_code(res_code));
 
 	if (version)
 		orig_version = xml_text2xmlChar(version);
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 6998745ee03..0a97c5cbe03 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -448,6 +448,16 @@ SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value)
  <?xml version="1.0" standalone="yes"?><foo/>
 (1 row)
 
+-- values with an XML declaration that cannot be parsed raise an error
+SELECT xmlconcat(xml '<foo/>', xmlroot(xml '<bar/>', version '1.0"x'));
+ERROR:  could not parse XML declaration in stored value
+DETAIL:  Parsing XML declaration: '?>' expected.
+SELECT xmlagg(x) FROM (VALUES (xml '<foo/>'), (xmlroot(xml '<bar/>', version '1.0"x'))) v(x);
+ERROR:  could not parse XML declaration in stored value
+DETAIL:  Parsing XML declaration: '?>' expected.
+SELECT xmlroot(xmlroot(xml '<bar/>', version '1.0"x'), version '1.0');
+ERROR:  could not parse XML declaration in stored value
+DETAIL:  Parsing XML declaration: '?>' expected.
 SELECT xmlroot (
   xmlelement (
     name gazonk,
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index 0581d21b03f..66c2c93799e 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -116,6 +116,10 @@ SELECT xmlroot(xmlroot(xml '<foo/>', version '1.0'), version '1.1', standalone n
 SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value, standalone no);
 SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value, standalone no value);
 SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value);
+-- values with an XML declaration that cannot be parsed raise an error
+SELECT xmlconcat(xml '<foo/>', xmlroot(xml '<bar/>', version '1.0"x'));
+SELECT xmlagg(x) FROM (VALUES (xml '<foo/>'), (xmlroot(xml '<bar/>', version '1.0"x'))) v(x);
+SELECT xmlroot(xmlroot(xml '<bar/>', version '1.0"x'), version '1.0');
 
 
 SELECT xmlroot (
-- 
2.56.0

From d17a223a9533b6ef353c982b0cb1c89dd43c50bb Mon Sep 17 00:00:00 2001
From: Jim Jones <[email protected]>
Date: Sun, 8 Nov 2026 18:38:22 +0100
Subject: [PATCH v1 2/2] Reject VERSION values in xmlroot() that break the XML
 declaration

xmlroot() copied VERSION into the XML declaration without checking it.
A version containing a quote or a non-ASCII character, e.g. '1.0"x',
produced a declaration that cannot be parsed, so the value could not be
restored from a dump.

Raise an error unless the version can be read back unchanged from the
declaration that was built.  The version is not otherwise checked for
validity.
---
 src/backend/utils/adt/xml.c       | 14 +++++++++++++-
 src/test/regress/expected/xml.out | 14 ++++----------
 src/test/regress/sql/xml.sql      |  6 ++----
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index b6e7e367723..1b58f56edd0 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -1133,7 +1133,19 @@ xmlroot(xmltype *data, text *version, int standalone)
 	}
 
 	initStringInfo(&buf);
-	print_xml_decl(&buf, orig_version, 0, orig_standalone);
+	if (print_xml_decl(&buf, orig_version, 0, orig_standalone) && orig_version)
+	{
+		xmlChar    *parsed_version;
+
+		/* Reject a version that the XML declaration cannot represent */
+		if (parse_xml_decl((xmlChar *) buf.data, NULL, &parsed_version,
+						   NULL, NULL) != XML_ERR_OK ||
+			xmlStrcmp(parsed_version, orig_version) != 0)
+			ereport(ERROR,
+					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+					 errmsg("invalid XML version \"%s\"",
+							(char *) orig_version)));
+	}
 	appendStringInfoString(&buf, str + len);
 
 	return stringinfo_to_xmltype(&buf);
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 0a97c5cbe03..83b4775938c 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -448,16 +448,10 @@ SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value)
  <?xml version="1.0" standalone="yes"?><foo/>
 (1 row)
 
--- values with an XML declaration that cannot be parsed raise an error
-SELECT xmlconcat(xml '<foo/>', xmlroot(xml '<bar/>', version '1.0"x'));
-ERROR:  could not parse XML declaration in stored value
-DETAIL:  Parsing XML declaration: '?>' expected.
-SELECT xmlagg(x) FROM (VALUES (xml '<foo/>'), (xmlroot(xml '<bar/>', version '1.0"x'))) v(x);
-ERROR:  could not parse XML declaration in stored value
-DETAIL:  Parsing XML declaration: '?>' expected.
-SELECT xmlroot(xmlroot(xml '<bar/>', version '1.0"x'), version '1.0');
-ERROR:  could not parse XML declaration in stored value
-DETAIL:  Parsing XML declaration: '?>' expected.
+SELECT xmlroot(xml '<foo/>', version '1.0"x');
+ERROR:  invalid XML version "1.0"x"
+SELECT xmlroot(xml '<foo/>', version '1.0" standalone="yes');
+ERROR:  invalid XML version "1.0" standalone="yes"
 SELECT xmlroot (
   xmlelement (
     name gazonk,
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index 66c2c93799e..88ddfa659ed 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -116,10 +116,8 @@ SELECT xmlroot(xmlroot(xml '<foo/>', version '1.0'), version '1.1', standalone n
 SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value, standalone no);
 SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value, standalone no value);
 SELECT xmlroot('<?xml version="1.1" standalone="yes"?><foo/>', version no value);
--- values with an XML declaration that cannot be parsed raise an error
-SELECT xmlconcat(xml '<foo/>', xmlroot(xml '<bar/>', version '1.0"x'));
-SELECT xmlagg(x) FROM (VALUES (xml '<foo/>'), (xmlroot(xml '<bar/>', version '1.0"x'))) v(x);
-SELECT xmlroot(xmlroot(xml '<bar/>', version '1.0"x'), version '1.0');
+SELECT xmlroot(xml '<foo/>', version '1.0"x');
+SELECT xmlroot(xml '<foo/>', version '1.0" standalone="yes');
 
 
 SELECT xmlroot (
-- 
2.56.0

Reply via email to