Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Florian Pflug
On Jul26, 2011, at 01:57 , Noah Misch wrote: On Mon, Jul 25, 2011 at 09:06:41PM +0200, Florian Pflug wrote: On Jul25, 2011, at 20:37 , Bernd Helmle wrote: Ah, but i got now what's wrong here: configure is confusing both libxml2 installations, and a quick look into config.log proves that: it

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Tom Lane
Florian Pflug f...@phlo.org writes: What's more, xml.c actually does attempt to protect against this situation by calling LIBXML_TEST_VERSION in pg_xml_init_library(). But that check doesn't fire in our case, because it explicitly allows the actual library version to be newer than the header

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Robert Haas
On Tue, Jul 26, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: As a side note, we don't add an rpath for libxml2 like we do for Perl and Python. Sounds like we should change that. I don't think so.  It would just be another headache for packagers --- in Red Hat distros, at least,

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Florian Pflug
On Jul26, 2011, at 15:52 , Tom Lane wrote: Florian Pflug f...@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

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Tom Lane
Florian Pflug f...@phlo.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

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Florian Pflug
On Jul26, 2011, at 16:22 , Tom Lane wrote: Florian Pflug f...@phlo.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

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Tue, Jul 26, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: I don't think so.  It would just be another headache for packagers --- in Red Hat distros, at least, packages are explicitly forbidden from containing any rpath settings. So what do

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Tom Lane
Florian Pflug f...@phlo.org writes: 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? No objection here --- I had considered writing it that way myself. I refrained for fear of making a bad situation even worse, but

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Tom Lane
Tom Lane t...@sss.pgh.pa.us writes: Robert Haas robertmh...@gmail.com writes: On Tue, Jul 26, 2011 at 9:52 AM, Tom Lane t...@sss.pgh.pa.us wrote: I don't think so.  It would just be another headache for packagers --- in Red Hat distros, at least, packages are explicitly forbidden from

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Noah Misch
On Tue, Jul 26, 2011 at 02:09:13PM +0200, Florian Pflug wrote: On Jul26, 2011, at 01:57 , Noah Misch wrote: We could rearrange things so the xml2-config -L flags (or lack thereof) take priority over a --with-libraries directory for the purpose of finding libxml2. Hm, how would we do that

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Florian Pflug
On Jul26, 2011, at 18:04 , Noah Misch wrote: On Tue, Jul 26, 2011 at 02:09:13PM +0200, Florian Pflug wrote: On Jul26, 2011, at 01:57 , Noah Misch wrote: We could rearrange things so the xml2-config -L flags (or lack thereof) take priority over a --with-libraries directory for the purpose of

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Florian Pflug
On Jul26, 2011, at 17:07 , Tom Lane wrote: Florian Pflug f...@phlo.org writes: 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? No objection here --- I had considered writing it that way myself. I refrained for

Re: [HACKERS] Another issue with invalid XML values

2011-07-26 Thread Tom Lane
Florian Pflug f...@phlo.org writes: Patch attached. Will check and apply this. I've pondered whether to add a check to configure which verifies that the headers match the libxml version exactly at compile time. In the end, I didn't do that, for two reasons. First, there isn't anything wrong

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Bernd Helmle
--On 20. Juli 2011 13:06:17 -0400 Tom Lane t...@sss.pgh.pa.us wrote: I've committed this patch with the discussed changes and some other editorialization. I have to leave for an appointment and can't write anything now about the changes, but feel free to ask questions if you have any. Hmm,

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Florian Pflug
On Jul25, 2011, at 18:53 , Bernd Helmle wrote: --On 20. Juli 2011 13:06:17 -0400 Tom Lane t...@sss.pgh.pa.us wrote: I've committed this patch with the discussed changes and some other editorialization. I have to leave for an appointment and can't write anything now about the changes, but feel

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Bernd Helmle
--On 25. Juli 2011 19:07:50 +0200 Florian Pflug f...@phlo.org wrote: Hm, I have libxml2 2.7.8, installed via Mac Ports, and I cannot reproduce this. Maybe Mac Ports uses a modified libxml2, though. I'll check that. Where did you obtain libxml2 from? This is MacPorts, too: % port installed

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Florian Pflug
On Jul25, 2011, at 19:37 , Bernd Helmle wrote: --On 25. Juli 2011 19:07:50 +0200 Florian Pflug f...@phlo.org wrote: Hm, I have libxml2 2.7.8, installed via Mac Ports, and I cannot reproduce this. Maybe Mac Ports uses a modified libxml2, though. I'll check that. Where did you obtain libxml2

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Bernd Helmle
--On 25. Juli 2011 19:57:40 +0200 Florian Pflug f...@phlo.org wrote: I got a theory. We do distinguish between libxml2 versions for which the structured and the generic error context handler share the error context (older ones), and those with don't (newer ones). Our configure scripts checks

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Florian Pflug
On Jul25, 2011, at 20:37 , Bernd Helmle wrote: Ah, but i got now what's wrong here: configure is confusing both libxml2 installations, and a quick look into config.log proves that: it uses the xml2-config from the OSX libs (my $PATH has /usr in front of the bindir of MacPorts, though i seem to

