| From: | Rui Zhao <zhaorui126(at)gmail(dot)com> |
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
| Cc: | Andrew Dunstan <andrew(at)dunslane(dot)net>, "Daniel Westermann (DWE)" <daniel(dot)westermann(at)dbi-services(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Deprecation warnings on Rocky 10.2 with current dev branch |
| Date: | 2026-10-13 06:37:48 |
| Message-ID: | CAHWVJhGqXPuAQpd1U2qftwf4Ub4qjPO97=Gefpzhg34S-RgZGA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Tom,
Thanks for your reply.
> Wow, how'd you find that? I do replicate this result on the 2.9.13
> that Apple is currently shipping, so I agree it might be worth
> worrying about. (But then again, it seems very likely nobody would
> ever notice.)
I noticed that the main change replaced xmlParseBalancedChunkMemory()
with xmlParseInNodeContext(). I read the latter's source and saw its
handling of doc->encoding, then asked AI to help construct a case
where that could lose XML content.
> Hmm, 0001 seems quite odd to me. Why bother to install a value for
> doc->encoding at all, if we're not going to do so until after all
> the calls that will pay attention to it? I tried simply commenting
> out the assignment:
>
> Assert(doc->encoding == NULL);
> + #ifdef NOT_USED
> doc->encoding = xmlStrdup((const xmlChar *) "UTF-8");
> if (doc->encoding == NULL || xmlerrcxt->err_occurred)
> xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
> "could not allocate XML document");
> + #endif
> doc->standalone = standalone;
>
> and that seemed to work fine.
I kept the assignment at the end of xml_parse() so later calls would
still get doc->encoding set to "UTF-8". I had checked xmlNewNode(),
xmlDocSetRootElement(), xmlParseInNodeContext() and xmlAddChildList()
in libxml2 2.9.7, 2.12.5 and 2.15.4. Leaving doc->encoding unset
during those calls fixed the U+FEFF loss without changing the later
serialization.
After your suggestion, I traced later uses of doc->encoding and asked
AI again to help construct a case. I verified it in PG with libxml2
2.9.7 and 2.12.5, in a UTF8 database:
SELECT xmlserialize(CONTENT '<a value="é"/>' AS text INDENT);
-- Master: <a value="é"/>
-- With 0001: <a value="é"/>
-- With #ifdef NOT_USED: <a value="é"/>
The reason is that xmlSaveTree() calls xmlBufAttrSerializeTxtContent()
through xmlAttrSerializeContent(), passing attr->doc. It checks the
document's encoding; when that is NULL, non-ASCII attribute characters
are written as character references. Here, é represents the same
é character.
> Hm. While that's not great, it sure seems like an edge case.
> There is no particularly well-defined behavior for whitespace in
> INDENT mode anyway, is there? How would you tell which whitespace
> was in the original input and which was INDENT's responsibility?
> So I'm kind of inclined to let it go. If the proposed fix weren't
> so ugly I might feel differently, but it's messy, overly familiar
> with libxml2 innards, and perhaps could introduce odd cases of its
> own in libxml2 versions other than the ones you tested. Plus the
> comments don't actually agree with what the code is doing AFAICT.
> (Why do we not need to take action if the input has leading
> whitespace? Wouldn't the injected comment mess it up anyway?)
Agreed. The workaround could introduce other differences in library
versions. I'm happy to leave this INDENT whitespace behavior to libxml2
and drop 0002.
Regards,
Rui
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hayato Kuroda (Fujitsu) | 2026-10-13 06:54:30 | RE: Test publication.sql racing with subscription.sql might fail due to cache lookup error |
| Previous Message | Mahendra Singh Thalor | 2026-10-13 06:12:52 | Re: Non-text mode for pg_dumpall |