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: <c355354e6c3f4a7aafb047361b73db247260fca0.camel@j-davis.com>
Date: Sun, 14 Dec 2025 07:22:28 +0800
Cc: pgsql-hackers@postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <541F240E-94AD-4D65-9794-7D6C316BC3FF@gmail.com>
References: <c355354e6c3f4a7aafb047361b73db247260fca0.camel@j-davis.com>
To: Jeff Davis <pgsql@j-davis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/541F240E-94AD-4D65-9794-7D6C316BC3FF%40gmail.com>
Precedence: bulk


> On Dec 13, 2025, at 07:24, Jeff Davis <pgsql@j-davis.com> wrote:
>=20
> Attached.
>=20
>=20
> <v1-0001-Make-utf8_to_unicode-safer.patch>


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=9Cimpr=
ove safety=E2=80=9D. It also deleted two redundant function declarations =
from pg_wchar.h, which may also worth a quick note in the commit =
message.

The code changes all look good to me. Only nitpicks are:

1
```
diff --git a/contrib/fuzzystrmatch/daitch_mokotoff.c =
b/contrib/fuzzystrmatch/daitch_mokotoff.c
index 07f895ae2bf..47bd2814460 100644
--- a/contrib/fuzzystrmatch/daitch_mokotoff.c
+++ b/contrib/fuzzystrmatch/daitch_mokotoff.c
@@ -401,7 +401,8 @@ read_char(const unsigned char *str, int *ix)
=20
 	/* Decode UTF-8 character to ISO 10646 code point. */
 	str +=3D *ix;
-	c =3D utf8_to_unicode(str);
+	/* Assume byte sequence has not been broken. */
+	c =3D utf8_to_unicode(str, MAX_MULTIBYTE_CHAR_LEN);
```

Here we need an empty line above the new comment.

2
```
diff --git a/src/common/wchar.c b/src/common/wchar.c
index a4bc29921de..c113cadf815 100644
--- a/src/common/wchar.c
+++ b/src/common/wchar.c
@@ -661,7 +661,8 @@ ucs_wcwidth(pg_wchar ucs)
 static int
 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));
 }
```

For the new comment, as a code reader, I wonder why we =E2=80=9Ctrust=E2=80=
=9D that? To me, it more feels like because of lacking length =
information, we have to trust. I would like this comment to be enhanced =
a little bit with more information.

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