Re: [HACKERS] Another issue with invalid XML values

2011-07-25 Thread Noah Misch
On Mon, Jul 25, 2011 at 09:06:41PM +0200, Florian Pflug wrote: On Jul25, 2011, at 20:37 , Bernd Helmle wrote: Ah, but i got now what's wrong here: configure is confusing both libxml2 installations, and a quick look into config.log proves that: it uses the xml2-config from the OSX libs (my

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Florian Pflug
On Jul20, 2011, at 01:42 , Tom Lane wrote: Florian Pflug f...@phlo.org writes: Updated patch attached. Do you think this is Ready for Committer? I've been looking through this patch. While it's mostly good, I'm pretty unhappy with the way that the pg_xml_init/pg_xml_done code is

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Tom Lane
I wrote: Alvaro Herrera alvhe...@commandprompt.com writes: 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

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Tom Lane
Florian Pflug f...@phlo.org writes: On Jul20, 2011, at 01:42 , Tom Lane wrote: 1. If you forget pg_xml_done in some code path, you'll find out from an Assert at the next pg_xml_init, which is probably far away from where the actual problem is. Very true. In fact, I did miss one pg_xml_done()

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Florian Pflug
On Jul20, 2011, at 17:08 , Tom Lane wrote: Florian Pflug f...@phlo.org writes: On Jul20, 2011, at 01:42 , Tom Lane wrote: 1. If you forget pg_xml_done in some code path, you'll find out from an Assert at the next pg_xml_init, which is probably far away from where the actual problem is. But

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Tom Lane
Florian Pflug f...@phlo.org writes: I'm fine with having pg_xml_init() palloc the state and pg_xml_done() pfree it, but I'm kinda curious about why you prefer that over making it the callers responsibility and letting callers use a stack-allocated struct if they wish to. We could do it that

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Florian Pflug
On Jul20, 2011, at 17:37 , Tom Lane wrote: Florian Pflug f...@phlo.org writes: I'm fine with having pg_xml_init() palloc the state and pg_xml_done() pfree it, but I'm kinda curious about why you prefer that over making it the callers responsibility and letting callers use a stack-allocated

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Tom Lane
I've committed this patch with the discussed changes and some other editorialization. I have to leave for an appointment and can't write anything now about the changes, but feel free to ask questions if you have any. regards, tom lane -- Sent via pgsql-hackers mailing

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Bruce Momjian
Tom Lane wrote: I've committed this patch with the discussed changes and some other editorialization. I have to leave for an appointment and can't write anything now about the changes, but feel free to ask questions if you have any. Did this fix any of the XML TODOs?

Re: [HACKERS] Another issue with invalid XML values

2011-07-20 Thread Tom Lane
Bruce Momjian br...@momjian.us writes: Did this fix any of the XML TODOs? http://wiki.postgresql.org/wiki/Todo#XML No, not that I know of. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your

Re: [HACKERS] Another issue with invalid XML values

2011-07-19 Thread Tom Lane
Florian Pflug f...@phlo.org writes: Updated patch attached. Do you think this is Ready for Committer? I've been looking through this patch. While it's mostly good, I'm pretty unhappy with the way that the pg_xml_init/pg_xml_done code is deliberately designed to be non-reentrant (ie, throw an

Re: [HACKERS] Another issue with invalid XML values

2011-07-19 Thread Alvaro Herrera
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: [HACKERS] Another issue with invalid XML values

2011-07-19 Thread Tom Lane
[ resend due to mail server hiccup ] Alvaro Herrera alvhe...@commandprompt.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

Re: [HACKERS] Another issue with invalid XML values

2011-06-27 Thread Noah Misch
On Mon, Jun 27, 2011 at 12:45:02AM +0200, Florian Pflug wrote: Updated patch attached. Do you think this is Ready for Committer? Thanks. Yes; I have just marked it that way. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Another issue with invalid XML values

2011-06-26 Thread Florian Pflug
Hi Updated patch attached. Do you think this is Ready for Committer? best regards, Florian Pflug pg_xml_errorhandling.v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:

Re: [HACKERS] Another issue with invalid XML values

