pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Chao Li <li.evan.chao@gmail.com>
Cc: Jeff Davis <pgsql@j-davis.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Small patch to improve safety of utf8_to_unicode().
Date: Mon, 13 Jul 2026 09:42:47 -0400
Message-ID: <3213927.1783950167@sss.pgh.pa.us> (raw)
In-Reply-To: <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>
	<FAEA6374-2ED2-4988-8CDD-95157599024D@gmail.com>

Chao Li <li.evan.chao@gmail.com> writes:
> On Jul 7, 2026, at 17:56, Jeff Davis <pgsql@j-davis.com> wrote:
>> 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.

> V6 looks good to me.

Coverity is unimpressed, although after reading 07211f64a I wonder if
this patch didn't simply trigger it to pick up a pre-existing problem
(it does seem to act that way sometimes).  It now complains:

/srv/coverity/git/pgsql-git/postgresql/src/common/unicode_case.c: 324             in convert_case()
318     	}
319     
320     	if (result_len < dstsize)
321     		dst[result_len] = '\0';
322     
323     	*pconsumed = srcoff;
>>>     CID 1696680:           (INTEGER_OVERFLOW)
>>>     "result_len", which might have overflowed, is returned from the function.
324     	return result_len;
325     }

What I think this is pointing out is that even if the input string's
length fits in size_t, the length of the case-converted equivalent
string might not.  The code won't write past dstsize, but it will
try to compute the full output length required, and it won't notice
if result_len overflows and wraps around.

I would have written this off as an unreachable edge case, but our
recent experience with unicode_normalize (cf. commit 066b7b144)
makes me hesitant to assume that case-folding can't result in
integer-multiple growth of the string length.  If it can, the overflow
might be reachable on 32-bit platforms, resulting in a silently-broken
result (but no memory clobber AFAICS).

Catering for this case seems like it would require messy warts on the
casefolding functions' API.  So if we can convince ourselves that
overflow can't really occur, I'd be content to add a comment
demonstrating that.  But if we can't prove that, I think we need some
warts :-(

			regards, tom lane





view thread (27+ messages)  latest in thread

Message-ID: <3213927.1783950167@sss.pgh.pa.us>
Permalink:  ../3213927.1783950167@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/3213927.1783950167@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, li.evan.chao@gmail.com, pgsql@j-davis.com
  Subject: Re: Small patch to improve safety of utf8_to_unicode().
  In-Reply-To: <3213927.1783950167@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox