Ensure that xpath() escapes special characters in string values.

Without this it's possible for the output to not be legal XML, as
illustrated by the added regression test cases.

NB: this change will need to be called out as an incompatibility in the
9.2 release notes, since it's possible somebody was relying on the old
behavior, even though it's clearly wrong.

Florian Pflug, reviewed by Radoslaw Smogura

4 files changed, 31 insertions, 1 deletions

diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 6786cd91bb5..c07232575e2 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -3537,7 +3537,11 @@ xml_xmlnodetoxmltype(xmlNodePtr cur)
 		str = xmlXPathCastNodeToString(cur);
 		PG_TRY();
 		{
-			result = (xmltype *) cstring_to_text((char *) str);
+			/* Here we rely on XML having the same representation as TEXT */
+			char   *escaped = escape_xml((char *) str);
+
+			result = (xmltype *) cstring_to_text(escaped);
+			pfree(escaped);
 		}
 		PG_CATCH();
 		{
diff --git a/src/test/regress/expected/xml.out b/src/test/regress/expected/xml.out
index 379777aced8..5cd602107b2 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -589,6 +589,18 @@ SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>');
  {<b>two</b>,<b>etc</b>}
 (1 row)
 
+SELECT xpath('//text()', '<root>&lt;</root>');
+ xpath  
+--------
+ {&lt;}
+(1 row)
+
+SELECT xpath('//@value', '<root value="&lt;"/>');
+ xpath  
+--------
+ {&lt;}
+(1 row)
+
 -- Test xmlexists and xpath_exists
 SELECT xmlexists('//town[text() = ''Toronto'']' PASSING BY REF '<towns><town>Bidford-on-Avon</town><town>Cwmbran</town><town>Bristol</town></towns>');
  xmlexists 
diff --git a/src/test/regress/expected/xml_1.out b/src/test/regress/expected/xml_1.out
index 1f17bffc0b2..53675f5536f 100644
--- a/src/test/regress/expected/xml_1.out
+++ b/src/test/regress/expected/xml_1.out
@@ -504,6 +504,18 @@ LINE 1: SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>'...
                             ^
 DETAIL:  This functionality requires the server to be built with libxml support.
 HINT:  You need to rebuild PostgreSQL using --with-libxml.
+SELECT xpath('//text()', '<root>&lt;</root>');
+ERROR:  unsupported XML feature
+LINE 1: SELECT xpath('//text()', '<root>&lt;</root>');
+                                 ^
+DETAIL:  This functionality requires the server to be built with libxml support.
+HINT:  You need to rebuild PostgreSQL using --with-libxml.
+SELECT xpath('//@value', '<root value="&lt;"/>');
+ERROR:  unsupported XML feature
+LINE 1: SELECT xpath('//@value', '<root value="&lt;"/>');
+                                 ^
+DETAIL:  This functionality requires the server to be built with libxml support.
+HINT:  You need to rebuild PostgreSQL using --with-libxml.
 -- Test xmlexists and xpath_exists
 SELECT xmlexists('//town[text() = ''Toronto'']' PASSING BY REF '<towns><town>Bidford-on-Avon</town><town>Cwmbran</town><town>Bristol</town></towns>');
 ERROR:  unsupported XML feature
diff --git a/src/test/regress/sql/xml.sql b/src/test/regress/sql/xml.sql
index f4e423618ec..3270e157214 100644
--- a/src/test/regress/sql/xml.sql
+++ b/src/test/regress/sql/xml.sql
@@ -175,6 +175,8 @@ SELECT xpath('', '<!-- error -->');
 SELECT xpath('//text()', '<local:data xmlns:local="http://127.0.0.1"><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>');
 SELECT xpath('//loc:piece/@id', '<local:data xmlns:local="http://127.0.0.1"><local:piece id="1">number one</local:piece><local:piece id="2" /></local:data>', ARRAY[ARRAY['loc', 'http://127.0.0.1']]);
 SELECT xpath('//b', '<a>one <b>two</b> three <b>etc</b></a>');
+SELECT xpath('//text()', '<root>&lt;</root>');
+SELECT xpath('//@value', '<root value="&lt;"/>');
 
 -- Test xmlexists and xpath_exists
 SELECT xmlexists('//town[text() = ''Toronto'']' PASSING BY REF '<towns><town>Bidford-on-Avon</town><town>Cwmbran</town><town>Bristol</town></towns>');