Re: Regression with large XML data input
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Erik Wienhold <ewie@ewie.name>
Cc: Jim Jones <jim.jones@uni-muenster.de>,
Michael Paquier <michael@paquier.xyz>, Robert Treat <rob@xzilla.net>,
Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-07-28T18:28:43Z
Lists: pgsql-hackers
Erik Wienhold <ewie@ewie.name> writes:
> I'm leaning towards Michael's proposal of adding a libxml2 version check
> in the stable branches before REL_18_STABLE and parsing the content with
> xmlParseBalancedChunkMemory on versions up to 2.12.x.
I spent some time investigating this. It appears that even when using
our old code with xmlParseBalancedChunkMemory (I tested against our
commit 6082b3d5d^), libxml2 versions 2.12.x and 2.13.x will throw
a resource-exhaustion error; but earlier and later releases do not.
Drilling down with "git bisect", the first libxml2 commit that throws
an error is
commit 834b8123efc4a4c369671cad2f1b0eb744ae67e9
Author: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Tue Aug 8 15:21:28 2023 +0200
parser: Stream data when reading from memory
Don't create a copy of the whole input buffer. Read the data chunk by
chunk to save memory.
and then the one that restores it to not throwing an error is
commit 7148b778209aaaf684c156c5e2e40d6e477f13f7
Author: Nick Wellnhofer <wellnhofer@aevum.de>
Date: Sun Jul 7 16:11:08 2024 +0200
parser: Optimize memory buffer I/O
Reenable zero-copy IO for zero-terminated static memory buffers.
Don't stream zero-terminated dynamic memory buffers on top of creating
a copy.
So apparently the "resource exhaustion" complaint is not about
anything deeper than not wanting to make a copy of a many-megabyte
input string, and the fact that it appeared and disappeared is an
artifact of libxml2 implementation choices that we have no control
over (and, IMO, no responsibility for working around).
Based on this, I'm okay with reverting to using
xmlParseBalancedChunkMemory: while that won't help users of 2.12.x or
2.13.x, it will help users of earlier and later libxml2. But I think
we should just do that across the board, not insert libxml2 version
tests nor do it differently in different PG versions.
I've not looked at the details of the proposed patches, but will
do so now that the direction to go in is apparent.
regards, tom lane
Commits
-
Remove unnecessary complication around xmlParseBalancedChunkMemory.
- 902f92221889 19 (unreleased) landed
- cdcdabce5b70 14.19 landed
- 0ae824704070 13.22 landed
- 0928e18eb85a 15.14 landed
- d5f014d897c8 18.0 landed
- 762c6d8d26e0 16.10 landed
- 7571e0f6e924 17.6 landed
-
Avoid regression in the size of XML input that we will accept.
- 6d5e493b4a15 16.10 landed
- 589d6e6408b4 13.22 landed
- 0ffbd345e43d 15.14 landed
- 0395464aff02 14.19 landed
- fd4ad33fe223 17.6 landed
- 71c0921b649d 19 (unreleased) landed
- 637ead2e1aa1 18.0 landed
-
Use xmlParseInNodeContext not xmlParseBalancedChunkMemory.
- 6082b3d5d3d1 18.0 cited
-
Revert "Add support for parsing of large XML data (>= 10MB)"
- f2743a7d70e7 17.0 cited