| From: | Jeff Davis <pgsql(at)j-davis(dot)com> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Small patch to improve safety of utf8_to_unicode(). |
| Date: | 2026-07-08 00:56:57 |
| Message-ID: | f7000dc6842c75ace64656f9bf697a050fda0cfb.camel@j-davis.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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” 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?
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.
> Then I continue to review 0002-0005:
>
> 0002 - overall looks good. A small comment is:
> ```
> + for (int i = offset; i > 0;)
>
> + for (int i = offset + ulen; i < len;)
> ```
Committed 0002 and backported to 18. The 'int' is pre-existing, so I
left it as-is.
> 0003 - looks good.
Committed to master only.
> 0004 - looks good. This commit introduces a new helper utf8decode()
> that will resolve my previous concern on 0001.
>
> 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.
>
> Just one tiny comment on 0005:
> ```
> + /* invalid UTF8: surrogates */
> + needed = unicode_strfold(NULL, 0, "abc\xED\xA0\x81xyz", 7,
> &consumed, false);
> + Assert(needed == 3 && consumed == 3);
> ```
>
> This test passes a 10-char string but uses 7 as srclen. I know that
> doesn’t 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.
Thank you, attached with fix.
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.
Regards,
Jeff Davis
| Attachment | Content-Type | Size |
|---|---|---|
| v6-0001-Validating-iterator-friendly-UTF8-encoder-decoder.patch | text/x-patch | 4.8 KB |
| v6-0002-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch | text/x-patch | 8.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Richard Guo | 2026-07-08 00:57:24 | Re: Propagate stadistinct through GROUP BY/DISTINCT in subqueries and CTEs |
| Previous Message | Michael Paquier | 2026-07-08 00:47:03 | Re: VACUUM FULL or CREATE INDEX fails with error: missing chunk number 0 for toast value XXX |