BUG #4822: xmlattributes encodes '&' twice
The following bug has been logged online:
Bug reference: 4822
Logged by: Itagaki Takahiro
Email address: itagaki.takahiro@oss.ntt.co.jp
PostgreSQL version: 8.4dev
Operating system: Linux, Windows
Description: xmlattributes encodes '&' twice
Details:
=# SELECT xmlelement(name a, xmlattributes('./qa?a=1&b=2' as href), 'Q&A');
xmlelement
--------------------------------------------
<a href="./qa?a=1&amp;b=2">Q&A</a>
(1 row)
'&' in xmlattributes seems to be encoded twice.
( '&' => '&' => '&amp;' )
The bug only exists in 8.4dev;
PostgreSQL 8.3 correctly encodes '&' only once.
"Itagaki Takahiro" <itagaki.takahiro@oss.ntt.co.jp> writes:
=# SELECT xmlelement(name a, xmlattributes('./qa?a=1&b=2' as href), 'Q&A');
xmlelement
--------------------------------------------
<a href="./qa?a=1&amp;b=2">Q&A</a>
(1 row)
'&' in xmlattributes seems to be encoded twice.
This was apparently broken by Peter's patch here:
http://archives.postgresql.org/pgsql-committers/2009-04/msg00124.php
map_sql_value_to_xml_value() performs mapping of & (and various other
special characters), and evidently xmlTextWriterWriteAttribute() does
it again.
I'm not sure about the most appropriate solution. The libxml2
documentation is so awful that it doesn't even tell you that
xmlTextWriterWriteAttribute does that, let alone suggest whether there
is another API function that doesn't. We might have to add a bool flag
to map_sql_value_to_xml_value() to enable or disable mapping of special
characters.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote:
=# SELECT xmlelement(name a, xmlattributes('./qa?a=1&b=2' as href), 'Q&A');
xmlelement
--------------------------------------------
<a href="./qa?a=1&amp;b=2">Q&A</a>'&' in xmlattributes seems to be encoded twice.
This was apparently broken by Peter's patch here:
http://archives.postgresql.org/pgsql-committers/2009-04/msg00124.phpWe might have to add a bool flag
to map_sql_value_to_xml_value() to enable or disable mapping of special
characters.
Here is a patch to fix the bug. I added a parameter 'encode' to
map_sql_value_to_xml_value() and pass false for xml attributes.
char *
map_sql_value_to_xml_value(Datum value, Oid type, bool encode)
Also a special regression test is added for it:
SELECT xmlelement(name element,
xmlattributes (1 as one, 'deuce' as two, '<>&"''' as three),
'content', '<>&"''');
xmlelement
--------------------------------------------------------------------------------------------
<element one="1" two="deuce" three="<>&"'">content<>&"'</element>
(1 row)
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Attachments:
not-encode-xmlattributes.patchapplication/octet-stream; name=not-encode-xmlattributes.patchDownload
diff -cpr head/src/backend/executor/execQual.c not-encode-xmlattributes/src/backend/executor/execQual.c
*** head/src/backend/executor/execQual.c Fri Apr 10 09:40:51 2009
--- not-encode-xmlattributes/src/backend/executor/execQual.c Thu May 28 19:26:47 2009
*************** ExecEvalXml(XmlExprState *xmlExpr, ExprC
*** 3242,3248 ****
{
appendStringInfo(&buf, "<%s>%s</%s>",
argname,
! map_sql_value_to_xml_value(value, exprType((Node *) e->expr)),
argname);
*isNull = false;
}
--- 3242,3248 ----
{
appendStringInfo(&buf, "<%s>%s</%s>",
argname,
! map_sql_value_to_xml_value(value, exprType((Node *) e->expr), true),
argname);
*isNull = false;
}
diff -cpr head/src/backend/utils/adt/xml.c not-encode-xmlattributes/src/backend/utils/adt/xml.c
*** head/src/backend/utils/adt/xml.c Mon May 18 12:23:11 2009
--- not-encode-xmlattributes/src/backend/utils/adt/xml.c Thu May 28 19:26:47 2009
*************** xmlelement(XmlExprState *xmlExpr, ExprCo
*** 569,575 ****
if (isnull)
str = NULL;
else
! str = map_sql_value_to_xml_value(value, exprType((Node *) e->expr));
named_arg_strings = lappend(named_arg_strings, str);
i++;
}
--- 569,575 ----
if (isnull)
str = NULL;
else
! str = map_sql_value_to_xml_value(value, exprType((Node *) e->expr), false);
named_arg_strings = lappend(named_arg_strings, str);
i++;
}
*************** xmlelement(XmlExprState *xmlExpr, ExprCo
*** 587,593 ****
if (!isnull)
{
str = map_sql_value_to_xml_value(value,
! exprType((Node *) e->expr));
arg_strings = lappend(arg_strings, str);
}
}
--- 587,593 ----
if (!isnull)
{
str = map_sql_value_to_xml_value(value,
! exprType((Node *) e->expr), true);
arg_strings = lappend(arg_strings, str);
}
}
*************** map_xml_name_to_sql_identifier(char *nam
*** 1582,1588 ****
* Map SQL value to XML value; see SQL/XML:2003 section 9.16.
*/
char *
! map_sql_value_to_xml_value(Datum value, Oid type)
{
StringInfoData buf;
--- 1582,1588 ----
* Map SQL value to XML value; see SQL/XML:2003 section 9.16.
*/
char *
! map_sql_value_to_xml_value(Datum value, Oid type, bool encode)
{
StringInfoData buf;
*************** map_sql_value_to_xml_value(Datum value,
*** 1616,1622 ****
appendStringInfoString(&buf, "<element>");
appendStringInfoString(&buf,
map_sql_value_to_xml_value(elem_values[i],
! elmtype));
appendStringInfoString(&buf, "</element>");
}
--- 1616,1622 ----
appendStringInfoString(&buf, "<element>");
appendStringInfoString(&buf,
map_sql_value_to_xml_value(elem_values[i],
! elmtype, true));
appendStringInfoString(&buf, "</element>");
}
*************** map_sql_value_to_xml_value(Datum value,
*** 1774,1781 ****
getTypeOutputInfo(type, &typeOut, &isvarlena);
str = OidOutputFunctionCall(typeOut, value);
! /* ... exactly as-is for XML */
! if (type == XMLOID)
return str;
/* otherwise, translate special characters as needed */
--- 1774,1781 ----
getTypeOutputInfo(type, &typeOut, &isvarlena);
str = OidOutputFunctionCall(typeOut, value);
! /* ... exactly as-is for XML or encode is not required */
! if (type == XMLOID || !encode)
return str;
/* otherwise, translate special characters as needed */
*************** SPI_sql_row_to_xmlelement(int rownum, St
*** 3179,3185 ****
appendStringInfo(result, " <%s>%s</%s>\n",
colname,
map_sql_value_to_xml_value(colval,
! SPI_gettypeid(SPI_tuptable->tupdesc, i)),
colname);
}
--- 3179,3185 ----
appendStringInfo(result, " <%s>%s</%s>\n",
colname,
map_sql_value_to_xml_value(colval,
! SPI_gettypeid(SPI_tuptable->tupdesc, i), true),
colname);
}
diff -cpr head/src/include/utils/xml.h not-encode-xmlattributes/src/include/utils/xml.h
*** head/src/include/utils/xml.h Mon May 18 12:23:11 2009
--- not-encode-xmlattributes/src/include/utils/xml.h Thu May 28 19:26:47 2009
*************** extern text *xmltotext_with_xmloption(xm
*** 73,79 ****
extern char *map_sql_identifier_to_xml_name(char *ident, bool fully_escaped, bool escape_period);
extern char *map_xml_name_to_sql_identifier(char *name);
! extern char *map_sql_value_to_xml_value(Datum value, Oid type);
typedef enum
{
--- 73,79 ----
extern char *map_sql_identifier_to_xml_name(char *ident, bool fully_escaped, bool escape_period);
extern char *map_xml_name_to_sql_identifier(char *name);
! extern char *map_sql_value_to_xml_value(Datum value, Oid type, bool encode);
typedef enum
{
diff -cpr head/src/test/regress/expected/xml.out not-encode-xmlattributes/src/test/regress/expected/xml.out
*** head/src/test/regress/expected/xml.out Fri Apr 10 09:40:51 2009
--- not-encode-xmlattributes/src/test/regress/expected/xml.out Thu May 28 19:26:47 2009
*************** SELECT xmlconcat(NULL, NULL);
*** 90,100 ****
(1 row)
SELECT xmlelement(name element,
! xmlattributes (1 as one, 'deuce' as two),
! 'content');
! xmlelement
! ------------------------------------------------
! <element one="1" two="deuce">content</element>
(1 row)
SELECT xmlelement(name element,
--- 90,100 ----
(1 row)
SELECT xmlelement(name element,
! xmlattributes (1 as one, 'deuce' as two, '<>&"''' as three),
! 'content', '<>&"''');
! xmlelement
! --------------------------------------------------------------------------------------------
! <element one="1" two="deuce" three="<>&"'">content<>&"'</element>
(1 row)
SELECT xmlelement(name element,
diff -cpr head/src/test/regress/sql/xml.sql not-encode-xmlattributes/src/test/regress/sql/xml.sql
*** head/src/test/regress/sql/xml.sql Fri Apr 10 09:40:51 2009
--- not-encode-xmlattributes/src/test/regress/sql/xml.sql Thu May 28 19:26:47 2009
*************** SELECT xmlconcat(NULL, NULL);
*** 31,38 ****
SELECT xmlelement(name element,
! xmlattributes (1 as one, 'deuce' as two),
! 'content');
SELECT xmlelement(name element,
xmlattributes ('unnamed and wrong'));
--- 31,38 ----
SELECT xmlelement(name element,
! xmlattributes (1 as one, 'deuce' as two, '<>&"''' as three),
! 'content', '<>&"''');
SELECT xmlelement(name element,
xmlattributes ('unnamed and wrong'));
Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
Here is a patch to fix the bug. I added a parameter 'encode' to
map_sql_value_to_xml_value() and pass false for xml attributes.
One thing I was wondering about, which is sort of highlighted by your
patch, is why is there the special exception for XML type in the
existing code, and how does that interact with this behavior?
! /* ... exactly as-is for XML or encode is not required */
! if (type == XMLOID || !encode)
return str;
Seems like there could be cases where we're getting one too many or too
few encoding passes when the input is XML.
regards, tom lane
On Thursday 28 May 2009 13:31:16 Itagaki Takahiro wrote:
Here is a patch to fix the bug. I added a parameter 'encode' to
map_sql_value_to_xml_value() and pass false for xml attributes.
I have committed your patch with minor editing. Thanks.
On Sunday 31 May 2009 20:00:44 Tom Lane wrote:
Itagaki Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
Here is a patch to fix the bug. I added a parameter 'encode' to
map_sql_value_to_xml_value() and pass false for xml attributes.One thing I was wondering about, which is sort of highlighted by your
patch, is why is there the special exception for XML type in the
existing code, and how does that interact with this behavior?
This is so that
xmlelement(name element, xml '<foo/>')
results in
<element><foo/></element>
and
xmlelement(name claim, text '1 < 2')
results in
<claim>1 < 2</claim>
Seems like there could be cases where we're getting one too many or too
few encoding passes when the input is XML.
The patch doesn't actually change anything when the input datum is of type
XML. But anyway I have added a few regression test bits to make the
expectations more explicit.