Jim Jones 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
On 29.07.25 17:27, Tom Lane wrote:
> Re-reading the prior thread, I see that my memory above is quite
> faulty: we added the node_list intermediate variable as a way to
> detect errors when the return code from xmlParseBalancedChunkMemory
> couldn't be trusted. So I think you're right to questi
Jim Jones writes:
> On 29.07.25 14:11, Tom Lane wrote:
>> In the original coding, there was a hazard of the node list getting
>> leaked if the caller passed parsed_nodes == NULL. Or at least I
>> thought there was. It may be that all releases of libxml2 are smart
>> enough to free the node list
On 29.07.25 14:11, Tom Lane wrote:
> In the original coding, there was a hazard of the node list getting
> leaked if the caller passed parsed_nodes == NULL. Or at least I
> thought there was. It may be that all releases of libxml2 are smart
> enough to free the node list if there's no way to p
Jim Jones writes:
> Out of curiosity, what's the reasoning behind keeping node_list instead
> of directly using parsed_nodes in the xmlParseBalancedChunkMemory call?
In the original coding, there was a hazard of the node list getting
leaked if the caller passed parsed_nodes == NULL. Or at least
On 28.07.25 22:16, Tom Lane wrote:
> Erik's v2 is slightly wrong as to the save-and-restore logic for
> the KeepBlanks setting: we need to restore in the error path too,
> and we'd better mark the save variable volatile since it's modified
> inside the PG_TRY. I made some other cosmetic changes,
On Mon, Jul 28, 2025 at 04:16:07PM -0400, Tom Lane wrote:
> Erik's v2 is slightly wrong as to the save-and-restore logic for
> the KeepBlanks setting: we need to restore in the error path too,
> and we'd better mark the save variable volatile since it's modified
> inside the PG_TRY. I made some ot
I wrote:
> I've not looked at the details of the proposed patches, but will
> do so now that the direction to go in is apparent.
Erik's v2 is slightly wrong as to the save-and-restore logic for
the KeepBlanks setting: we need to restore in the error path too,
and we'd better mark the save variable
Erik Wienhold 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 usin
On 2025-07-28 09:45 +0200, Jim Jones wrote:
>
> On 28.07.25 04:47, Michael Paquier wrote:
> > I understand that from the point of view of a maintainer this is
> > rather bad, but from the customer point of view the current
> > situation is also bad to deal with in the scope of a minor upgrade,
> >
On 28.07.25 04:47, Michael Paquier wrote:
> I understand that from the point of view of a
> maintainer this is rather bad, but from the customer point of view the
> current situation is also bad to deal with in the scope of a minor
> upgrade, because applications suddenly break.
I totally get i
On Sunday, July 27, 2025, Michael Paquier wrote:
>
> I am not going to argue against the commits that have reached
> REL_18_STABLE to add compatibility for libxml2 2.13.X, we can leave
> this stuff as-is and enforce stronger restrictions across all versions
> of libxml2, letting users deal with ap
On Sun, Jul 27, 2025 at 10:16:47PM -0400, Tom Lane wrote:
> Michael Paquier writes:
>> This sentence is incorrect after I have double-checked the behaviors I
>> am seeing based on local builds of libxml2 2.9.7 and 2.9.14.
>
> Hmm, okay, I misread Jim's results then. But there still remains
> the
Michael Paquier writes:
> On Fri, Jul 25, 2025 at 02:21:26PM -0400, Tom Lane wrote:
>> However, from Jim Jones' result upthread, a "minor update" of libxml2
>> could also have caused this problem: 2.9.7 and 2.9.14 behave
>> differently. So we don't have sole control --- or sole responsibility
>>
On Fri, Jul 25, 2025 at 02:21:26PM -0400, Tom Lane wrote:
> I'll be the first to say that I'm not too pleased with it either.
> However, from Jim Jones' result upthread, a "minor update" of libxml2
> could also have caused this problem: 2.9.7 and 2.9.14 behave
> differently. So we don't have sole
Robert Treat writes:
> On Thu, Jul 24, 2025 at 8:08 PM Michael Paquier wrote:
>> If it were discussing things from the perspective where this new code
>> was added after a major version bump of Postgres, I would not argue
>> much about that: breakages happen every year and users adapt their
>> ap
On Thu, Jul 24, 2025 at 8:08 PM Michael Paquier wrote:
> On Fri, Jul 25, 2025 at 01:25:48AM +0200, Jim Jones wrote:
> > On 24.07.25 21:23, Tom Lane wrote:
> >> However, when testing on RHEL8 with libxml2 2.9.7, indeed
> >> I get "Huge input lookup" with our current code but no
> >> failure with f6
On Fri, Jul 25, 2025 at 01:25:48AM +0200, Jim Jones wrote:
> On 24.07.25 21:23, Tom Lane wrote:
>> However, when testing on RHEL8 with libxml2 2.9.7, indeed
>> I get "Huge input lookup" with our current code but no
>> failure with f68d6aabb7e2^.
>>
>> The way I interpret these results is that in ol
On 24.07.25 21:23, Tom Lane wrote:
> Oh, wait ... the plot thickens! The above statement is true
> when testing on my Mac with libxml2 2.13.8 from MacPorts.
> With either HEAD or f68d6aabb7e2^, I get errors similar to
> what Erik just showed:
>
> ERROR: invalid XML content
> DETAIL: line 1: R
I wrote:
> BTW, further testing shows that the same failure occurs at
> f68d6aabb7e2^. So AFAICS, the answer as to why the behavior
> changed there is that it didn't.
Oh, wait ... the plot thickens! The above statement is true
when testing on my Mac with libxml2 2.13.8 from MacPorts.
With either
On 2025-07-24 05:12 +0200, Michael Paquier wrote:
> Switching back to the previous code, where we rely on
> xmlParseBalancedChunkMemory() fixes the issue. A quick POC is
> attached. It fails one case in check-world with SERIALIZE because I
> am not sure it is possible to pass down some options th
Michael Paquier writes:
>>> A customer has reported a regression with the parsing of rather large
>>> XML data, introduced by the set of backpatches done with f68d6aabb7e2
>>> & friends.
BTW, further testing shows that the same failure occurs at
f68d6aabb7e2^. So AFAICS, the answer as to why the
On 2025-07-24 20:10 +0200, Tom Lane wrote:
> The supplied test case hides important details in the error message.
> If you get rid of the exception block so that the error is reported
> in full, what you see is
>
> regression=# CREATE TEMP TABLE xmldata (id BIGINT PRIMARY KEY, message XML );
> CRE
I wrote:
> Michael Paquier writes:
>> A customer has reported a regression with the parsing of rather large
>> XML data, introduced by the set of backpatches done with f68d6aabb7e2
>> & friends.
> Bleah.
The supplied test case hides important details in the error message.
If you get rid of the e
Michael Paquier writes:
> Are you planning to look at that for the next minor release? It would
> take me a couple of hours to dig into all that, and I am sure that I
> am going to need your stamp or Erik's to avoid doing a stupid thing.
It was my commit, so my responsibility, so I'll deal with
On Wed, Jul 23, 2025 at 11:28:38PM -0400, Tom Lane wrote:
> Michael Paquier writes:
>> Switching back to the previous code, where we rely on
>> xmlParseBalancedChunkMemory() fixes the issue.
>
> Yeah, just reverting these commits might be an acceptable answer,
> since the main point was to work a
Michael Paquier writes:
> A customer has reported a regression with the parsing of rather large
> XML data, introduced by the set of backpatches done with f68d6aabb7e2
> & friends.
Bleah.
> Switching back to the previous code, where we rely on
> xmlParseBalancedChunkMemory() fixes the issue.
Ye
Hi all,
(Adding in CC Tom and Eric, as committer and author.)
A customer has reported a regression with the parsing of rather large
XML data, introduced by the set of backpatches done with f68d6aabb7e2
& friends.
The problem is introduced by the change from
xmlParseBalancedChunkMemory() to xmlNe
28 matches
Mail list logo
