Thread: 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.php
>
> We 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

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.