2011-06-24 Thread Noah Misch
Hi Florian, On Wed, Jun 22, 2011 at 12:47:42PM +0200, Florian Pflug wrote: Updated patch attached. This builds and passes all tests on both test systems I used previously (CentOS 5 with libxml2-2.6.26-2.1.2.8.el5_5.1 and Ubuntu 8.04 LTS with libxml2 2.6.31.dfsg-2ubuntu1.6). Tested behaviors

Re: [HACKERS] Another issue with invalid XML values

2011-06-24 Thread Florian Pflug
On Jun24, 2011, at 13:24 , Noah Misch wrote: On Wed, Jun 22, 2011 at 12:47:42PM +0200, Florian Pflug wrote: Updated patch attached. This builds and passes all tests on both test systems I used previously (CentOS 5 with libxml2-2.6.26-2.1.2.8.el5_5.1 and Ubuntu 8.04 LTS with libxml2

Re: [HACKERS] Another issue with invalid XML values

2011-06-24 Thread Noah Misch
On Fri, Jun 24, 2011 at 04:15:35PM +0200, Florian Pflug wrote: On Jun24, 2011, at 13:24 , Noah Misch wrote: On Wed, Jun 22, 2011 at 12:47:42PM +0200, Florian Pflug wrote: There's also the parser error : difference; would that also be easy to re-add? (I'm assuming it's not a constant but

Re: [HACKERS] Another issue with invalid XML values

2011-06-20 Thread Noah Misch
Hi Florian, I tested this patch using two systems, a CentOS 5 box with libxml2-2.6.26-2.1.2.8.el5_5.1 and an Ubuntu 8.04 box with libxml2 2.6.31.dfsg-2ubuntu1.6. Both failed to build with this error: xml.c: In function `pg_xml_init': xml.c:934: error: `xmlStructuredErrorContext' undeclared

Re: [HACKERS] Another issue with invalid XML values

2011-06-20 Thread Florian Pflug
Hi Thanks for the extensive review, it's very much appreciated! On Jun20, 2011, at 19:57 , Noah Misch wrote: I tested this patch using two systems, a CentOS 5 box with libxml2-2.6.26-2.1.2.8.el5_5.1 and an Ubuntu 8.04 box with libxml2 2.6.31.dfsg-2ubuntu1.6. Both failed to build with this

Re: [HACKERS] Another issue with invalid XML values

2011-06-20 Thread Noah Misch
On Mon, Jun 20, 2011 at 09:15:51PM +0200, Florian Pflug wrote: On Jun20, 2011, at 19:57 , Noah Misch wrote: I tested this patch using two systems, a CentOS 5 box with libxml2-2.6.26-2.1.2.8.el5_5.1 and an Ubuntu 8.04 box with libxml2 2.6.31.dfsg-2ubuntu1.6. Both failed to build with this

Re: [HACKERS] Another issue with invalid XML values

2011-06-09 Thread Florian Pflug
On Jun2, 2011, at 01:34 , Florian Pflug wrote: On Jun2, 2011, at 00:02 , Noah Misch wrote: On Wed, Jun 01, 2011 at 06:16:21PM +0200, Florian Pflug wrote: Anyway, I'll try to come up with a patch that replaces xmlSetGenericErrorFunc() with xmlSetStructuredErrorFunc(). Sounds sensible. Will

Re: [HACKERS] Another issue with invalid XML values

2011-06-01 Thread Florian Pflug
On Jun1, 2011, at 03:17 , Florian Pflug wrote: My nagging suspicion is that libxml reports errors like there via some callback function, and only returns a non-zero result if there are structural errors in the XML. But my experience with libxml is pretty limited, so maybe someone with more

Re: [HACKERS] Another issue with invalid XML values

2011-06-01 Thread Noah Misch
On Wed, Jun 01, 2011 at 06:16:21PM +0200, Florian Pflug wrote: On Jun1, 2011, at 03:17 , Florian Pflug wrote: My nagging suspicion is that libxml reports errors like there via some callback function, and only returns a non-zero result if there are structural errors in the XML. But my

Re: [HACKERS] Another issue with invalid XML values

2011-06-01 Thread Florian Pflug
On Jun2, 2011, at 00:02 , Noah Misch wrote: On Wed, Jun 01, 2011 at 06:16:21PM +0200, Florian Pflug wrote: Anyway, I'll try to come up with a patch that replaces xmlSetGenericErrorFunc() with xmlSetStructuredErrorFunc(). Sounds sensible. Will this impose any new libxml2 version dependency?

[HACKERS] Another issue with invalid XML values

2011-05-31 Thread Florian Pflug
Hi Unfortunately, I found another way to produce invalid XML values. template1=# SELECT (XPATH('/*', XMLELEMENT(NAME root, XMLATTRIBUTES('' as xmlns[1]; xpath --- root xmlns=/ Since a literal is not allowed in XML attributes, this XML value is not