Horton

Re: Regression with large XML data input

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Jim Jones <jim.jones@uni-muenster.de>
Cc: Erik Wienhold <ewie@ewie.name>, Michael Paquier <michael@paquier.xyz>, Robert Treat <rob@xzilla.net>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-07-29T19:23:37Z
Lists: pgsql-hackers 
Jim Jones <jim.jones@uni-muenster.de> writes:
> I also didn't spot any leaks, but I was rather hesitant to remove it
> after re-reading the code, since there's still a risk of leakage if the
> caller fails to free parsed_nodes in case of an error. However, it seems
> that only xmltotext_with_options relies on this, and even then, the
> result of parsed_nodes is added to a document that gets freed in case of
> failure, so it looks like we're covered.

Yeah, that's a separate issue: once we assign it to *parsed_nodes,
it's on the caller's head to make sure it's freed.  I also spent
awhile staring at xmltotext_with_options.  The early exit for !indent
is safer than it looks: we can only take that path in DOCUMENT mode,
and we won't have any content nodes in that case.  But if we hit
an error somewhere between there and attaching the content nodes to
the document, they'd get leaked.  It looks to me like the only
plausible way for that to happen is an OOM failure within libxml2
operations, which is unlikely but not impossible.  So perhaps it's
worth the trouble to make xmltotext_with_options more wary.

			regards, tom lane

Commits

  1. Remove unnecessary complication around xmlParseBalancedChunkMemory.

  2. Avoid regression in the size of XML input that we will accept.

  3. Use xmlParseInNodeContext not xmlParseBalancedChunkMemory.

  4. Revert "Add support for parsing of large XML data (>= 10MB)"