Content-Type: text/plain;
	charset=us-ascii
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: <592441c1d871a4ed0ac1708e132f447caa35869c.camel@j-davis.com>
Date: Thu, 25 Jun 2026 12:10:26 +0800
Cc: pgsql-hackers@postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <3058CAA1-40E0-4098-893B-615F77CD35E1@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>
To: Jeff Davis <pgsql@j-davis.com>
Archived-At: 
 <https://www.postgresql.org/message-id/3058CAA1-40E0-4098-893B-615F77CD35E1%40gmail.com>
Precedence: bulk


> On Jun 25, 2026, at 05:57, Jeff Davis <pgsql@j-davis.com> wrote:
>=20
> On Wed, 2026-06-24 at 16:44 +0800, Chao Li wrote:
>> There is a compile warning against pg_wchar.h in 0004:
>=20
> Fixed. I also used a loop in utf8decode() which is slightly smaller,
> which is good if we intend it to be inlined by a lot of callers.
>=20
> Regards,
> Jeff Davis
>=20
> =
<v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch><v5-0002-pg_unic=
ode_fast-fix-final-sigma-logic.patch><v5-0003-unicode_case.c-change-API-to=
-signal-UTF8-decoding.patch><v5-0004-Validating-iterator-friendly-UTF8-enc=
oder-decoder.patch><v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-A=
PIs.patch>

I just reviewed v5-0001 and got one concern.

In initcap_wbnext(), the new check only verifies that the input has =
enough bytes:
```
if (wbstate->offset + ulen > wbstate->len)
```

What about an invalid continuation byte, for example "\xCE "? In this =
case, pg_utf_mblen() sees \xCE, so ulen will be 2. Since there is still =
one more byte, the length check won't catch the invalid continuation =
byte \x20, and the code will proceed to utf8_to_unicode().

Looking at utf8_to_unicode():
```
static inline char32_t
utf8_to_unicode(const unsigned char *c)
{
	if ((*c & 0x80) =3D=3D 0)
		return (char32_t) c[0];
	else if ((*c & 0xe0) =3D=3D 0xc0)
		return (char32_t) (((c[0] & 0x1f) << 6) |
						   (c[1] & 0x3f));
	else if ((*c & 0xf0) =3D=3D 0xe0)
		return (char32_t) (((c[0] & 0x0f) << 12) |
						   ((c[1] & 0x3f) << 6) =
|
						   (c[2] & 0x3f));
	else if ((*c & 0xf8) =3D=3D 0xf0)
		return (char32_t) (((c[0] & 0x07) << 18) |
						   ((c[1] & 0x3f) << 12) =
|
						   ((c[2] & 0x3f) << 6) =
|
						   (c[3] & 0x3f));
	else
		return PG_INVALID_CODEPOINT;
}
```

For "\xCE ", it will take this branch:
```
	else if ((*c & 0xe0) =3D=3D 0xc0)
		return (char32_t) (((c[0] & 0x1f) << 6) |
						   (c[1] & 0x3f));
```

This uses the second byte, \x20, without validating. So it looks like =
the patch prevents reading past the end of the string, but it may not =
fully defend against invalid UTF-8 sequences.

Am I missing anything?

(I will continue to review 0002 tomorrow.)

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