Re: Another issue with invalid XML values

[ resend due to mail server hiccup ]

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Excerpts from Tom Lane's message of mar jul 19 19:42:54 -0400 2011:
>> Now the risk factor if we do that is that if someone misses a
>> pg_xml_done call, we leave an error handler installed with a context
>> argument that's probably pointing at garbage, and if someone then tries
>> to use libxml without re-establishing their error handler, they've
>> got problems.

> I don't see any holes in this idea (though I didn't look very hard), but
> I was thinking that maybe it's time for this module to hook onto the
> cleanup stuff for the xact error case; or at least have a check that it
> has been properly cleaned up elesewhere. Maybe this can be made to work
> reentrantly if there's a global var holding the current context, and it
> contains a link to the next one up the stack. At least, my impression
> is that the PG_TRY blocks are already messy.

Yeah, that's another way we could go. But I'm not sure how well it
would interact with potential third-party modules setting up their own
libxml error handlers. Anybody have a thought about that?

> BTW I'd like to know your opinion on the fact that this patch added
> two new StringInfo routines defined as static in xml.c. It seems to me
> that if we're going to extend some module's API we should do it properly
> in its own files; otherwise we're bound to repeat the functionality
> elsewhere, and lose opportunities for cleaning up some other code that
> could presumably use similar functionality.

I did think about that for a little bit, but the functions in question
are only a couple lines long and seem rather specialized to what xml.c
needs. I'd just as soon leave them as-is until we actually have a
second use-case to help with picking a generalized API.

regards, tom lane