From: Michael Paquier <michael@paquier.xyz>
To: Dmitry Koval <d.koval@postgrespro.ru>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18274: Error 'invalid XML content'
Date: Tue, 16 Jan 2024 18:14:09 +0900
Message-ID: <ZaZI4WAHZ67ZHq7p@paquier.xyz> (raw)
In-Reply-To: <35ef78f9-8958-4225-ae7e-aead71b0feed@postgrespro.ru>
References: <18274-98d16bc03520665f@postgresql.org>
<1616257.1704651529@sss.pgh.pa.us>
<5588db01-45be-48ff-b158-85f789bc502c@postgrespro.ru>
<1314313.1705259848@sss.pgh.pa.us>
<0eea123f-388a-41ea-9df3-d4606cf9ed8d@postgrespro.ru>
<ZaRvbaQ8V9uGxXGD@paquier.xyz>
<35ef78f9-8958-4225-ae7e-aead71b0feed@postgrespro.ru>
On Mon, Jan 15, 2024 at 06:47:17PM +0300, Dmitry Koval wrote:
> I think that's right (flag XML_PARSE_HUGE shouldn't cause any problems).
> My main doubts are related to the replacement of the
> xmlParseBalancedChunkMemory() function (that haven't argument for pass
> XML_PARSE_HUGE flag) with xmlNewNode() + xmlParseInNodeContext() functions
> (create a fake node and pass the XML_PARSE_HUGE flag to
> xmlParseInNodeContext function).
>
> I'm not sure if this replacement is 100% equivalent (although simple tests
> work the same).
Hmm, it looks like this is actually equivalent in terms of parsing a
well-balanced chunk. This was suggested in the upstream ticket you
have opened and I find that pretty cool, reusing the trick of a fake
root node to use the other API. Now, there are two things that we'd
better do here:
- Document in a comment why a fake root node is necessary (aka the
current routines don't give enough control over the limits you'd like
to enforce).
- The top comment of xml_parse() still mentions
xmlParseBalancedChunkMemory() as an effect of 483bdb2afec9, so this
needs to be updated.
The switch xmlParseMemory() -> xmlReadMemory() is recommended by the
upstream docs and the former is deprecated:
https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-parser.html#xmlParseMemory
Also, it may be worth double-checking if there are other things marked
as deprecated in the upstream doc, and switch to "newer" things.
It seems like anything discussed here should only be done on HEAD.
I suspect that the buildfarm may get bumpy on that, but let's see.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../ZaZI4WAHZ67ZHq7p@paquier.xyz/2-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, d.koval@postgrespro.ru, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18274: Error 'invalid XML content'
In-Reply-To: <ZaZI4WAHZ67ZHq7p@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox