Re: possible encoding issues with libxml2 functions

From: Noah Misch <noah(at)leadboat(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: possible encoding issues with libxml2 functions
Date: 2017-03-12 20:57:50
Message-ID: 20170312205750.GA1892760@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sun, Mar 12, 2017 at 08:36:58PM +0100, Pavel Stehule wrote:
> 2017-03-12 0:56 GMT+01:00 Noah Misch <noah(at)leadboat(dot)com>:
> > On Mon, Feb 20, 2017 at 07:48:18PM +0100, Pavel Stehule wrote:
> > > There are possible two fixes
> > >
> > > a) clean decl on input - the encoding info can be removed from decl part
> > >
> > > b) use xml_out_internal everywhere before transformation to
> > > xmlChar. pg_xmlCharStrndup can be good candidate.
> >
> > I'd prefer (a) if the xml type were a new feature, because no good can
> > come of
> > storing an encoding in each xml field when we know the actual encoding is
> > the
> > database encoding. However, if you implemented (a), we'd still see
> > untreated
> > values brought over via pg_upgrade. Therefore, I would try (b) first. I
> > suspect the intent of xml_parse() was to implement (b); it will be
> > interesting
> > to see your test case that malfunctions.
> >
>
> I looked there again and I found so this issue is related to xpath function
> only
>
> Functions based on xml_parse are working without problems. xpath_internal
> uses own direct xmlCtxtReadMemory without correct encoding sanitation.
>
> so fix is pretty simple

Please add a test case.

> --- a/src/backend/utils/adt/xml.c
> +++ b/src/backend/utils/adt/xml.c
> @@ -3874,9 +3874,11 @@ xpath_internal(text *xpath_expr_text, xmltype *data,
> ArrayType *namespaces,
> ns_count = 0;
> }
>
> - datastr = VARDATA(data);
> - len = VARSIZE(data) - VARHDRSZ;
> + datastr = xml_out_internal(data, 0);

Why not use xml_parse() instead of calling xmlCtxtReadMemory() directly? The
answer is probably in the archives, because someone understood the problem
enough to document "Some XML-related functions may not work at all on
non-ASCII data when the server encoding is not UTF-8. This is known to be an
issue for xpath() in particular."

> + len = strlen(datastr);
> +
> xpath_len = VARSIZE(xpath_expr_text) - VARHDRSZ;
> +

The two lines of functional change don't create a cause for more newlines, so
don't add these two newlines.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2017-03-12 21:08:12 Re: delta relations in AFTER triggers
Previous Message Stephen Frost 2017-03-12 19:52:30 Re: [REVIEW] macaddr 64 bit (EUI-64) datatype support