Re: Another issue with invalid XML values

Florian G. Pflug <fgp@phlo.org>

From: Florian Pflug <fgp@phlo.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Noah Misch <noah@2ndQuadrant.com>, Bernd Helmle <mailings@oopsware.de>, PG Hackers <pgsql-hackers@postgresql.org>
Date: 2011-07-26T14:15:42Z
Lists: pgsql-hackers
On Jul26, 2011, at 15:52 , Tom Lane wrote:
> Florian Pflug <fgp@phlo.org> writes:
>> The only fix I can come up with is to explicitly test whether or not we're
>> able to restore the structured error context in pg_xml_init_library().
> 
> Yeah, I think you are right.
> 
>> The downside of this is that a libxml version upgrade might break postgres,
> 
> Such an upgrade would break Postgres anyway --- better we are able to
> detect and report it clearly than that we fail in bizarre ways.

On further reflection, instead of checking whether we can restore the error
handler in pg_xml_init_library(), we could simply upgrade the elog(WARNING)
in pg_xml_done() to ereport(ERROR), and include a hint that explains the
probably cause.

The upside being that we only fail when we actually need to restore the
error handler. Since there's one caller (parse_xml_decl) who calls
pg_xml_init_library() but not pg_xml_init()/pg_xml_done(), the difference
isn't only academic. At least XML would output will continue to work
work after libxml is upgraded from < 2.7.4 to >= 2.7.4.

If nobody objects, I'll post a patch to that effect later today.

BTW, come to think of it, I believe the elog(ERROR) that you put into
xml_errorHandler should to be upgraded to elog(FATAL). Once we longjmp()
out of libxml, it doesn't seem safe to re-enter it, so making the backend
exit seems to be the only safe thing to do.

best regards,
Florian Pflug