Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.600.51.1.1\))
Subject: Re: Small patch to improve safety of utf8_to_unicode().
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <f7000dc6842c75ace64656f9bf697a050fda0cfb.camel@j-davis.com>
Date: Wed, 8 Jul 2026 09:48:02 -0700
Cc: pgsql-hackers@postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <FAEA6374-2ED2-4988-8CDD-95157599024D@gmail.com>
References: <c355354e6c3f4a7aafb047361b73db247260fca0.camel@j-davis.com>
 <541F240E-94AD-4D65-9794-7D6C316BC3FF@gmail.com>
 <f11b6c89612f89b76b758807a32a1d3769652d90.camel@j-davis.com>
 <C09D685C-FA5C-47E1-8EDB-B1FDD635309F@gmail.com>
 <c4b27c82decf5b85523ad01f1df3785d8999f333.camel@j-davis.com>
 <fbcb039a9ab3ba834f34174915254732fdcfae86.camel@j-davis.com>
 <c82358d3b1bbd250a03937534bf76de9f4250d0c.camel@j-davis.com>
 <fb7979177b7ab44d9abc2d423391de4ad9faea46.camel@j-davis.com>
 <313BCA0E-2E74-4EB6-9AA8-10DB7F439151@gmail.com>
 <592441c1d871a4ed0ac1708e132f447caa35869c.camel@j-davis.com>
 <3058CAA1-40E0-4098-893B-615F77CD35E1@gmail.com>
 <0cedb517aee8b79b8e31a4e42885ed88c2a67c5f.camel@j-davis.com>
 <16CAD4DB-9D4A-481A-BB4E-78775C4F4B2C@gmail.com>
 <f7000dc6842c75ace64656f9bf697a050fda0cfb.camel@j-davis.com>
To: Jeff Davis <pgsql@j-davis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/FAEA6374-2ED2-4988-8CDD-95157599024D%40gmail.com>
Precedence: bulk


> On Jul 7, 2026, at 17:56, Jeff Davis <pgsql@j-davis.com> wrote:
>=20
> On Fri, 2026-06-26 at 12:38 +0800, Chao Li wrote:
>> Sounds like 0001 will be back patched. In that case, the commit
>> message "defend against invalid UTF8=E2=80=9D seems too broad. Does =
it make
>> sense to add some brief description about the defend behavior to the
>> function header comment and the commit message?
>=20
> Committed 0001 and backported to 17. I think adding too much
> explanation about behavior we don't expect to actually see would just
> add confusion.
>=20
>> Then I continue to review 0002-0005:
>>=20
>> 0002 - overall looks good. A small comment is:
>> ```
>> + for (int i =3D offset; i > 0;)
>>=20
>> + for (int i =3D offset + ulen; i < len;)
>> ```
>=20
> Committed 0002 and backported to 18. The 'int' is pre-existing, so I
> left it as-is.
>=20
>> 0003 - looks good.
>=20
> Committed to master only.
>=20
>> 0004 - looks good. This commit introduces a new helper utf8decode()
>> that will resolve my previous concern on 0001.
>>=20
>> 0005 - Mostly looks good. This commit applies the new help and my
>> previous concern is resolved. But from what you talked, I guess 0004
>> and 0005 will only be pushed to HEAD.
>>=20
>> Just one tiny comment on 0005:
>> ```
>> + /* invalid UTF8: surrogates */
>> + needed =3D unicode_strfold(NULL, 0, "abc\xED\xA0\x81xyz", 7,
>> &consumed, false);
>> + Assert(needed =3D=3D 3 && consumed =3D=3D 3);
>> ```
>>=20
>> This test passes a 10-char string but uses 7 as srclen. I know that
>> doesn=E2=80=99t affect the test result, but it just adds unnecessary
>> confusion to readers. So maybe change 7 to 10 to reflect to the real
>> string length.
>=20
> Thank you, attached with fix.
>=20
> I'd like to wait for more comments before I commit these last two
> patches, to see if the functions are generally useful for other =
callers
> as well.
>=20
> Regards,
> Jeff Davis
>=20
> =
<v6-0001-Validating-iterator-friendly-UTF8-encoder-decoder.patch><v6-0002-=
unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch>

V6 looks good to me.

Best regards,
=E2=80=94
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/