Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: Small patch to improve safety of utf8_to_unicode().
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <f11b6c89612f89b76b758807a32a1d3769652d90.camel@j-davis.com>
Date: Tue, 16 Dec 2025 07:34:41 +0800
Cc: pgsql-hackers@postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <C09D685C-FA5C-47E1-8EDB-B1FDD635309F@gmail.com>
References: <c355354e6c3f4a7aafb047361b73db247260fca0.camel@j-davis.com>
 <541F240E-94AD-4D65-9794-7D6C316BC3FF@gmail.com>
 <f11b6c89612f89b76b758807a32a1d3769652d90.camel@j-davis.com>
To: Jeff Davis <pgsql@j-davis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/C09D685C-FA5C-47E1-8EDB-B1FDD635309F%40gmail.com>
Precedence: bulk


> On Dec 16, 2025, at 04:23, Jeff Davis <pgsql@j-davis.com> wrote:
>=20
> On Sun, 2025-12-14 at 07:22 +0800, Chao Li wrote:
>> This patch adds a length check to utf8_to_unicode(), I think which is
>> where =E2=80=9Csafety=E2=80=9D comes from. Can you please add an a =
little bit more to
>> the commit message instead of only saying =E2=80=9Cimprove safety=E2=80=
=9D.
>=20
> Right: it does not read past pg_mblen(), nor past the supplied length.
>=20
> We could go further and check for valid continuation bytes, but the
> other routines don't do that.
>=20
>> It also deleted two redundant function declarations from pg_wchar.h,
>> which may also worth a quick note in the commit message.
>=20
> I committed that as an independent change and removed it from this
> patch.
>=20
>> + /* Assume byte sequence has not been broken. */
>> + c =3D utf8_to_unicode(str, MAX_MULTIBYTE_CHAR_LEN);
>> ```
>>=20
>> Here we need an empty line above the new comment.
>=20
> Done, and I expanded the comment to explain why it's safe.
>=20
>>  pg_utf_dsplen(const unsigned char *s)
>>  {
>> - return ucs_wcwidth(utf8_to_unicode(s));
>> + /* trust that input is not a truncated byte sequence */
>> + return ucs_wcwidth(utf8_to_unicode(s,
>> MAX_MULTIBYTE_CHAR_LEN));
>>  }
>> ```
>>=20
>> For the new comment, as a code reader, I wonder why we =E2=80=9Ctrust=E2=
=80=9D that?
>=20
> We could use strlen(), but I was concerned that it might be used for
> string fragments that aren't NUL-terminated, because it's intended for
> a single character. A caller might reasonably assume that it wouldn't
> read past pg_mblen().
>=20
> So I changed the comment slightly to just say that it requires the
> input is a valid UTF-8 sequence. Let me know if you have another
> suggestion.
>=20
> Regards,
> Jeff Davis
>=20
>=20
> <v2-0001-Make-utf8_to_unicode-safer.patch>

V2 LGTM.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/