Re: Another issue with invalid XML values

On Jul26, 2011, at 16:22 , Tom Lane wrote:
> Florian Pflug <fgp(at)phlo(dot)org> writes:
>> 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.
>
> Good point. But what about failing in pg_xml_init? That is, after
> calling xmlSetStructuredErrorFunc, check that it set the variable we
> expected it to set.

Yeah, that's even better. Will do it that way.

What about the suggested upgrade of the elog(ERROR) in xml_errorHandler
to elog(FATAL)? Shall I do that too, or leave it for now?

> The purpose of the check in pg_xml_done is not to detect library issues,
> but to detect omitted save/restore pairs and similar coding mistakes.
> I don't want to upgrade it to an ERROR, and I don't want to confuse
> people by hinting that the problem is with libxml.

I can see the concern about possible confusion, and I agree that
pg_xml_init(), right after we set our error handler, is the most logical
place to test whether we can read it back.

I guess I don't really see the benefit of the check in pg_xml_done()
triggering a WARNING instead of an ERROR, but since we won't be hijacking
that check now anyway, I don't mind it being a WARNING either.

best regards,
Florian Pflug