public inbox for [email protected]help / color / mirror / Atom feed
Small patch to improve safety of utf8_to_unicode(). 18+ messages / 3 participants [nested] [flat]
* Small patch to improve safety of utf8_to_unicode(). @ 2025-12-12 23:24 Jeff Davis <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2025-12-12 23:24 UTC (permalink / raw) To: pgsql-hackers Attached. Attachments: [text/x-patch] v1-0001-Make-utf8_to_unicode-safer.patch (9.9K, ../../[email protected]/2-v1-0001-Make-utf8_to_unicode-safer.patch) download | inline diff: From 4777d1af75bc9e570e7db954b6642916fa1420bc Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 4 Dec 2025 19:37:32 -0800 Subject: [PATCH v1] Make utf8_to_unicode() safer. --- contrib/fuzzystrmatch/daitch_mokotoff.c | 3 ++- src/backend/utils/adt/pg_locale_builtin.c | 5 +++-- src/backend/utils/adt/varlena.c | 26 +++++++++++------------ src/common/saslprep.c | 4 +++- src/common/unicode/case_test.c | 5 +++-- src/common/unicode_case.c | 12 +++++++---- src/common/wchar.c | 3 ++- src/include/mb/pg_wchar.h | 14 +++++------- 8 files changed, 38 insertions(+), 34 deletions(-) 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) /* Decode UTF-8 character to ISO 10646 code point. */ str += *ix; - c = utf8_to_unicode(str); + /* Assume byte sequence has not been broken. */ + c = utf8_to_unicode(str, MAX_MULTIBYTE_CHAR_LEN); /* Advance *ix, but (for safety) not if we've reached end of string. */ if (c) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 0d4c754a267..5c1358e5347 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -59,12 +59,13 @@ static size_t initcap_wbnext(void *state) { struct WordBoundaryState *wbstate = (struct WordBoundaryState *) state; + unsigned char *str = (unsigned char *) wbstate->str; while (wbstate->offset < wbstate->len && wbstate->str[wbstate->offset] != '\0') { - char32_t u = utf8_to_unicode((unsigned char *) wbstate->str + - wbstate->offset); + char32_t u = utf8_to_unicode(str + wbstate->offset, + wbstate->len - wbstate->offset); bool curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index baa5b44ea8d..d684370900d 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -5423,19 +5423,17 @@ Datum unicode_assigned(PG_FUNCTION_ARGS) { text *input = PG_GETARG_TEXT_PP(0); - unsigned char *p; - int size; + unsigned char *p = (unsigned char *) VARDATA_ANY(input); + unsigned char *p_end = p + VARSIZE_ANY_EXHDR(input); if (GetDatabaseEncoding() != PG_UTF8) ereport(ERROR, (errmsg("Unicode categorization can only be performed if server encoding is UTF8"))); /* convert to char32_t */ - size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); - p = (unsigned char *) VARDATA_ANY(input); - for (int i = 0; i < size; i++) + while (p < p_end) { - char32_t uchar = utf8_to_unicode(p); + char32_t uchar = utf8_to_unicode(p, p_end - p); int category = unicode_category(uchar); if (category == PG_U_UNASSIGNED) @@ -5456,7 +5454,8 @@ unicode_normalize_func(PG_FUNCTION_ARGS) int size; char32_t *input_chars; char32_t *output_chars; - unsigned char *p; + unsigned char *p = (unsigned char *) VARDATA_ANY(input); + unsigned char *p_end = p + VARSIZE_ANY_EXHDR(input); text *result; int i; @@ -5465,14 +5464,13 @@ unicode_normalize_func(PG_FUNCTION_ARGS) /* convert to char32_t */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); input_chars = palloc((size + 1) * sizeof(char32_t)); - p = (unsigned char *) VARDATA_ANY(input); for (i = 0; i < size; i++) { - input_chars[i] = utf8_to_unicode(p); + input_chars[i] = utf8_to_unicode(p, p_end - p); p += pg_utf_mblen(p); } input_chars[i] = (char32_t) '\0'; - Assert((char *) p == VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input)); + Assert(p == p_end); /* action */ output_chars = unicode_normalize(form, input_chars); @@ -5522,7 +5520,8 @@ unicode_is_normalized(PG_FUNCTION_ARGS) int size; char32_t *input_chars; char32_t *output_chars; - unsigned char *p; + unsigned char *p = (unsigned char *) VARDATA_ANY(input); + unsigned char *p_end = p + VARSIZE_ANY_EXHDR(input); int i; UnicodeNormalizationQC quickcheck; int output_size; @@ -5533,14 +5532,13 @@ unicode_is_normalized(PG_FUNCTION_ARGS) /* convert to char32_t */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); input_chars = palloc((size + 1) * sizeof(char32_t)); - p = (unsigned char *) VARDATA_ANY(input); for (i = 0; i < size; i++) { - input_chars[i] = utf8_to_unicode(p); + input_chars[i] = utf8_to_unicode(p, p_end - p); p += pg_utf_mblen(p); } input_chars[i] = (char32_t) '\0'; - Assert((char *) p == VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input)); + Assert(p == p_end); /* quick check (see UAX #15) */ quickcheck = unicode_is_normalized_quickcheck(form, input_chars); diff --git a/src/common/saslprep.c b/src/common/saslprep.c index 101e8d65a4d..083702654b0 100644 --- a/src/common/saslprep.c +++ b/src/common/saslprep.c @@ -1055,6 +1055,7 @@ pg_saslprep(const char *input, char **output) int i; bool contains_RandALCat; unsigned char *p; + unsigned char *p_end; char32_t *wp; /* Ensure we return *output as NULL on failure */ @@ -1088,9 +1089,10 @@ pg_saslprep(const char *input, char **output) goto oom; p = (unsigned char *) input; + p_end = p + strlen(input); for (i = 0; i < input_size; i++) { - input_chars[i] = utf8_to_unicode(p); + input_chars[i] = utf8_to_unicode(p, p_end - p); p += pg_utf_mblen(p); } input_chars[i] = (char32_t) '\0'; diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index 00d4f85e5a5..e63ce2fbeb9 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -51,12 +51,13 @@ static size_t initcap_wbnext(void *state) { struct WordBoundaryState *wbstate = (struct WordBoundaryState *) state; + unsigned char *str = (unsigned char *) wbstate->str; while (wbstate->offset < wbstate->len && wbstate->str[wbstate->offset] != '\0') { - char32_t u = utf8_to_unicode((unsigned char *) wbstate->str + - wbstate->offset); + char32_t u = utf8_to_unicode(str + wbstate->offset, + wbstate->len - wbstate->offset); bool curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index e5e494db43c..a760094104c 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -223,15 +223,19 @@ convert_case(char *dst, size_t dstsize, const char *src, ssize_t srclen, Assert((str_casekind == CaseTitle && wbnext && wbstate) || (str_casekind != CaseTitle && !wbnext && !wbstate)); + if (srclen < 0) + srclen = strlen(src); + if (str_casekind == CaseTitle) { boundary = wbnext(wbstate); Assert(boundary == 0); /* start of text is always a boundary */ } - while ((srclen < 0 || srcoff < srclen) && src[srcoff] != '\0') + while (srcoff < srclen) { - char32_t u1 = utf8_to_unicode((unsigned char *) src + srcoff); + char32_t u1 = utf8_to_unicode((unsigned char *) src + srcoff, + srclen - srcoff); int u1len = unicode_utf8len(u1); char32_t simple = 0; const char32_t *special = NULL; @@ -320,7 +324,7 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + char32_t curr = utf8_to_unicode(str + i, len - i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -344,7 +348,7 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + char32_t curr = utf8_to_unicode(str + i, len - i); if (pg_u_prop_case_ignorable(curr)) continue; 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)); } /* diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index 4d84bdc81e4..6dc0fff332f 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -558,22 +558,20 @@ surrogate_pair_to_codepoint(char16_t first, char16_t second) /* * Convert a UTF-8 character to a Unicode code point. * This is a one-character version of pg_utf2wchar_with_len. - * - * No error checks here, c must point to a long-enough string. */ static inline char32_t -utf8_to_unicode(const unsigned char *c) +utf8_to_unicode(const unsigned char *c, size_t len) { - if ((*c & 0x80) == 0) + if ((*c & 0x80) == 0 && len >= 1) return (char32_t) c[0]; - else if ((*c & 0xe0) == 0xc0) + else if ((*c & 0xe0) == 0xc0 && len >= 2) return (char32_t) (((c[0] & 0x1f) << 6) | (c[1] & 0x3f)); - else if ((*c & 0xf0) == 0xe0) + else if ((*c & 0xf0) == 0xe0 && len >= 3) return (char32_t) (((c[0] & 0x0f) << 12) | ((c[1] & 0x3f) << 6) | (c[2] & 0x3f)); - else if ((*c & 0xf8) == 0xf0) + else if ((*c & 0xf8) == 0xf0 && len >= 4) return (char32_t) (((c[0] & 0x07) << 18) | ((c[1] & 0x3f) << 12) | ((c[2] & 0x3f) << 6) | @@ -676,8 +674,6 @@ extern int pg_valid_server_encoding(const char *name); extern bool is_encoding_supported_by_icu(int encoding); extern const char *get_encoding_name_for_icu(int encoding); -extern unsigned char *unicode_to_utf8(char32_t c, unsigned char *utf8string); -extern char32_t utf8_to_unicode(const unsigned char *c); extern bool pg_utf8_islegal(const unsigned char *source, int length); extern int pg_utf_mblen(const unsigned char *s); extern int pg_mule_mblen(const unsigned char *s); -- 2.43.0 ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2025-12-13 23:22 Chao Li <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Chao Li @ 2025-12-13 23:22 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Dec 13, 2025, at 07:24, Jeff Davis <[email protected]> wrote: > > Attached. > > > <v1-0001-Make-utf8_to_unicode-safer.patch> This patch adds a length check to utf8_to_unicode(), I think which is where “safety” comes from. Can you please add an a little bit more to the commit message instead of only saying “improve safety”. 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) /* Decode UTF-8 character to ISO 10646 code point. */ str += *ix; - c = utf8_to_unicode(str); + /* Assume byte sequence has not been broken. */ + c = 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 “trust” 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/ ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2025-12-15 20:23 Jeff Davis <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2025-12-15 20:23 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers 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 “safety” comes from. Can you please add an a little bit more to > the commit message instead of only saying “improve safety”. Right: it does not read past pg_mblen(), nor past the supplied length. We could go further and check for valid continuation bytes, but the other routines don't do that. > It also deleted two redundant function declarations from pg_wchar.h, > which may also worth a quick note in the commit message. I committed that as an independent change and removed it from this patch. > + /* Assume byte sequence has not been broken. */ > + c = utf8_to_unicode(str, MAX_MULTIBYTE_CHAR_LEN); > ``` > > Here we need an empty line above the new comment. Done, and I expanded the comment to explain why it's safe. > 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 “trust” that? 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(). 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. Regards, Jeff Davis Attachments: [text/x-patch] v2-0001-Make-utf8_to_unicode-safer.patch (9.8K, ../../[email protected]/2-v2-0001-Make-utf8_to_unicode-safer.patch) download | inline diff: From 94a80385eec23dbb85cc601063cac04e8d845a52 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 4 Dec 2025 19:37:32 -0800 Subject: [PATCH v2] Make utf8_to_unicode() safer. Require the input string length so that it doesn't read past the end of a string if given an invalid (or truncated) byte sequence. Discussion: https://postgr.es/m/[email protected] --- contrib/fuzzystrmatch/daitch_mokotoff.c | 8 ++++++- src/backend/utils/adt/pg_locale_builtin.c | 5 +++-- src/backend/utils/adt/varlena.c | 26 +++++++++++------------ src/common/saslprep.c | 4 +++- src/common/unicode/case_test.c | 5 +++-- src/common/unicode_case.c | 12 +++++++---- src/common/wchar.c | 3 ++- src/include/mb/pg_wchar.h | 12 +++++------ 8 files changed, 43 insertions(+), 32 deletions(-) diff --git a/contrib/fuzzystrmatch/daitch_mokotoff.c b/contrib/fuzzystrmatch/daitch_mokotoff.c index 07f895ae2bf..103a29d89d4 100644 --- a/contrib/fuzzystrmatch/daitch_mokotoff.c +++ b/contrib/fuzzystrmatch/daitch_mokotoff.c @@ -401,7 +401,13 @@ read_char(const unsigned char *str, int *ix) /* Decode UTF-8 character to ISO 10646 code point. */ str += *ix; - c = utf8_to_unicode(str); + + /* + * We assume the string does not contain truncated byte sequences because + * it came from pg_server_to_any(..., PG_UTF8), so we can safely use + * MAX_MULTIBYTE_CHAR_LEN. + */ + c = utf8_to_unicode(str, MAX_MULTIBYTE_CHAR_LEN); /* Advance *ix, but (for safety) not if we've reached end of string. */ if (c) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 0c2920112bb..28fc55278b0 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -59,12 +59,13 @@ static size_t initcap_wbnext(void *state) { struct WordBoundaryState *wbstate = (struct WordBoundaryState *) state; + unsigned char *str = (unsigned char *) wbstate->str; while (wbstate->offset < wbstate->len && wbstate->str[wbstate->offset] != '\0') { - char32_t u = utf8_to_unicode((unsigned char *) wbstate->str + - wbstate->offset); + char32_t u = utf8_to_unicode(str + wbstate->offset, + wbstate->len - wbstate->offset); bool curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index baa5b44ea8d..d684370900d 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -5423,19 +5423,17 @@ Datum unicode_assigned(PG_FUNCTION_ARGS) { text *input = PG_GETARG_TEXT_PP(0); - unsigned char *p; - int size; + unsigned char *p = (unsigned char *) VARDATA_ANY(input); + unsigned char *p_end = p + VARSIZE_ANY_EXHDR(input); if (GetDatabaseEncoding() != PG_UTF8) ereport(ERROR, (errmsg("Unicode categorization can only be performed if server encoding is UTF8"))); /* convert to char32_t */ - size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); - p = (unsigned char *) VARDATA_ANY(input); - for (int i = 0; i < size; i++) + while (p < p_end) { - char32_t uchar = utf8_to_unicode(p); + char32_t uchar = utf8_to_unicode(p, p_end - p); int category = unicode_category(uchar); if (category == PG_U_UNASSIGNED) @@ -5456,7 +5454,8 @@ unicode_normalize_func(PG_FUNCTION_ARGS) int size; char32_t *input_chars; char32_t *output_chars; - unsigned char *p; + unsigned char *p = (unsigned char *) VARDATA_ANY(input); + unsigned char *p_end = p + VARSIZE_ANY_EXHDR(input); text *result; int i; @@ -5465,14 +5464,13 @@ unicode_normalize_func(PG_FUNCTION_ARGS) /* convert to char32_t */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); input_chars = palloc((size + 1) * sizeof(char32_t)); - p = (unsigned char *) VARDATA_ANY(input); for (i = 0; i < size; i++) { - input_chars[i] = utf8_to_unicode(p); + input_chars[i] = utf8_to_unicode(p, p_end - p); p += pg_utf_mblen(p); } input_chars[i] = (char32_t) '\0'; - Assert((char *) p == VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input)); + Assert(p == p_end); /* action */ output_chars = unicode_normalize(form, input_chars); @@ -5522,7 +5520,8 @@ unicode_is_normalized(PG_FUNCTION_ARGS) int size; char32_t *input_chars; char32_t *output_chars; - unsigned char *p; + unsigned char *p = (unsigned char *) VARDATA_ANY(input); + unsigned char *p_end = p + VARSIZE_ANY_EXHDR(input); int i; UnicodeNormalizationQC quickcheck; int output_size; @@ -5533,14 +5532,13 @@ unicode_is_normalized(PG_FUNCTION_ARGS) /* convert to char32_t */ size = pg_mbstrlen_with_len(VARDATA_ANY(input), VARSIZE_ANY_EXHDR(input)); input_chars = palloc((size + 1) * sizeof(char32_t)); - p = (unsigned char *) VARDATA_ANY(input); for (i = 0; i < size; i++) { - input_chars[i] = utf8_to_unicode(p); + input_chars[i] = utf8_to_unicode(p, p_end - p); p += pg_utf_mblen(p); } input_chars[i] = (char32_t) '\0'; - Assert((char *) p == VARDATA_ANY(input) + VARSIZE_ANY_EXHDR(input)); + Assert(p == p_end); /* quick check (see UAX #15) */ quickcheck = unicode_is_normalized_quickcheck(form, input_chars); diff --git a/src/common/saslprep.c b/src/common/saslprep.c index 101e8d65a4d..083702654b0 100644 --- a/src/common/saslprep.c +++ b/src/common/saslprep.c @@ -1055,6 +1055,7 @@ pg_saslprep(const char *input, char **output) int i; bool contains_RandALCat; unsigned char *p; + unsigned char *p_end; char32_t *wp; /* Ensure we return *output as NULL on failure */ @@ -1088,9 +1089,10 @@ pg_saslprep(const char *input, char **output) goto oom; p = (unsigned char *) input; + p_end = p + strlen(input); for (i = 0; i < input_size; i++) { - input_chars[i] = utf8_to_unicode(p); + input_chars[i] = utf8_to_unicode(p, p_end - p); p += pg_utf_mblen(p); } input_chars[i] = (char32_t) '\0'; diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index 00d4f85e5a5..e63ce2fbeb9 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -51,12 +51,13 @@ static size_t initcap_wbnext(void *state) { struct WordBoundaryState *wbstate = (struct WordBoundaryState *) state; + unsigned char *str = (unsigned char *) wbstate->str; while (wbstate->offset < wbstate->len && wbstate->str[wbstate->offset] != '\0') { - char32_t u = utf8_to_unicode((unsigned char *) wbstate->str + - wbstate->offset); + char32_t u = utf8_to_unicode(str + wbstate->offset, + wbstate->len - wbstate->offset); bool curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index e5e494db43c..a760094104c 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -223,15 +223,19 @@ convert_case(char *dst, size_t dstsize, const char *src, ssize_t srclen, Assert((str_casekind == CaseTitle && wbnext && wbstate) || (str_casekind != CaseTitle && !wbnext && !wbstate)); + if (srclen < 0) + srclen = strlen(src); + if (str_casekind == CaseTitle) { boundary = wbnext(wbstate); Assert(boundary == 0); /* start of text is always a boundary */ } - while ((srclen < 0 || srcoff < srclen) && src[srcoff] != '\0') + while (srcoff < srclen) { - char32_t u1 = utf8_to_unicode((unsigned char *) src + srcoff); + char32_t u1 = utf8_to_unicode((unsigned char *) src + srcoff, + srclen - srcoff); int u1len = unicode_utf8len(u1); char32_t simple = 0; const char32_t *special = NULL; @@ -320,7 +324,7 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + char32_t curr = utf8_to_unicode(str + i, len - i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -344,7 +348,7 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + char32_t curr = utf8_to_unicode(str + i, len - i); if (pg_u_prop_case_ignorable(curr)) continue; diff --git a/src/common/wchar.c b/src/common/wchar.c index a4bc29921de..a104234ae92 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)); + /* input must be a valid UTF-8 sequence */ + return ucs_wcwidth(utf8_to_unicode(s, MAX_MULTIBYTE_CHAR_LEN)); } /* diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index 5df67ceea87..6dc0fff332f 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -558,22 +558,20 @@ surrogate_pair_to_codepoint(char16_t first, char16_t second) /* * Convert a UTF-8 character to a Unicode code point. * This is a one-character version of pg_utf2wchar_with_len. - * - * No error checks here, c must point to a long-enough string. */ static inline char32_t -utf8_to_unicode(const unsigned char *c) +utf8_to_unicode(const unsigned char *c, size_t len) { - if ((*c & 0x80) == 0) + if ((*c & 0x80) == 0 && len >= 1) return (char32_t) c[0]; - else if ((*c & 0xe0) == 0xc0) + else if ((*c & 0xe0) == 0xc0 && len >= 2) return (char32_t) (((c[0] & 0x1f) << 6) | (c[1] & 0x3f)); - else if ((*c & 0xf0) == 0xe0) + else if ((*c & 0xf0) == 0xe0 && len >= 3) return (char32_t) (((c[0] & 0x0f) << 12) | ((c[1] & 0x3f) << 6) | (c[2] & 0x3f)); - else if ((*c & 0xf8) == 0xf0) + else if ((*c & 0xf8) == 0xf0 && len >= 4) return (char32_t) (((c[0] & 0x07) << 18) | ((c[1] & 0x3f) << 12) | ((c[2] & 0x3f) << 6) | -- 2.43.0 ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2025-12-15 23:34 Chao Li <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Chao Li @ 2025-12-15 23:34 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Dec 16, 2025, at 04:23, Jeff Davis <[email protected]> wrote: > > 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 “safety” comes from. Can you please add an a little bit more to >> the commit message instead of only saying “improve safety”. > > Right: it does not read past pg_mblen(), nor past the supplied length. > > We could go further and check for valid continuation bytes, but the > other routines don't do that. > >> It also deleted two redundant function declarations from pg_wchar.h, >> which may also worth a quick note in the commit message. > > I committed that as an independent change and removed it from this > patch. > >> + /* Assume byte sequence has not been broken. */ >> + c = utf8_to_unicode(str, MAX_MULTIBYTE_CHAR_LEN); >> ``` >> >> Here we need an empty line above the new comment. > > Done, and I expanded the comment to explain why it's safe. > >> 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 “trust” that? > > 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(). > > 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. > > Regards, > Jeff Davis > > > <v2-0001-Make-utf8_to_unicode-safer.patch> V2 LGTM. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2025-12-17 19:37 Jeff Davis <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2025-12-17 19:37 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers On Tue, 2025-12-16 at 07:34 +0800, Chao Li wrote: > > <v2-0001-Make-utf8_to_unicode-safer.patch> > > V2 LGTM. On second thought, if we're going to change something here, we should probably have a more flexible API for both utf8_to_unicode() and unicode_to_utf8(). Looking at the callers, I think we want to have signatures something like: /* returns number of bytes consumed, or -1 */ static inline ssize_t utf8_to_unicode(char32_t *cp, const unsigned char *src, size_t srclen) { ... } /* returns number of bytes written, or -1 */ static inline ssize_t unicode_to_utf8(unsigned char *dst, size_t dstsize, char32_t cp) { ... } That would make both APIs safer, and the caller wouldn't need to call unicode_utf8len() or pg_utf8_mblen() separately. We could also do more validation, but of course then the callers would need to do something if they encounter a failure. We could also try to catch NUL terminators in the middle of a sequence, which might be useful. Regards, Jeff Davis ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-19 23:22 Jeff Davis <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2026-06-19 23:22 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers On Wed, 2025-12-17 at 11:37 -0800, Jeff Davis wrote: > On Tue, 2025-12-16 at 07:34 +0800, Chao Li wrote: > > > <v2-0001-Make-utf8_to_unicode-safer.patch> > > > > V2 LGTM. > > On second thought, if we're going to change something here, we should > probably have a more flexible API for both utf8_to_unicode() and > unicode_to_utf8(). New series: 0001: validates UTF8 before calling into unicode_case.c. Extra defense, and simple to backport, but regresses performance of those functions. It also might risk errors if somehow there is invalid UTF8. 0002: refactors to create an error path from unicode_case.c into pg_locale_builtin.c, where a proper error can be thrown. This wins back the performance lost in the previous commit. This is perhaps backportable, but technically it changes an exported function signature, so carries some very low risk. 0003: Adds utf8encode() and utf8decode(), which are iteration-friendly and inlinable, and fully-validate UTF8 (e.g. rejects surrogate halves). This is an enhancement so should not be backported. 0004: Make use of new API from unicode_case.c. Regards, Jeff Davis Attachments: [text/x-patch] v3-0001-unicode_case.c-ensure-valid-UTF8.patch (1.8K, ../../[email protected]/2-v3-0001-unicode_case.c-ensure-valid-UTF8.patch) download | inline diff: From 4d59a316a147d68ff0113cec6b969037d2ee169e Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Fri, 19 Jun 2026 14:09:31 -0700 Subject: [PATCH v3 1/4] unicode_case.c: ensure valid UTF8. Should be valid, but check before calling unicode_strlower(), etc. Discussion: https://postgr.es/m/[email protected] Reviewed-by: Chao Li <[email protected]> Backpatch-through: 17 --- src/backend/utils/adt/pg_locale_builtin.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 01d4f55b07e..7f167e751ea 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -86,6 +86,7 @@ static size_t strlower_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { + pg_verifymbstr(src, srclen, false); return unicode_strlower(dest, destsize, src, srclen, locale->builtin.casemap_full); } @@ -103,6 +104,7 @@ strtitle_builtin(char *dest, size_t destsize, const char *src, size_t srclen, .prev_alnum = false, }; + pg_verifymbstr(src, srclen, false); return unicode_strtitle(dest, destsize, src, srclen, locale->builtin.casemap_full, initcap_wbnext, &wbstate); @@ -112,6 +114,7 @@ static size_t strupper_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { + pg_verifymbstr(src, srclen, false); return unicode_strupper(dest, destsize, src, srclen, locale->builtin.casemap_full); } @@ -120,6 +123,7 @@ static size_t strfold_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { + pg_verifymbstr(src, srclen, false); return unicode_strfold(dest, destsize, src, srclen, locale->builtin.casemap_full); } -- 2.43.0 [text/x-patch] v3-0002-Move-UTF8-checks-into-unicode_case.c.patch (15.0K, ../../[email protected]/3-v3-0002-Move-UTF8-checks-into-unicode_case.c.patch) download | inline diff: From fffa7153f563a19663a02e44196f377b83bf217f Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 16 Apr 2026 14:56:11 -0700 Subject: [PATCH v3 2/4] Move UTF8 checks into unicode_case.c. Pre-checking UTF-8 is inefficient. Refactor the error paths so we can catch UTF-8 errors while iterating, and return back to pg_locale_builtin.c where we can throw the error. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 85 +++++++++++++++------ src/common/unicode/case_test.c | 33 +++++--- src/common/unicode_case.c | 92 ++++++++++++++++------- src/include/common/unicode_case.h | 8 +- 4 files changed, 156 insertions(+), 62 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 7f167e751ea..96da9c6fcf3 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,21 +62,32 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - char32_t u = utf8_to_unicode((const unsigned char *) wbstate->str + + int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + wbstate->offset); - bool curr_alnum = pg_u_isalnum(u, wbstate->posix); + char32_t u; + bool curr_alnum; + + if (wbstate->offset + ulen > wbstate->len) + { + wbstate->offset = wbstate->len; + return wbstate->len; + } + + u = utf8_to_unicode((const unsigned char *) wbstate->str + + wbstate->offset); + curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) { size_t prev_offset = wbstate->offset; wbstate->init = true; - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; return prev_offset; } - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; } return wbstate->len; @@ -86,9 +97,16 @@ static size_t strlower_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - pg_verifymbstr(src, srclen, false); - return unicode_strlower(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strlower(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t @@ -96,36 +114,57 @@ strtitle_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { struct WordBoundaryState wbstate = { - .str = src, - .len = srclen, - .offset = 0, - .posix = !locale->builtin.casemap_full, - .init = false, - .prev_alnum = false, + .str = src, + .len = srclen, + .offset = 0, + .posix = !locale->builtin.casemap_full, + .init = false, + .prev_alnum = false, }; + size_t consumed; + size_t result; + + result = unicode_strtitle(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full, + initcap_wbnext, &wbstate); - pg_verifymbstr(src, srclen, false); - return unicode_strtitle(dest, destsize, src, srclen, - locale->builtin.casemap_full, - initcap_wbnext, &wbstate); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strupper_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - pg_verifymbstr(src, srclen, false); - return unicode_strupper(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strupper(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strfold_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - pg_verifymbstr(src, srclen, false); - return unicode_strfold(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strfold(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static bool diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index a0dbf00b671..ae0f86ffa0c 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -115,6 +115,7 @@ icu_test_full(char *str) char icu_fold[BUFSZ]; UErrorCode status; size_t len = strlen(str); + size_t consumed; /* full case mapping doesn't use posix semantics */ struct WordBoundaryState wbstate = { @@ -126,10 +127,10 @@ icu_test_full(char *str) .prev_alnum = false, }; - unicode_strlower(lower, BUFSZ, str, len, true); - unicode_strtitle(title, BUFSZ, str, len, true, initcap_wbnext, &wbstate); - unicode_strupper(upper, BUFSZ, str, len, true); - unicode_strfold(fold, BUFSZ, str, len, true); + unicode_strlower(lower, BUFSZ, str, len, &consumed, true); + unicode_strtitle(title, BUFSZ, str, len, &consumed, true, initcap_wbnext, &wbstate); + unicode_strupper(upper, BUFSZ, str, len, &consumed, true); + unicode_strfold(fold, BUFSZ, str, len, &consumed, true); status = U_ZERO_ERROR; ucasemap_utf8ToLower(casemap, icu_lower, BUFSZ, str, len, &status); status = U_ZERO_ERROR; @@ -260,13 +261,16 @@ static size_t tfunc_lower(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strlower(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strlower(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_title(char *dst, size_t dstsize, const char *src, size_t srclen) { + size_t consumed; struct WordBoundaryState wbstate = { .str = src, .len = srclen, @@ -275,27 +279,34 @@ tfunc_title(char *dst, size_t dstsize, const char *src, .prev_alnum = false, }; - return unicode_strtitle(dst, dstsize, src, srclen, true, initcap_wbnext, - &wbstate); + return unicode_strtitle(dst, dstsize, src, srclen, &consumed, true, + initcap_wbnext, &wbstate); } static size_t tfunc_upper(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strupper(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strupper(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_fold(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strfold(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strfold(dst, dstsize, src, srclen, &consumed, true); } static void test_convert_case(void) { + size_t needed; + size_t consumed; + /* test string with no case changes */ test_convert(tfunc_lower, "√∞", "√∞"); /* test adjust-to-cased behavior */ @@ -320,6 +331,10 @@ test_convert_case(void) /* U+FF11 FULLWIDTH ONE is alphanumeric for full case mapping */ test_convert(tfunc_title, "\uFF11a", "\uFF11a"); + /* invalid UTF8 */ + needed = unicode_strfold(NULL, 0, "abc\xCE", 4, &consumed, false); + Assert(consumed == 3); + Assert(needed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index d6ee00b7d9c..4a692cfa249 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -40,8 +40,8 @@ static const char32_t *const casekind_map[NCaseKind] = static char32_t find_case_map(char32_t ucs, const char32_t *map); static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate); + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate); static enum CaseMapResult casemap(char32_t u1, CaseKind casekind, bool full, const char *src, size_t srclen, size_t srcoff, char32_t *simple, const char32_t **special); @@ -82,7 +82,8 @@ unicode_casefold_simple(char32_t code) * unicode_strlower() * * Convert src to lowercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -98,17 +99,18 @@ unicode_casefold_simple(char32_t code) */ size_t unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseLower, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseLower, full, + NULL, NULL); } /* * unicode_strtitle() * * Convert src to titlecase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -134,17 +136,19 @@ unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full, WordBoundaryNext wbnext, void *wbstate) + size_t *pconsumed, bool full, WordBoundaryNext wbnext, + void *wbstate) { - return convert_case(dst, dstsize, src, srclen, CaseTitle, full, wbnext, - wbstate); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseTitle, full, + wbnext, wbstate); } /* * unicode_strupper() * * Convert src to uppercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -160,17 +164,18 @@ unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseUpper, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseUpper, full, + NULL, NULL); } /* * unicode_strfold() * * Case fold src, and return the result length (not including terminating - * NUL). + * NUL). Sets *pconsumed to the amount of src successfully consumed; if less + * than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -183,10 +188,26 @@ unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseFold, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseFold, full, + NULL, NULL); +} + +/* local version of pg_utf_mblen() to be inlinable */ +static int +utf8_mblen(const unsigned char *s) +{ + if ((*s & 0x80) == 0) + return 1; + else if ((*s & 0xe0) == 0xc0) + return 2; + else if ((*s & 0xf0) == 0xe0) + return 3; + else if ((*s & 0xf8) == 0xf0) + return 4; + else + return -1; } /* @@ -207,8 +228,8 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, */ static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate) + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate) { /* character CaseKind varies while titlecasing */ CaseKind chr_casekind = str_casekind; @@ -227,12 +248,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - char32_t u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - int u1len = unicode_utf8len(u1); + int u1len = utf8_mblen((const unsigned char *) src + srcoff); + char32_t u1; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + /* invalid UTF8 */ + if (u1len < 0 || srcoff + u1len > srclen) + break; + + u1 = utf8_to_unicode((const unsigned char *) src + srcoff); + if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -293,6 +320,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, if (result_len < dstsize) dst[result_len] = '\0'; + *pconsumed = srcoff; return result_len; } @@ -316,7 +344,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -327,8 +362,6 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ } /* end of string is not followed by a Cased character */ @@ -340,7 +373,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; diff --git a/src/include/common/unicode_case.h b/src/include/common/unicode_case.h index 03add78cabe..1cbc0c14bc2 100644 --- a/src/include/common/unicode_case.h +++ b/src/include/common/unicode_case.h @@ -21,13 +21,13 @@ char32_t unicode_titlecase_simple(char32_t code); char32_t unicode_uppercase_simple(char32_t code); char32_t unicode_casefold_simple(char32_t code); size_t unicode_strlower(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full, + size_t srclen, size_t *pconsumed, bool full, WordBoundaryNext wbnext, void *wbstate); size_t unicode_strupper(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strfold(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); #endif /* UNICODE_CASE_H */ -- 2.43.0 [text/x-patch] v3-0003-Validating-iterator-friendly-UTF8-encoder-decoder.patch (5.3K, ../../[email protected]/4-v3-0003-Validating-iterator-friendly-UTF8-encoder-decoder.patch) download | inline diff: From ba582fcb653d27110675831f29b3b088609c02ff Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 4 Jun 2026 12:08:51 -0700 Subject: [PATCH v3 3/4] Validating, iterator-friendly UTF8 encoder/decoder API. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/include/mb/pg_wchar.h | 160 +++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 2 deletions(-) diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index deee2a832c3..d8ea77c3fe0 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -27,6 +27,11 @@ */ typedef unsigned int pg_wchar; +/* + * Returned for decoding failures in utf8decode() and utf8_to_unicode(). + */ +#define PG_INVALID_CODEPOINT 0xFFFFFFFF + /* * Maximum byte length of multibyte characters in any backend encoding */ @@ -392,11 +397,161 @@ surrogate_pair_to_codepoint(char16_t first, char16_t second) return ((first & 0x3FF) << 10) + 0x10000 + (second & 0x3FF); } +/* + * Encode the codepoint as UTF8 and return the number of bytes required. If + * the number of bytes required exceeds dstsize, just return the number of + * bytes required without modifying dst. If dstsize is zero, dst may be + * NULL. If codepoint is not a valid Unicode Scalar, return -1. + */ +static inline int +utf8encode(unsigned char *dst, size_t dstsize, char32_t codepoint) +{ + int nbytes; + + if (codepoint <= 0x7F) + nbytes = 1; + else if (codepoint <= 0x7FF) + nbytes = 2; + else if (codepoint <= 0xFFFF) + { + /* surrogate halves not valid for UTF8 */ + if (codepoint >= 0xD800 && codepoint <= 0xDFFF) + return -1; + nbytes = 3; + } + else if (codepoint <= 0x10FFFF) + nbytes = 4; + else + return -1; + + if (nbytes > dstsize) + return nbytes; + + if (codepoint <= 0x7F) + { + dst[0] = codepoint; + } + else if (codepoint <= 0x7FF) + { + dst[0] = 0xC0 | ((codepoint >> 6) & 0x1F); + dst[1] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0xFFFF) + { + dst[0] = 0xE0 | ((codepoint >> 12) & 0x0F); + dst[1] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[2] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0x10FFFF) + { + dst[0] = 0xF0 | ((codepoint >> 18) & 0x07); + dst[1] = 0x80 | ((codepoint >> 12) & 0x3F); + dst[2] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[3] = 0x80 | (codepoint & 0x3F); + } + + return nbytes; +} + +/* + * Decode the next Unicode codepoint from UTF8 at src, reading no more than + * srclen bytes (which must be at least 1). On success, *pcodepoint will be a + * valid Unicode Scalar; otherwise it will be set to PG_INVALID_CODEPOINT. + * + * Returns the number of bytes consumed. If srclen is not large enough + * (i.e. src is truncated in the middle of a sequence), returns 0. If invalid, + * returns -1. + */ +static inline int +utf8decode(char32_t *pcodepoint, const unsigned char *src, size_t srclen) +{ + int nbytes; + char32_t codepoint; + + Assert(srclen >= 1); + + if ((*src & 0x80) == 0) + { + *pcodepoint = (char32_t) src[0]; + return 1; + } + + if ((*src & 0xe0) == 0xc0) + nbytes = 2; + else if ((*src & 0xf0) == 0xe0) + nbytes = 3; + else if ((*src & 0xf8) == 0xf0) + nbytes = 4; + else + goto invalid; + + /* truncated */ + if (srclen < nbytes) + { + *pcodepoint = PG_INVALID_CODEPOINT; + return 0; + } + + if (nbytes == 2) + { + /* check continuation byte */ + if ((src[1] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x1f) << 6) | + (src[1] & 0x3f)); + + /* overlong */ + if (codepoint < 0x0080) + goto invalid; + } + else if (nbytes == 3) + { + /* check continuation bytes */ + if ((src[1] & 0xc0) != 0x80 || (src[2] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x0f) << 12) | + ((src[1] & 0x3f) << 6) | + (src[2] & 0x3f)); + + /* overlong or surrogate half */ + if (codepoint < 0x0800 || + (codepoint >= 0xD800 && codepoint <= 0xDFFF)) + goto invalid; + } + else if (nbytes == 4) + { + /* check continuation bytes */ + if ((src[1] & 0xc0) != 0x80 || (src[2] & 0xc0) != 0x80 || + (src[3] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x07) << 18) | + ((src[1] & 0x3f) << 12) | + ((src[2] & 0x3f) << 6) | + (src[3] & 0x3f)); + + /* overlong or out-of-range */ + if (codepoint < 0x10000 || codepoint > 0x10FFFF) + goto invalid; + } + + *pcodepoint = codepoint; + return nbytes; + +invalid: + *pcodepoint = PG_INVALID_CODEPOINT; + return -1; +} + /* * Convert a UTF-8 character to a Unicode code point. * This is a one-character version of pg_utf2wchar_with_len. * * No error checks here, c must point to a long-enough string. + * + * XXX: Callers should consider utf8decode() instead. */ static inline char32_t utf8_to_unicode(const unsigned char *c) @@ -416,13 +571,14 @@ utf8_to_unicode(const unsigned char *c) ((c[2] & 0x3f) << 6) | (c[3] & 0x3f)); else - /* that is an invalid code on purpose */ - return 0xffffffff; + return PG_INVALID_CODEPOINT; } /* * Map a Unicode code point to UTF-8. utf8string must have at least * unicode_utf8len(c) bytes available. + * + * XXX: Callers should consider utf8encode() instead. */ static inline unsigned char * unicode_to_utf8(char32_t c, unsigned char *utf8string) -- 2.43.0 [text/x-patch] v3-0004-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch (6.4K, ../../[email protected]/5-v3-0004-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch) download | inline diff: From 942f374a4383502cbb66d0d83f0ba317962a9a24 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Fri, 19 Jun 2026 14:58:47 -0700 Subject: [PATCH v3 4/4] unicode_case.c: use new utf8encode/utf8decode APIs. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 17 +++--- src/common/unicode/case_test.c | 2 +- src/common/unicode_case.c | 70 +++++++++++------------ 3 files changed, 44 insertions(+), 45 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 96da9c6fcf3..a826ec1cfa8 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,25 +62,26 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + - wbstate->offset); + int ulen; char32_t u; bool curr_alnum; + size_t prev_offset = wbstate->offset; - if (wbstate->offset + ulen > wbstate->len) + ulen = utf8decode(&u, (const unsigned char *) wbstate->str + wbstate->offset, + wbstate->len - wbstate->offset); + + /* invalid UTF8 */ + if (ulen <= 0) { + wbstate->init = true; wbstate->offset = wbstate->len; - return wbstate->len; + return prev_offset; } - u = utf8_to_unicode((const unsigned char *) wbstate->str + - wbstate->offset); curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) { - size_t prev_offset = wbstate->offset; - wbstate->init = true; wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index ae0f86ffa0c..71a6f2bbe70 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -179,7 +179,7 @@ test_icu(void) { pg_unicode_category category = unicode_category(code); - if (category != PG_U_UNASSIGNED) + if (category != PG_U_UNASSIGNED && category != PG_U_SURROGATE) { uint8_t icu_category = u_charType(code); char code_str[5] = {0}; diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 4a692cfa249..d89f5ca4740 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -194,22 +194,6 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL, NULL); } -/* local version of pg_utf_mblen() to be inlinable */ -static int -utf8_mblen(const unsigned char *s) -{ - if ((*s & 0x80) == 0) - return 1; - else if ((*s & 0xe0) == 0xc0) - return 2; - else if ((*s & 0xf0) == 0xe0) - return 3; - else if ((*s & 0xf8) == 0xf0) - return 4; - else - return -1; -} - /* * Implement Unicode Default Case Conversion algorithm. * @@ -248,18 +232,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - int u1len = utf8_mblen((const unsigned char *) src + srcoff); char32_t u1; + int u1len; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + u1len = utf8decode(&u1, (const unsigned char *) src + srcoff, + srclen - srcoff); + /* invalid UTF8 */ - if (u1len < 0 || srcoff + u1len > srclen) + if (u1len <= 0) break; - u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -280,6 +265,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, /* no mapping; copy bytes from src */ Assert(simple == 0); Assert(special == NULL); + if (result_len + u1len <= dstsize) memcpy(dst + result_len, src + srcoff, u1len); @@ -289,11 +275,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, { /* replace with single character */ char32_t u2 = simple; - char32_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } Assert(special == NULL); - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); result_len += u2len; } @@ -304,10 +297,17 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, for (int i = 0; i < MAX_CASE_EXPANSION && special[i]; i++) { char32_t u2 = special[i]; - size_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } + + u2len = utf8encode(p, remaining, u2); result_len += u2len; } @@ -344,15 +344,15 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - int u1len = utf8_mblen((const unsigned char *) str + i); + int u1len; char32_t curr; + u1len = utf8decode(&curr, (const unsigned char *) str + i, len - i); + /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) + if (u1len <= 0) return false; - curr = utf8_to_unicode(str + i); - if (pg_u_prop_case_ignorable(curr)) continue; else if (pg_u_prop_cased(curr)) @@ -373,15 +373,15 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - int u1len = utf8_mblen((const unsigned char *) str + i); + int u1len; char32_t curr; + u1len = utf8decode(&curr, (const unsigned char *) str + i, len - i); + /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) + if (u1len <= 0) return false; - curr = utf8_to_unicode(str + i); - if (pg_u_prop_case_ignorable(curr)) continue; else if (pg_u_prop_cased(curr)) @@ -391,8 +391,6 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ } return true; -- 2.43.0 ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-23 02:02 Jeff Davis <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2026-06-23 02:02 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers On Fri, 2026-06-19 at 16:22 -0700, Jeff Davis wrote: > On Wed, 2025-12-17 at 11:37 -0800, Jeff Davis wrote: > > On Tue, 2025-12-16 at 07:34 +0800, Chao Li wrote: > > > > <v2-0001-Make-utf8_to_unicode-safer.patch> > > > > > > V2 LGTM. > > > > On second thought, if we're going to change something here, we > > should > > probably have a more flexible API for both utf8_to_unicode() and > > unicode_to_utf8(). v4 attached. The main difference is that the first patch is more backportable. For backbranches, I think the safest thing if we encounter invalid UTF8 is to just terminate and return early. In master, we can change the API to properly return the error upward. Performance is not affected much, though in my brief tests it appeared that 0002 lost a bit and then 0004 gained it back. But we gain full UTF8 validation and safer UTF8 iterator APIs. Regards, Jeff Davis Attachments: [text/x-patch] v4-0001-unicode_case.c-defend-against-invalid-UTF8.patch (4.6K, ../../[email protected]/2-v4-0001-unicode_case.c-defend-against-invalid-UTF8.patch) download | inline diff: From 3ccf14e0d6234ab4fcf24cf56f53f512649bc658 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Mon, 22 Jun 2026 16:30:01 -0700 Subject: [PATCH v4 1/4] unicode_case.c: defend against invalid UTF8. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] Backpatch-through: 17 --- src/backend/utils/adt/pg_locale_builtin.c | 24 ++++++++--- src/common/unicode_case.c | 52 +++++++++++++++++++---- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 01d4f55b07e..7c36fd5091b 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,21 +62,33 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - char32_t u = utf8_to_unicode((const unsigned char *) wbstate->str + + int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + wbstate->offset); - bool curr_alnum = pg_u_isalnum(u, wbstate->posix); + char32_t u; + bool curr_alnum; + size_t prev_offset = wbstate->offset; - if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + /* invalid UTF8 */ + if (wbstate->offset + ulen > wbstate->len) { - size_t prev_offset = wbstate->offset; + wbstate->init = true; + wbstate->offset = wbstate->len; + return prev_offset; + } + u = utf8_to_unicode((const unsigned char *) wbstate->str + + wbstate->offset); + curr_alnum = pg_u_isalnum(u, wbstate->posix); + + if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + { wbstate->init = true; - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; return prev_offset; } - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; } return wbstate->len; diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index d6ee00b7d9c..42eb7d22211 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -189,6 +189,22 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL); } +/* local version of pg_utf_mblen() to be inlinable */ +static int +utf8_mblen(const unsigned char *s) +{ + if ((*s & 0x80) == 0) + return 1; + else if ((*s & 0xe0) == 0xc0) + return 2; + else if ((*s & 0xf0) == 0xe0) + return 3; + else if ((*s & 0xf8) == 0xf0) + return 4; + else + return -1; +} + /* * Implement Unicode Default Case Conversion algorithm. * @@ -227,12 +243,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - char32_t u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - int u1len = unicode_utf8len(u1); + int u1len = utf8_mblen((const unsigned char *) src + srcoff); + char32_t u1; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + /* invalid UTF8 */ + if (u1len < 0 || srcoff + u1len > srclen) + break; + + u1 = utf8_to_unicode((const unsigned char *) src + srcoff); + if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -316,7 +338,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -327,8 +356,8 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ + else + return false; /* invalid UTF8 */ } /* end of string is not followed by a Cased character */ @@ -340,7 +369,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -351,8 +387,8 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ + else + return false; /* invalid UTF8 */ } return true; -- 2.43.0 [text/x-patch] v4-0002-unicode_case.c-change-API-to-signal-UTF8-decoding.patch (11.5K, ../../[email protected]/3-v4-0002-unicode_case.c-change-API-to-signal-UTF8-decoding.patch) download | inline diff: From d3621a42037e91a02b24c8974bd48ce9a42febf4 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Mon, 22 Jun 2026 16:31:38 -0700 Subject: [PATCH v4 2/4] unicode_case.c: change API to signal UTF8 decoding error. Errors at this point are not expected, but if encountered, signal to the caller so it can raise the appropriate error. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 50 +++++++++++++++++++---- src/common/unicode/case_test.c | 33 +++++++++++---- src/common/unicode_case.c | 46 ++++++++++++--------- src/include/common/unicode_case.h | 8 ++-- 4 files changed, 95 insertions(+), 42 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 7c36fd5091b..5619daf43c3 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -98,8 +98,16 @@ static size_t strlower_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strlower(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strlower(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t @@ -114,26 +122,50 @@ strtitle_builtin(char *dest, size_t destsize, const char *src, size_t srclen, .init = false, .prev_alnum = false, }; + size_t consumed; + size_t result; - return unicode_strtitle(dest, destsize, src, srclen, - locale->builtin.casemap_full, - initcap_wbnext, &wbstate); + result = unicode_strtitle(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full, + initcap_wbnext, &wbstate); + + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strupper_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strupper(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strupper(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strfold_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strfold(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strfold(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static bool diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index a0dbf00b671..ae0f86ffa0c 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -115,6 +115,7 @@ icu_test_full(char *str) char icu_fold[BUFSZ]; UErrorCode status; size_t len = strlen(str); + size_t consumed; /* full case mapping doesn't use posix semantics */ struct WordBoundaryState wbstate = { @@ -126,10 +127,10 @@ icu_test_full(char *str) .prev_alnum = false, }; - unicode_strlower(lower, BUFSZ, str, len, true); - unicode_strtitle(title, BUFSZ, str, len, true, initcap_wbnext, &wbstate); - unicode_strupper(upper, BUFSZ, str, len, true); - unicode_strfold(fold, BUFSZ, str, len, true); + unicode_strlower(lower, BUFSZ, str, len, &consumed, true); + unicode_strtitle(title, BUFSZ, str, len, &consumed, true, initcap_wbnext, &wbstate); + unicode_strupper(upper, BUFSZ, str, len, &consumed, true); + unicode_strfold(fold, BUFSZ, str, len, &consumed, true); status = U_ZERO_ERROR; ucasemap_utf8ToLower(casemap, icu_lower, BUFSZ, str, len, &status); status = U_ZERO_ERROR; @@ -260,13 +261,16 @@ static size_t tfunc_lower(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strlower(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strlower(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_title(char *dst, size_t dstsize, const char *src, size_t srclen) { + size_t consumed; struct WordBoundaryState wbstate = { .str = src, .len = srclen, @@ -275,27 +279,34 @@ tfunc_title(char *dst, size_t dstsize, const char *src, .prev_alnum = false, }; - return unicode_strtitle(dst, dstsize, src, srclen, true, initcap_wbnext, - &wbstate); + return unicode_strtitle(dst, dstsize, src, srclen, &consumed, true, + initcap_wbnext, &wbstate); } static size_t tfunc_upper(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strupper(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strupper(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_fold(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strfold(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strfold(dst, dstsize, src, srclen, &consumed, true); } static void test_convert_case(void) { + size_t needed; + size_t consumed; + /* test string with no case changes */ test_convert(tfunc_lower, "√∞", "√∞"); /* test adjust-to-cased behavior */ @@ -320,6 +331,10 @@ test_convert_case(void) /* U+FF11 FULLWIDTH ONE is alphanumeric for full case mapping */ test_convert(tfunc_title, "\uFF11a", "\uFF11a"); + /* invalid UTF8 */ + needed = unicode_strfold(NULL, 0, "abc\xCE", 4, &consumed, false); + Assert(consumed == 3); + Assert(needed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 42eb7d22211..79f8c7f14a7 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -40,8 +40,8 @@ static const char32_t *const casekind_map[NCaseKind] = static char32_t find_case_map(char32_t ucs, const char32_t *map); static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate); + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate); static enum CaseMapResult casemap(char32_t u1, CaseKind casekind, bool full, const char *src, size_t srclen, size_t srcoff, char32_t *simple, const char32_t **special); @@ -82,7 +82,8 @@ unicode_casefold_simple(char32_t code) * unicode_strlower() * * Convert src to lowercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -98,17 +99,18 @@ unicode_casefold_simple(char32_t code) */ size_t unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseLower, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseLower, full, + NULL, NULL); } /* * unicode_strtitle() * * Convert src to titlecase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -134,17 +136,19 @@ unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full, WordBoundaryNext wbnext, void *wbstate) + size_t *pconsumed, bool full, WordBoundaryNext wbnext, + void *wbstate) { - return convert_case(dst, dstsize, src, srclen, CaseTitle, full, wbnext, - wbstate); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseTitle, full, + wbnext, wbstate); } /* * unicode_strupper() * * Convert src to uppercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -160,17 +164,18 @@ unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseUpper, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseUpper, full, + NULL, NULL); } /* * unicode_strfold() * * Case fold src, and return the result length (not including terminating - * NUL). + * NUL). Sets *pconsumed to the amount of src successfully consumed; if less + * than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -183,10 +188,10 @@ unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseFold, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseFold, full, + NULL, NULL); } /* local version of pg_utf_mblen() to be inlinable */ @@ -223,8 +228,8 @@ utf8_mblen(const unsigned char *s) */ static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate) + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate) { /* character CaseKind varies while titlecasing */ CaseKind chr_casekind = str_casekind; @@ -315,6 +320,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, if (result_len < dstsize) dst[result_len] = '\0'; + *pconsumed = srcoff; return result_len; } diff --git a/src/include/common/unicode_case.h b/src/include/common/unicode_case.h index 03add78cabe..1cbc0c14bc2 100644 --- a/src/include/common/unicode_case.h +++ b/src/include/common/unicode_case.h @@ -21,13 +21,13 @@ char32_t unicode_titlecase_simple(char32_t code); char32_t unicode_uppercase_simple(char32_t code); char32_t unicode_casefold_simple(char32_t code); size_t unicode_strlower(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full, + size_t srclen, size_t *pconsumed, bool full, WordBoundaryNext wbnext, void *wbstate); size_t unicode_strupper(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strfold(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); #endif /* UNICODE_CASE_H */ -- 2.43.0 [text/x-patch] v4-0003-Validating-iterator-friendly-UTF8-encoder-decoder.patch (5.3K, ../../[email protected]/4-v4-0003-Validating-iterator-friendly-UTF8-encoder-decoder.patch) download | inline diff: From 3ef15be4a2ad8e99d9f8b6d757c17044166c5b8d Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 4 Jun 2026 12:08:51 -0700 Subject: [PATCH v4 3/4] Validating, iterator-friendly UTF8 encoder/decoder API. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/include/mb/pg_wchar.h | 160 +++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 2 deletions(-) diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index deee2a832c3..d8ea77c3fe0 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -27,6 +27,11 @@ */ typedef unsigned int pg_wchar; +/* + * Returned for decoding failures in utf8decode() and utf8_to_unicode(). + */ +#define PG_INVALID_CODEPOINT 0xFFFFFFFF + /* * Maximum byte length of multibyte characters in any backend encoding */ @@ -392,11 +397,161 @@ surrogate_pair_to_codepoint(char16_t first, char16_t second) return ((first & 0x3FF) << 10) + 0x10000 + (second & 0x3FF); } +/* + * Encode the codepoint as UTF8 and return the number of bytes required. If + * the number of bytes required exceeds dstsize, just return the number of + * bytes required without modifying dst. If dstsize is zero, dst may be + * NULL. If codepoint is not a valid Unicode Scalar, return -1. + */ +static inline int +utf8encode(unsigned char *dst, size_t dstsize, char32_t codepoint) +{ + int nbytes; + + if (codepoint <= 0x7F) + nbytes = 1; + else if (codepoint <= 0x7FF) + nbytes = 2; + else if (codepoint <= 0xFFFF) + { + /* surrogate halves not valid for UTF8 */ + if (codepoint >= 0xD800 && codepoint <= 0xDFFF) + return -1; + nbytes = 3; + } + else if (codepoint <= 0x10FFFF) + nbytes = 4; + else + return -1; + + if (nbytes > dstsize) + return nbytes; + + if (codepoint <= 0x7F) + { + dst[0] = codepoint; + } + else if (codepoint <= 0x7FF) + { + dst[0] = 0xC0 | ((codepoint >> 6) & 0x1F); + dst[1] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0xFFFF) + { + dst[0] = 0xE0 | ((codepoint >> 12) & 0x0F); + dst[1] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[2] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0x10FFFF) + { + dst[0] = 0xF0 | ((codepoint >> 18) & 0x07); + dst[1] = 0x80 | ((codepoint >> 12) & 0x3F); + dst[2] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[3] = 0x80 | (codepoint & 0x3F); + } + + return nbytes; +} + +/* + * Decode the next Unicode codepoint from UTF8 at src, reading no more than + * srclen bytes (which must be at least 1). On success, *pcodepoint will be a + * valid Unicode Scalar; otherwise it will be set to PG_INVALID_CODEPOINT. + * + * Returns the number of bytes consumed. If srclen is not large enough + * (i.e. src is truncated in the middle of a sequence), returns 0. If invalid, + * returns -1. + */ +static inline int +utf8decode(char32_t *pcodepoint, const unsigned char *src, size_t srclen) +{ + int nbytes; + char32_t codepoint; + + Assert(srclen >= 1); + + if ((*src & 0x80) == 0) + { + *pcodepoint = (char32_t) src[0]; + return 1; + } + + if ((*src & 0xe0) == 0xc0) + nbytes = 2; + else if ((*src & 0xf0) == 0xe0) + nbytes = 3; + else if ((*src & 0xf8) == 0xf0) + nbytes = 4; + else + goto invalid; + + /* truncated */ + if (srclen < nbytes) + { + *pcodepoint = PG_INVALID_CODEPOINT; + return 0; + } + + if (nbytes == 2) + { + /* check continuation byte */ + if ((src[1] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x1f) << 6) | + (src[1] & 0x3f)); + + /* overlong */ + if (codepoint < 0x0080) + goto invalid; + } + else if (nbytes == 3) + { + /* check continuation bytes */ + if ((src[1] & 0xc0) != 0x80 || (src[2] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x0f) << 12) | + ((src[1] & 0x3f) << 6) | + (src[2] & 0x3f)); + + /* overlong or surrogate half */ + if (codepoint < 0x0800 || + (codepoint >= 0xD800 && codepoint <= 0xDFFF)) + goto invalid; + } + else if (nbytes == 4) + { + /* check continuation bytes */ + if ((src[1] & 0xc0) != 0x80 || (src[2] & 0xc0) != 0x80 || + (src[3] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x07) << 18) | + ((src[1] & 0x3f) << 12) | + ((src[2] & 0x3f) << 6) | + (src[3] & 0x3f)); + + /* overlong or out-of-range */ + if (codepoint < 0x10000 || codepoint > 0x10FFFF) + goto invalid; + } + + *pcodepoint = codepoint; + return nbytes; + +invalid: + *pcodepoint = PG_INVALID_CODEPOINT; + return -1; +} + /* * Convert a UTF-8 character to a Unicode code point. * This is a one-character version of pg_utf2wchar_with_len. * * No error checks here, c must point to a long-enough string. + * + * XXX: Callers should consider utf8decode() instead. */ static inline char32_t utf8_to_unicode(const unsigned char *c) @@ -416,13 +571,14 @@ utf8_to_unicode(const unsigned char *c) ((c[2] & 0x3f) << 6) | (c[3] & 0x3f)); else - /* that is an invalid code on purpose */ - return 0xffffffff; + return PG_INVALID_CODEPOINT; } /* * Map a Unicode code point to UTF-8. utf8string must have at least * unicode_utf8len(c) bytes available. + * + * XXX: Callers should consider utf8encode() instead. */ static inline unsigned char * unicode_to_utf8(char32_t c, unsigned char *utf8string) -- 2.43.0 [text/x-patch] v4-0004-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch (6.2K, ../../[email protected]/5-v4-0004-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch) download | inline diff: From 3b7ebe21b1df897579832690eaecf9cf1041328f Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Fri, 19 Jun 2026 14:58:47 -0700 Subject: [PATCH v4 4/4] unicode_case.c: use new utf8encode/utf8decode APIs. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 10 ++-- src/common/unicode/case_test.c | 2 +- src/common/unicode_case.c | 72 ++++++++++++----------- 3 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 5619daf43c3..63516e7174f 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,22 +62,22 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + - wbstate->offset); + int ulen; char32_t u; bool curr_alnum; size_t prev_offset = wbstate->offset; + ulen = utf8decode(&u, (const unsigned char *) wbstate->str + wbstate->offset, + wbstate->len - wbstate->offset); + /* invalid UTF8 */ - if (wbstate->offset + ulen > wbstate->len) + if (ulen <= 0) { wbstate->init = true; wbstate->offset = wbstate->len; return prev_offset; } - u = utf8_to_unicode((const unsigned char *) wbstate->str + - wbstate->offset); curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index ae0f86ffa0c..71a6f2bbe70 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -179,7 +179,7 @@ test_icu(void) { pg_unicode_category category = unicode_category(code); - if (category != PG_U_UNASSIGNED) + if (category != PG_U_UNASSIGNED && category != PG_U_SURROGATE) { uint8_t icu_category = u_charType(code); char code_str[5] = {0}; diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 79f8c7f14a7..26833cf0c53 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -194,22 +194,6 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL, NULL); } -/* local version of pg_utf_mblen() to be inlinable */ -static int -utf8_mblen(const unsigned char *s) -{ - if ((*s & 0x80) == 0) - return 1; - else if ((*s & 0xe0) == 0xc0) - return 2; - else if ((*s & 0xf0) == 0xe0) - return 3; - else if ((*s & 0xf8) == 0xf0) - return 4; - else - return -1; -} - /* * Implement Unicode Default Case Conversion algorithm. * @@ -248,18 +232,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - int u1len = utf8_mblen((const unsigned char *) src + srcoff); char32_t u1; + int u1len; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + u1len = utf8decode(&u1, (const unsigned char *) src + srcoff, + srclen - srcoff); + /* invalid UTF8 */ - if (u1len < 0 || srcoff + u1len > srclen) + if (u1len <= 0) break; - u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -280,6 +265,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, /* no mapping; copy bytes from src */ Assert(simple == 0); Assert(special == NULL); + if (result_len + u1len <= dstsize) memcpy(dst + result_len, src + srcoff, u1len); @@ -289,11 +275,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, { /* replace with single character */ char32_t u2 = simple; - char32_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } Assert(special == NULL); - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); + Assert(u2len > 0); result_len += u2len; } @@ -304,10 +298,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, for (int i = 0; i < MAX_CASE_EXPANSION && special[i]; i++) { char32_t u2 = special[i]; - size_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); + Assert(u2len > 0); result_len += u2len; } @@ -344,15 +346,15 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - int u1len = utf8_mblen((const unsigned char *) str + i); + int u1len; char32_t curr; + u1len = utf8decode(&curr, (const unsigned char *) str + i, len - i); + /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) + if (u1len <= 0) return false; - curr = utf8_to_unicode(str + i); - if (pg_u_prop_case_ignorable(curr)) continue; else if (pg_u_prop_cased(curr)) @@ -371,19 +373,19 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) return true; /* iterate forwards, looking for Cased character */ - for (int i = offset + 1; i < len && str[i] != '\0'; i++) + for (int i = offset + 1; i < len; i++) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - int u1len = utf8_mblen((const unsigned char *) str + i); + int u1len; char32_t curr; + u1len = utf8decode(&curr, (const unsigned char *) str + i, len - i); + /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) + if (u1len <= 0) return false; - curr = utf8_to_unicode(str + i); - if (pg_u_prop_case_ignorable(curr)) continue; else if (pg_u_prop_cased(curr)) -- 2.43.0 ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-24 05:45 Jeff Davis <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 2 replies; 18+ messages in thread From: Jeff Davis @ 2026-06-24 05:45 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers On Mon, 2026-06-22 at 19:02 -0700, Jeff Davis wrote: > v4 attached. v5 attached. There's an extra patch 0002 to fix a logic bug when handling final sigma (only affects the builtin pg_unicode_fast locale), which I think should be backported to 18. Also added tests. Regards, Jeff Davis Attachments: [text/x-patch] v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch (5.5K, ../../[email protected]/2-v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch) download | inline diff: From 9a9bfbc4f3866d77e48933df052086a2558e7263 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Mon, 22 Jun 2026 16:30:01 -0700 Subject: [PATCH v5 1/5] unicode_case.c: defend against invalid UTF8. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] Backpatch-through: 17 --- src/backend/utils/adt/pg_locale_builtin.c | 24 ++++++++--- src/common/unicode/case_test.c | 8 ++++ src/common/unicode_case.c | 52 +++++++++++++++++++---- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 01d4f55b07e..7c36fd5091b 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,21 +62,33 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - char32_t u = utf8_to_unicode((const unsigned char *) wbstate->str + + int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + wbstate->offset); - bool curr_alnum = pg_u_isalnum(u, wbstate->posix); + char32_t u; + bool curr_alnum; + size_t prev_offset = wbstate->offset; - if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + /* invalid UTF8 */ + if (wbstate->offset + ulen > wbstate->len) { - size_t prev_offset = wbstate->offset; + wbstate->init = true; + wbstate->offset = wbstate->len; + return prev_offset; + } + u = utf8_to_unicode((const unsigned char *) wbstate->str + + wbstate->offset); + curr_alnum = pg_u_isalnum(u, wbstate->posix); + + if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + { wbstate->init = true; - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; return prev_offset; } - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; } return wbstate->len; diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index a0dbf00b671..31ea94513bf 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -296,6 +296,8 @@ tfunc_fold(char *dst, size_t dstsize, const char *src, static void test_convert_case(void) { + size_t needed; + /* test string with no case changes */ test_convert(tfunc_lower, "√∞", "√∞"); /* test adjust-to-cased behavior */ @@ -320,6 +322,12 @@ test_convert_case(void) /* U+FF11 FULLWIDTH ONE is alphanumeric for full case mapping */ test_convert(tfunc_title, "\uFF11a", "\uFF11a"); + /* invalid UTF8: truncated multibyte sequence */ + needed = unicode_strfold(NULL, 0, "abc\xCE", 4, false); + Assert(needed == 3); + /* invalid UTF8: invalid byte */ + needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, false); + Assert(needed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index d6ee00b7d9c..42eb7d22211 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -189,6 +189,22 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL); } +/* local version of pg_utf_mblen() to be inlinable */ +static int +utf8_mblen(const unsigned char *s) +{ + if ((*s & 0x80) == 0) + return 1; + else if ((*s & 0xe0) == 0xc0) + return 2; + else if ((*s & 0xf0) == 0xe0) + return 3; + else if ((*s & 0xf8) == 0xf0) + return 4; + else + return -1; +} + /* * Implement Unicode Default Case Conversion algorithm. * @@ -227,12 +243,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - char32_t u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - int u1len = unicode_utf8len(u1); + int u1len = utf8_mblen((const unsigned char *) src + srcoff); + char32_t u1; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + /* invalid UTF8 */ + if (u1len < 0 || srcoff + u1len > srclen) + break; + + u1 = utf8_to_unicode((const unsigned char *) src + srcoff); + if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -316,7 +338,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -327,8 +356,8 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ + else + return false; /* invalid UTF8 */ } /* end of string is not followed by a Cased character */ @@ -340,7 +369,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -351,8 +387,8 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ + else + return false; /* invalid UTF8 */ } return true; -- 2.43.0 [text/x-patch] v5-0002-pg_unicode_fast-fix-final-sigma-logic.patch (5.3K, ../../[email protected]/3-v5-0002-pg_unicode_fast-fix-final-sigma-logic.patch) download | inline diff: From c389e8f3d47c51183702f468846c0ad8c33beaae Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Tue, 23 Jun 2026 17:09:49 -0700 Subject: [PATCH v5 2/5] pg_unicode_fast: fix final sigma logic. If the string is preceded only by Case Ignorable characters, don't consider it to be a final sigma. In the process, refactor so that the preceding and following characters are found first, and then the rule is applied, to improve clarity. Discussion: https://postgr.es/m/[email protected] Backpatch-through: 18 --- src/common/unicode_case.c | 88 ++++++++++------------ src/test/regress/expected/collate.utf8.out | 6 ++ src/test/regress/sql/collate.utf8.sql | 1 + 3 files changed, 47 insertions(+), 48 deletions(-) diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 42eb7d22211..dd5b3ba86d0 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -323,75 +323,67 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, * 3-17. The character at the given offset must be directly preceded by a * Cased character, and must not be directly followed by a Cased character. * - * Case_Ignorable characters are ignored. NB: some characters may be both + * Case_Ignorable characters are ignored. Neither beginning of string nor end + * of string are considered Cased characters. NB: some characters may be both * Cased and Case_Ignorable, in which case they are ignored. */ static bool check_final_sigma(const unsigned char *str, size_t len, size_t offset) { - /* the start of the string is not preceded by a Cased character */ - if (offset == 0) - return false; + bool preceded_by_cased = false; + bool followed_by_cased = false; + char32_t curr; + int ulen; - /* iterate backwards, looking for Cased character */ - for (int i = offset - 1; i >= 0; i--) + /* iterate backwards looking for preceding character */ + for (int i = offset; i > 0;) { - if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) - { - int u1len = utf8_mblen((const unsigned char *) str + i); - char32_t curr; + /* skip backwards through continuation bytes */ + i--; + if ((str[i] & 0xC0) == 0x80) + continue; - /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) - return false; + /* now at leading byte of previous sequence */ + Assert((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0); - curr = utf8_to_unicode(str + i); + ulen = utf8_mblen((const unsigned char *) str + i); - if (pg_u_prop_case_ignorable(curr)) - continue; - else if (pg_u_prop_cased(curr)) - break; - else - return false; + /* invalid UTF8 */ + if (ulen < 0 || i + ulen > len) + return false; + + curr = utf8_to_unicode((const unsigned char *) str + i); + + if (!pg_u_prop_case_ignorable(curr)) + { + preceded_by_cased = pg_u_prop_cased(curr); + break; } - else if ((str[i] & 0xC0) == 0x80) - continue; - else - return false; /* invalid UTF8 */ } - /* end of string is not followed by a Cased character */ - if (offset == len) - return true; + ulen = utf8_mblen((const unsigned char *) str + offset); - /* iterate forwards, looking for Cased character */ - for (int i = offset + 1; i < len && str[i] != '\0'; i++) + /* iterate forward looking for following character */ + for (int i = offset + ulen; i < len;) { - if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) - { - int u1len = utf8_mblen((const unsigned char *) str + i); - char32_t curr; + ulen = utf8_mblen((const unsigned char *) str + i); - /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) - return false; + /* invalid UTF8 */ + if (ulen < 0 || i + ulen > len) + return false; - curr = utf8_to_unicode(str + i); + curr = utf8_to_unicode((const unsigned char *) str + i); - if (pg_u_prop_case_ignorable(curr)) - continue; - else if (pg_u_prop_cased(curr)) - return false; - else - break; + if (!pg_u_prop_case_ignorable(curr)) + { + followed_by_cased = pg_u_prop_cased(curr); + break; } - else if ((str[i] & 0xC0) == 0x80) - continue; - else - return false; /* invalid UTF8 */ + + i += ulen; } - return true; + return (preceded_by_cased && !followed_by_cased); } /* diff --git a/src/test/regress/expected/collate.utf8.out b/src/test/regress/expected/collate.utf8.out index 0c3ab5c89b2..99fdc111fa4 100644 --- a/src/test/regress/expected/collate.utf8.out +++ b/src/test/regress/expected/collate.utf8.out @@ -263,6 +263,12 @@ SELECT lower('ᾼΣͅΑ' COLLATE PG_UNICODE_FAST); -- 0391 0345 03A3 0345 0391 ᾳσͅα (1 row) +SELECT lower(U&'\0300\03A3' COLLATE PG_UNICODE_FAST); + lower +------- + ̀σ +(1 row) + -- properties SELECT 'xyz' ~ '[[:alnum:]]' COLLATE PG_UNICODE_FAST; ?column? diff --git a/src/test/regress/sql/collate.utf8.sql b/src/test/regress/sql/collate.utf8.sql index d6d14220ab3..22aecee3a60 100644 --- a/src/test/regress/sql/collate.utf8.sql +++ b/src/test/regress/sql/collate.utf8.sql @@ -128,6 +128,7 @@ SELECT lower('0Σ' COLLATE PG_UNICODE_FAST); -- 0030 03A3 SELECT lower('ΑΣΑ' COLLATE PG_UNICODE_FAST); -- 0391 03A3 0391 SELECT lower('ἈΣ̓Α' COLLATE PG_UNICODE_FAST); -- 0391 0343 03A3 0343 0391 SELECT lower('ᾼΣͅΑ' COLLATE PG_UNICODE_FAST); -- 0391 0345 03A3 0345 0391 +SELECT lower(U&'\0300\03A3' COLLATE PG_UNICODE_FAST); -- properties -- 2.43.0 [text/x-patch] v5-0003-unicode_case.c-change-API-to-signal-UTF8-decoding.patch (11.8K, ../../[email protected]/4-v5-0003-unicode_case.c-change-API-to-signal-UTF8-decoding.patch) download | inline diff: From 7f2494e90f041cb6e118c0a6a062b9e5364bc799 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Mon, 22 Jun 2026 16:31:38 -0700 Subject: [PATCH v5 3/5] unicode_case.c: change API to signal UTF8 decoding error. Errors at this point are not expected, but if encountered, signal to the caller so it can raise the appropriate error. Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 50 +++++++++++++++++++---- src/common/unicode/case_test.c | 37 ++++++++++------- src/common/unicode_case.c | 46 ++++++++++++--------- src/include/common/unicode_case.h | 8 ++-- 4 files changed, 94 insertions(+), 47 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 7c36fd5091b..5619daf43c3 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -98,8 +98,16 @@ static size_t strlower_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strlower(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strlower(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t @@ -114,26 +122,50 @@ strtitle_builtin(char *dest, size_t destsize, const char *src, size_t srclen, .init = false, .prev_alnum = false, }; + size_t consumed; + size_t result; - return unicode_strtitle(dest, destsize, src, srclen, - locale->builtin.casemap_full, - initcap_wbnext, &wbstate); + result = unicode_strtitle(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full, + initcap_wbnext, &wbstate); + + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strupper_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strupper(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strupper(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strfold_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strfold(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strfold(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static bool diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index 31ea94513bf..08421d9e5ca 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -115,6 +115,7 @@ icu_test_full(char *str) char icu_fold[BUFSZ]; UErrorCode status; size_t len = strlen(str); + size_t consumed; /* full case mapping doesn't use posix semantics */ struct WordBoundaryState wbstate = { @@ -126,10 +127,10 @@ icu_test_full(char *str) .prev_alnum = false, }; - unicode_strlower(lower, BUFSZ, str, len, true); - unicode_strtitle(title, BUFSZ, str, len, true, initcap_wbnext, &wbstate); - unicode_strupper(upper, BUFSZ, str, len, true); - unicode_strfold(fold, BUFSZ, str, len, true); + unicode_strlower(lower, BUFSZ, str, len, &consumed, true); + unicode_strtitle(title, BUFSZ, str, len, &consumed, true, initcap_wbnext, &wbstate); + unicode_strupper(upper, BUFSZ, str, len, &consumed, true); + unicode_strfold(fold, BUFSZ, str, len, &consumed, true); status = U_ZERO_ERROR; ucasemap_utf8ToLower(casemap, icu_lower, BUFSZ, str, len, &status); status = U_ZERO_ERROR; @@ -260,13 +261,16 @@ static size_t tfunc_lower(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strlower(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strlower(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_title(char *dst, size_t dstsize, const char *src, size_t srclen) { + size_t consumed; struct WordBoundaryState wbstate = { .str = src, .len = srclen, @@ -275,28 +279,33 @@ tfunc_title(char *dst, size_t dstsize, const char *src, .prev_alnum = false, }; - return unicode_strtitle(dst, dstsize, src, srclen, true, initcap_wbnext, - &wbstate); + return unicode_strtitle(dst, dstsize, src, srclen, &consumed, true, + initcap_wbnext, &wbstate); } static size_t tfunc_upper(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strupper(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strupper(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_fold(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strfold(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strfold(dst, dstsize, src, srclen, &consumed, true); } static void test_convert_case(void) { size_t needed; + size_t consumed; /* test string with no case changes */ test_convert(tfunc_lower, "√∞", "√∞"); @@ -323,11 +332,11 @@ test_convert_case(void) test_convert(tfunc_title, "\uFF11a", "\uFF11a"); /* invalid UTF8: truncated multibyte sequence */ - needed = unicode_strfold(NULL, 0, "abc\xCE", 4, false); - Assert(needed == 3); - /* invalid UTF8: invalid byte */ - needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, false); - Assert(needed == 3); + needed = unicode_strfold(NULL, 0, "abc\xCE", 4, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: leading byte invalid length */ + needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index dd5b3ba86d0..24753aaab09 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -40,8 +40,8 @@ static const char32_t *const casekind_map[NCaseKind] = static char32_t find_case_map(char32_t ucs, const char32_t *map); static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate); + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate); static enum CaseMapResult casemap(char32_t u1, CaseKind casekind, bool full, const char *src, size_t srclen, size_t srcoff, char32_t *simple, const char32_t **special); @@ -82,7 +82,8 @@ unicode_casefold_simple(char32_t code) * unicode_strlower() * * Convert src to lowercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -98,17 +99,18 @@ unicode_casefold_simple(char32_t code) */ size_t unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseLower, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseLower, full, + NULL, NULL); } /* * unicode_strtitle() * * Convert src to titlecase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -134,17 +136,19 @@ unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full, WordBoundaryNext wbnext, void *wbstate) + size_t *pconsumed, bool full, WordBoundaryNext wbnext, + void *wbstate) { - return convert_case(dst, dstsize, src, srclen, CaseTitle, full, wbnext, - wbstate); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseTitle, full, + wbnext, wbstate); } /* * unicode_strupper() * * Convert src to uppercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -160,17 +164,18 @@ unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseUpper, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseUpper, full, + NULL, NULL); } /* * unicode_strfold() * * Case fold src, and return the result length (not including terminating - * NUL). + * NUL). Sets *pconsumed to the amount of src successfully consumed; if less + * than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -183,10 +188,10 @@ unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseFold, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseFold, full, + NULL, NULL); } /* local version of pg_utf_mblen() to be inlinable */ @@ -223,8 +228,8 @@ utf8_mblen(const unsigned char *s) */ static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate) + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate) { /* character CaseKind varies while titlecasing */ CaseKind chr_casekind = str_casekind; @@ -315,6 +320,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, if (result_len < dstsize) dst[result_len] = '\0'; + *pconsumed = srcoff; return result_len; } diff --git a/src/include/common/unicode_case.h b/src/include/common/unicode_case.h index 03add78cabe..1cbc0c14bc2 100644 --- a/src/include/common/unicode_case.h +++ b/src/include/common/unicode_case.h @@ -21,13 +21,13 @@ char32_t unicode_titlecase_simple(char32_t code); char32_t unicode_uppercase_simple(char32_t code); char32_t unicode_casefold_simple(char32_t code); size_t unicode_strlower(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full, + size_t srclen, size_t *pconsumed, bool full, WordBoundaryNext wbnext, void *wbstate); size_t unicode_strupper(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strfold(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); #endif /* UNICODE_CASE_H */ -- 2.43.0 [text/x-patch] v5-0004-Validating-iterator-friendly-UTF8-encoder-decoder.patch (5.2K, ../../[email protected]/5-v5-0004-Validating-iterator-friendly-UTF8-encoder-decoder.patch) download | inline diff: From 4f86b3ccfac7a55b905b3f5d8201c953a3a295f1 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 4 Jun 2026 12:08:51 -0700 Subject: [PATCH v5 4/5] Validating, iterator-friendly UTF8 encoder/decoder API. Discussion: https://postgr.es/m/[email protected] --- src/include/mb/pg_wchar.h | 160 +++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 2 deletions(-) diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index deee2a832c3..d8ea77c3fe0 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -27,6 +27,11 @@ */ typedef unsigned int pg_wchar; +/* + * Returned for decoding failures in utf8decode() and utf8_to_unicode(). + */ +#define PG_INVALID_CODEPOINT 0xFFFFFFFF + /* * Maximum byte length of multibyte characters in any backend encoding */ @@ -392,11 +397,161 @@ surrogate_pair_to_codepoint(char16_t first, char16_t second) return ((first & 0x3FF) << 10) + 0x10000 + (second & 0x3FF); } +/* + * Encode the codepoint as UTF8 and return the number of bytes required. If + * the number of bytes required exceeds dstsize, just return the number of + * bytes required without modifying dst. If dstsize is zero, dst may be + * NULL. If codepoint is not a valid Unicode Scalar, return -1. + */ +static inline int +utf8encode(unsigned char *dst, size_t dstsize, char32_t codepoint) +{ + int nbytes; + + if (codepoint <= 0x7F) + nbytes = 1; + else if (codepoint <= 0x7FF) + nbytes = 2; + else if (codepoint <= 0xFFFF) + { + /* surrogate halves not valid for UTF8 */ + if (codepoint >= 0xD800 && codepoint <= 0xDFFF) + return -1; + nbytes = 3; + } + else if (codepoint <= 0x10FFFF) + nbytes = 4; + else + return -1; + + if (nbytes > dstsize) + return nbytes; + + if (codepoint <= 0x7F) + { + dst[0] = codepoint; + } + else if (codepoint <= 0x7FF) + { + dst[0] = 0xC0 | ((codepoint >> 6) & 0x1F); + dst[1] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0xFFFF) + { + dst[0] = 0xE0 | ((codepoint >> 12) & 0x0F); + dst[1] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[2] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0x10FFFF) + { + dst[0] = 0xF0 | ((codepoint >> 18) & 0x07); + dst[1] = 0x80 | ((codepoint >> 12) & 0x3F); + dst[2] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[3] = 0x80 | (codepoint & 0x3F); + } + + return nbytes; +} + +/* + * Decode the next Unicode codepoint from UTF8 at src, reading no more than + * srclen bytes (which must be at least 1). On success, *pcodepoint will be a + * valid Unicode Scalar; otherwise it will be set to PG_INVALID_CODEPOINT. + * + * Returns the number of bytes consumed. If srclen is not large enough + * (i.e. src is truncated in the middle of a sequence), returns 0. If invalid, + * returns -1. + */ +static inline int +utf8decode(char32_t *pcodepoint, const unsigned char *src, size_t srclen) +{ + int nbytes; + char32_t codepoint; + + Assert(srclen >= 1); + + if ((*src & 0x80) == 0) + { + *pcodepoint = (char32_t) src[0]; + return 1; + } + + if ((*src & 0xe0) == 0xc0) + nbytes = 2; + else if ((*src & 0xf0) == 0xe0) + nbytes = 3; + else if ((*src & 0xf8) == 0xf0) + nbytes = 4; + else + goto invalid; + + /* truncated */ + if (srclen < nbytes) + { + *pcodepoint = PG_INVALID_CODEPOINT; + return 0; + } + + if (nbytes == 2) + { + /* check continuation byte */ + if ((src[1] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x1f) << 6) | + (src[1] & 0x3f)); + + /* overlong */ + if (codepoint < 0x0080) + goto invalid; + } + else if (nbytes == 3) + { + /* check continuation bytes */ + if ((src[1] & 0xc0) != 0x80 || (src[2] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x0f) << 12) | + ((src[1] & 0x3f) << 6) | + (src[2] & 0x3f)); + + /* overlong or surrogate half */ + if (codepoint < 0x0800 || + (codepoint >= 0xD800 && codepoint <= 0xDFFF)) + goto invalid; + } + else if (nbytes == 4) + { + /* check continuation bytes */ + if ((src[1] & 0xc0) != 0x80 || (src[2] & 0xc0) != 0x80 || + (src[3] & 0xc0) != 0x80) + goto invalid; + + codepoint = (char32_t) (((src[0] & 0x07) << 18) | + ((src[1] & 0x3f) << 12) | + ((src[2] & 0x3f) << 6) | + (src[3] & 0x3f)); + + /* overlong or out-of-range */ + if (codepoint < 0x10000 || codepoint > 0x10FFFF) + goto invalid; + } + + *pcodepoint = codepoint; + return nbytes; + +invalid: + *pcodepoint = PG_INVALID_CODEPOINT; + return -1; +} + /* * Convert a UTF-8 character to a Unicode code point. * This is a one-character version of pg_utf2wchar_with_len. * * No error checks here, c must point to a long-enough string. + * + * XXX: Callers should consider utf8decode() instead. */ static inline char32_t utf8_to_unicode(const unsigned char *c) @@ -416,13 +571,14 @@ utf8_to_unicode(const unsigned char *c) ((c[2] & 0x3f) << 6) | (c[3] & 0x3f)); else - /* that is an invalid code on purpose */ - return 0xffffffff; + return PG_INVALID_CODEPOINT; } /* * Map a Unicode code point to UTF-8. utf8string must have at least * unicode_utf8len(c) bytes available. + * + * XXX: Callers should consider utf8encode() instead. */ static inline unsigned char * unicode_to_utf8(char32_t c, unsigned char *utf8string) -- 2.43.0 [text/x-patch] v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch (8.5K, ../../[email protected]/6-v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch) download | inline diff: From 25562a48cfbd93c25f23e2734d27307d6f5459d6 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Fri, 19 Jun 2026 14:58:47 -0700 Subject: [PATCH v5 5/5] unicode_case.c: use new utf8encode/utf8decode APIs. Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 10 +-- src/common/unicode/case_test.c | 45 +++++++++---- src/common/unicode_case.c | 77 +++++++++++------------ 3 files changed, 77 insertions(+), 55 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 5619daf43c3..63516e7174f 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,22 +62,22 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + - wbstate->offset); + int ulen; char32_t u; bool curr_alnum; size_t prev_offset = wbstate->offset; + ulen = utf8decode(&u, (const unsigned char *) wbstate->str + wbstate->offset, + wbstate->len - wbstate->offset); + /* invalid UTF8 */ - if (wbstate->offset + ulen > wbstate->len) + if (ulen <= 0) { wbstate->init = true; wbstate->offset = wbstate->len; return prev_offset; } - u = utf8_to_unicode((const unsigned char *) wbstate->str + - wbstate->offset); curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index 08421d9e5ca..9461b56742b 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -52,24 +52,35 @@ initcap_wbnext(void *state) { struct WordBoundaryState *wbstate = (struct WordBoundaryState *) state; - while (wbstate->offset < wbstate->len && - wbstate->str[wbstate->offset] != '\0') + while (wbstate->offset < wbstate->len) { - char32_t u = utf8_to_unicode((const unsigned char *) wbstate->str + - wbstate->offset); - bool curr_alnum = pg_u_isalnum(u, wbstate->posix); + int ulen; + char32_t u; + bool curr_alnum; + size_t prev_offset = wbstate->offset; - if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + ulen = utf8decode(&u, (const unsigned char *) wbstate->str + wbstate->offset, + wbstate->len - wbstate->offset); + + /* invalid UTF8 */ + if (ulen <= 0) { - size_t prev_offset = wbstate->offset; + wbstate->init = true; + wbstate->offset = wbstate->len; + return prev_offset; + } + + curr_alnum = pg_u_isalnum(u, wbstate->posix); + if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + { wbstate->init = true; - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; return prev_offset; } - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; } return wbstate->len; @@ -179,7 +190,7 @@ test_icu(void) { pg_unicode_category category = unicode_category(code); - if (category != PG_U_UNASSIGNED) + if (category != PG_U_UNASSIGNED && category != PG_U_SURROGATE) { uint8_t icu_category = u_charType(code); char code_str[5] = {0}; @@ -191,7 +202,7 @@ test_icu(void) } icu_test_simple(code); - unicode_to_utf8(code, (unsigned char *) code_str); + utf8encode((unsigned char *) code_str, 5, code); icu_test_full(code_str); successful++; @@ -337,6 +348,18 @@ test_convert_case(void) /* invalid UTF8: leading byte invalid length */ needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, &consumed, false); Assert(needed == 3 && consumed == 3); + /* invalid UTF8: surrogates */ + needed = unicode_strfold(NULL, 0, "abc\xED\xA0\x81xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: continuation with no leading byte */ + needed = unicode_strfold(NULL, 0, "abc\x80xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: out of range */ + needed = unicode_strfold(NULL, 0, "abc\xF5\x80\x80\x80xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: overlong */ + needed = unicode_strfold(NULL, 0, "abc\xC1\xBFxyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 24753aaab09..4d8ee71e8dc 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -194,22 +194,6 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL, NULL); } -/* local version of pg_utf_mblen() to be inlinable */ -static int -utf8_mblen(const unsigned char *s) -{ - if ((*s & 0x80) == 0) - return 1; - else if ((*s & 0xe0) == 0xc0) - return 2; - else if ((*s & 0xf0) == 0xe0) - return 3; - else if ((*s & 0xf8) == 0xf0) - return 4; - else - return -1; -} - /* * Implement Unicode Default Case Conversion algorithm. * @@ -248,18 +232,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - int u1len = utf8_mblen((const unsigned char *) src + srcoff); char32_t u1; + int u1len; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + u1len = utf8decode(&u1, (const unsigned char *) src + srcoff, + srclen - srcoff); + /* invalid UTF8 */ - if (u1len < 0 || srcoff + u1len > srclen) + if (u1len <= 0) break; - u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -280,6 +265,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, /* no mapping; copy bytes from src */ Assert(simple == 0); Assert(special == NULL); + if (result_len + u1len <= dstsize) memcpy(dst + result_len, src + srcoff, u1len); @@ -289,11 +275,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, { /* replace with single character */ char32_t u2 = simple; - char32_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } Assert(special == NULL); - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); + Assert(u2len > 0); result_len += u2len; } @@ -304,10 +298,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, for (int i = 0; i < MAX_CASE_EXPANSION && special[i]; i++) { char32_t u2 = special[i]; - size_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); + Assert(u2len > 0); result_len += u2len; } @@ -352,13 +354,10 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) /* now at leading byte of previous sequence */ Assert((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0); - ulen = utf8_mblen((const unsigned char *) str + i); - - /* invalid UTF8 */ - if (ulen < 0 || i + ulen > len) - return false; + ulen = utf8decode(&curr, (const unsigned char *) str + i, len - i); - curr = utf8_to_unicode((const unsigned char *) str + i); + if (ulen <= 0) + return false; /* invalid UTF8 */ if (!pg_u_prop_case_ignorable(curr)) { @@ -367,18 +366,18 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } } - ulen = utf8_mblen((const unsigned char *) str + offset); + ulen = utf8decode(&curr, (const unsigned char *) str + offset, + len - offset); + if (ulen <= 0) + return false; /* invalid UTF8 */ /* iterate forward looking for following character */ for (int i = offset + ulen; i < len;) { - ulen = utf8_mblen((const unsigned char *) str + i); - - /* invalid UTF8 */ - if (ulen < 0 || i + ulen > len) - return false; + ulen = utf8decode(&curr, (const unsigned char *) str + i, len - i); - curr = utf8_to_unicode((const unsigned char *) str + i); + if (ulen <= 0) + return false; /* invalid UTF8 */ if (!pg_u_prop_case_ignorable(curr)) { -- 2.43.0 ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-24 08:44 Chao Li <[email protected]> parent: Jeff Davis <[email protected]> 1 sibling, 1 reply; 18+ messages in thread From: Chao Li @ 2026-06-24 08:44 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Jun 24, 2026, at 13:45, Jeff Davis <[email protected]> wrote: > > On Mon, 2026-06-22 at 19:02 -0700, Jeff Davis wrote: >> v4 attached. > > v5 attached. > > There's an extra patch 0002 to fix a logic bug when handling final > sigma (only affects the builtin pg_unicode_fast locale), which I think > should be backported to 18. > > Also added tests. > > Regards, > Jeff Davis > > <v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch><v5-0002-pg_unicode_fast-fix-final-sigma-logic.patch><v5-0003-unicode_case.c-change-API-to-signal-UTF8-decoding.patch><v5-0004-Validating-iterator-friendly-UTF8-encoder-decoder.patch><v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch> There is a compile warning against pg_wchar.h in 0004: ``` ../../../src/include/mb/pg_wchar.h:523:11: warning: variable 'codepoint' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] 523 | else if (nbytes == 4) | ^~~~~~~~~~~ ../../../src/include/mb/pg_wchar.h:540:16: note: uninitialized use occurs here 540 | *pcodepoint = codepoint; | ^~~~~~~~~ ../../../src/include/mb/pg_wchar.h:523:7: note: remove the 'if' if its condition is always true 523 | else if (nbytes == 4) | ^~~~~~~~~~~~~~~~ 524 | { ../../../src/include/mb/pg_wchar.h:469:20: note: initialize the variable 'codepoint' to silence this warning 469 | char32_t codepoint; | ^ | = 0 1 warning generated. ``` Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-24 09:29 Ayush Tiwari <[email protected]> parent: Jeff Davis <[email protected]> 1 sibling, 1 reply; 18+ messages in thread From: Ayush Tiwari @ 2026-06-24 09:29 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: Chao Li <[email protected]>; pgsql-hackers Hi, On Wed, 24 Jun 2026 at 11:15, Jeff Davis <[email protected]> wrote: > On Mon, 2026-06-22 at 19:02 -0700, Jeff Davis wrote: > > v4 attached. > > v5 attached. > > There's an extra patch 0002 to fix a logic bug when handling final > sigma (only affects the builtin pg_unicode_fast locale), which I think > should be backported to 18. > > Also added tests. > Thanks for the patch! I took a look at the v5 series and tried it locally. The split between the backpatchable defensive change, the final-sigma fix, and the newer utf8encode/utf8decode API work for master makes sense to me. On my machine, case-check fails before reaching the new invalid-UTF8 assertions. I think it's because PostgreSQL's Unicode tables are 17.0 while my system ICU is 15.1, so the exhaustive ICU comparison in test_icu() hits a changed mapping and exits first. Would it be worth running test_convert_case() before test_icu()? It wouldn't make case-check pass on a mismatched-ICU system, but it would at least let the non-ICU conversion and the new invalid-UTF8 cases run before the ICU comparison aborts. Regards, Ayush ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-24 21:57 Jeff Davis <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2026-06-24 21:57 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers On Wed, 2026-06-24 at 16:44 +0800, Chao Li wrote: > There is a compile warning against pg_wchar.h in 0004: 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. Regards, Jeff Davis Attachments: [text/x-patch] v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch (5.5K, ../../[email protected]/2-v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch) download | inline diff: From 9a9bfbc4f3866d77e48933df052086a2558e7263 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Mon, 22 Jun 2026 16:30:01 -0700 Subject: [PATCH v5 1/5] unicode_case.c: defend against invalid UTF8. Reviewed-by: Chao Li <[email protected]> Discussion: https://postgr.es/m/[email protected] Backpatch-through: 17 --- src/backend/utils/adt/pg_locale_builtin.c | 24 ++++++++--- src/common/unicode/case_test.c | 8 ++++ src/common/unicode_case.c | 52 +++++++++++++++++++---- 3 files changed, 70 insertions(+), 14 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 01d4f55b07e..7c36fd5091b 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,21 +62,33 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - char32_t u = utf8_to_unicode((const unsigned char *) wbstate->str + + int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + wbstate->offset); - bool curr_alnum = pg_u_isalnum(u, wbstate->posix); + char32_t u; + bool curr_alnum; + size_t prev_offset = wbstate->offset; - if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + /* invalid UTF8 */ + if (wbstate->offset + ulen > wbstate->len) { - size_t prev_offset = wbstate->offset; + wbstate->init = true; + wbstate->offset = wbstate->len; + return prev_offset; + } + u = utf8_to_unicode((const unsigned char *) wbstate->str + + wbstate->offset); + curr_alnum = pg_u_isalnum(u, wbstate->posix); + + if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + { wbstate->init = true; - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; return prev_offset; } - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; } return wbstate->len; diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index a0dbf00b671..31ea94513bf 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -296,6 +296,8 @@ tfunc_fold(char *dst, size_t dstsize, const char *src, static void test_convert_case(void) { + size_t needed; + /* test string with no case changes */ test_convert(tfunc_lower, "√∞", "√∞"); /* test adjust-to-cased behavior */ @@ -320,6 +322,12 @@ test_convert_case(void) /* U+FF11 FULLWIDTH ONE is alphanumeric for full case mapping */ test_convert(tfunc_title, "\uFF11a", "\uFF11a"); + /* invalid UTF8: truncated multibyte sequence */ + needed = unicode_strfold(NULL, 0, "abc\xCE", 4, false); + Assert(needed == 3); + /* invalid UTF8: invalid byte */ + needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, false); + Assert(needed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index d6ee00b7d9c..42eb7d22211 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -189,6 +189,22 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL); } +/* local version of pg_utf_mblen() to be inlinable */ +static int +utf8_mblen(const unsigned char *s) +{ + if ((*s & 0x80) == 0) + return 1; + else if ((*s & 0xe0) == 0xc0) + return 2; + else if ((*s & 0xf0) == 0xe0) + return 3; + else if ((*s & 0xf8) == 0xf0) + return 4; + else + return -1; +} + /* * Implement Unicode Default Case Conversion algorithm. * @@ -227,12 +243,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - char32_t u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - int u1len = unicode_utf8len(u1); + int u1len = utf8_mblen((const unsigned char *) src + srcoff); + char32_t u1; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + /* invalid UTF8 */ + if (u1len < 0 || srcoff + u1len > srclen) + break; + + u1 = utf8_to_unicode((const unsigned char *) src + srcoff); + if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -316,7 +338,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -327,8 +356,8 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ + else + return false; /* invalid UTF8 */ } /* end of string is not followed by a Cased character */ @@ -340,7 +369,14 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) { if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) { - char32_t curr = utf8_to_unicode(str + i); + int u1len = utf8_mblen((const unsigned char *) str + i); + char32_t curr; + + /* invalid UTF8 */ + if (u1len < 0 || i + u1len > len) + return false; + + curr = utf8_to_unicode(str + i); if (pg_u_prop_case_ignorable(curr)) continue; @@ -351,8 +387,8 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } else if ((str[i] & 0xC0) == 0x80) continue; - - Assert(false); /* invalid UTF-8 */ + else + return false; /* invalid UTF8 */ } return true; -- 2.43.0 [text/x-patch] v5-0002-pg_unicode_fast-fix-final-sigma-logic.patch (5.3K, ../../[email protected]/3-v5-0002-pg_unicode_fast-fix-final-sigma-logic.patch) download | inline diff: From c389e8f3d47c51183702f468846c0ad8c33beaae Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Tue, 23 Jun 2026 17:09:49 -0700 Subject: [PATCH v5 2/5] pg_unicode_fast: fix final sigma logic. If the string is preceded only by Case Ignorable characters, don't consider it to be a final sigma. In the process, refactor so that the preceding and following characters are found first, and then the rule is applied, to improve clarity. Discussion: https://postgr.es/m/[email protected] Backpatch-through: 18 --- src/common/unicode_case.c | 88 ++++++++++------------ src/test/regress/expected/collate.utf8.out | 6 ++ src/test/regress/sql/collate.utf8.sql | 1 + 3 files changed, 47 insertions(+), 48 deletions(-) diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 42eb7d22211..dd5b3ba86d0 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -323,75 +323,67 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, * 3-17. The character at the given offset must be directly preceded by a * Cased character, and must not be directly followed by a Cased character. * - * Case_Ignorable characters are ignored. NB: some characters may be both + * Case_Ignorable characters are ignored. Neither beginning of string nor end + * of string are considered Cased characters. NB: some characters may be both * Cased and Case_Ignorable, in which case they are ignored. */ static bool check_final_sigma(const unsigned char *str, size_t len, size_t offset) { - /* the start of the string is not preceded by a Cased character */ - if (offset == 0) - return false; + bool preceded_by_cased = false; + bool followed_by_cased = false; + char32_t curr; + int ulen; - /* iterate backwards, looking for Cased character */ - for (int i = offset - 1; i >= 0; i--) + /* iterate backwards looking for preceding character */ + for (int i = offset; i > 0;) { - if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) - { - int u1len = utf8_mblen((const unsigned char *) str + i); - char32_t curr; + /* skip backwards through continuation bytes */ + i--; + if ((str[i] & 0xC0) == 0x80) + continue; - /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) - return false; + /* now at leading byte of previous sequence */ + Assert((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0); - curr = utf8_to_unicode(str + i); + ulen = utf8_mblen((const unsigned char *) str + i); - if (pg_u_prop_case_ignorable(curr)) - continue; - else if (pg_u_prop_cased(curr)) - break; - else - return false; + /* invalid UTF8 */ + if (ulen < 0 || i + ulen > len) + return false; + + curr = utf8_to_unicode((const unsigned char *) str + i); + + if (!pg_u_prop_case_ignorable(curr)) + { + preceded_by_cased = pg_u_prop_cased(curr); + break; } - else if ((str[i] & 0xC0) == 0x80) - continue; - else - return false; /* invalid UTF8 */ } - /* end of string is not followed by a Cased character */ - if (offset == len) - return true; + ulen = utf8_mblen((const unsigned char *) str + offset); - /* iterate forwards, looking for Cased character */ - for (int i = offset + 1; i < len && str[i] != '\0'; i++) + /* iterate forward looking for following character */ + for (int i = offset + ulen; i < len;) { - if ((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0) - { - int u1len = utf8_mblen((const unsigned char *) str + i); - char32_t curr; + ulen = utf8_mblen((const unsigned char *) str + i); - /* invalid UTF8 */ - if (u1len < 0 || i + u1len > len) - return false; + /* invalid UTF8 */ + if (ulen < 0 || i + ulen > len) + return false; - curr = utf8_to_unicode(str + i); + curr = utf8_to_unicode((const unsigned char *) str + i); - if (pg_u_prop_case_ignorable(curr)) - continue; - else if (pg_u_prop_cased(curr)) - return false; - else - break; + if (!pg_u_prop_case_ignorable(curr)) + { + followed_by_cased = pg_u_prop_cased(curr); + break; } - else if ((str[i] & 0xC0) == 0x80) - continue; - else - return false; /* invalid UTF8 */ + + i += ulen; } - return true; + return (preceded_by_cased && !followed_by_cased); } /* diff --git a/src/test/regress/expected/collate.utf8.out b/src/test/regress/expected/collate.utf8.out index 0c3ab5c89b2..99fdc111fa4 100644 --- a/src/test/regress/expected/collate.utf8.out +++ b/src/test/regress/expected/collate.utf8.out @@ -263,6 +263,12 @@ SELECT lower('ᾼΣͅΑ' COLLATE PG_UNICODE_FAST); -- 0391 0345 03A3 0345 0391 ᾳσͅα (1 row) +SELECT lower(U&'\0300\03A3' COLLATE PG_UNICODE_FAST); + lower +------- + ̀σ +(1 row) + -- properties SELECT 'xyz' ~ '[[:alnum:]]' COLLATE PG_UNICODE_FAST; ?column? diff --git a/src/test/regress/sql/collate.utf8.sql b/src/test/regress/sql/collate.utf8.sql index d6d14220ab3..22aecee3a60 100644 --- a/src/test/regress/sql/collate.utf8.sql +++ b/src/test/regress/sql/collate.utf8.sql @@ -128,6 +128,7 @@ SELECT lower('0Σ' COLLATE PG_UNICODE_FAST); -- 0030 03A3 SELECT lower('ΑΣΑ' COLLATE PG_UNICODE_FAST); -- 0391 03A3 0391 SELECT lower('ἈΣ̓Α' COLLATE PG_UNICODE_FAST); -- 0391 0343 03A3 0343 0391 SELECT lower('ᾼΣͅΑ' COLLATE PG_UNICODE_FAST); -- 0391 0345 03A3 0345 0391 +SELECT lower(U&'\0300\03A3' COLLATE PG_UNICODE_FAST); -- properties -- 2.43.0 [text/x-patch] v5-0003-unicode_case.c-change-API-to-signal-UTF8-decoding.patch (11.8K, ../../[email protected]/4-v5-0003-unicode_case.c-change-API-to-signal-UTF8-decoding.patch) download | inline diff: From 7f2494e90f041cb6e118c0a6a062b9e5364bc799 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Mon, 22 Jun 2026 16:31:38 -0700 Subject: [PATCH v5 3/5] unicode_case.c: change API to signal UTF8 decoding error. Errors at this point are not expected, but if encountered, signal to the caller so it can raise the appropriate error. Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 50 +++++++++++++++++++---- src/common/unicode/case_test.c | 37 ++++++++++------- src/common/unicode_case.c | 46 ++++++++++++--------- src/include/common/unicode_case.h | 8 ++-- 4 files changed, 94 insertions(+), 47 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 7c36fd5091b..5619daf43c3 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -98,8 +98,16 @@ static size_t strlower_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strlower(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strlower(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t @@ -114,26 +122,50 @@ strtitle_builtin(char *dest, size_t destsize, const char *src, size_t srclen, .init = false, .prev_alnum = false, }; + size_t consumed; + size_t result; - return unicode_strtitle(dest, destsize, src, srclen, - locale->builtin.casemap_full, - initcap_wbnext, &wbstate); + result = unicode_strtitle(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full, + initcap_wbnext, &wbstate); + + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strupper_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strupper(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strupper(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static size_t strfold_builtin(char *dest, size_t destsize, const char *src, size_t srclen, pg_locale_t locale) { - return unicode_strfold(dest, destsize, src, srclen, - locale->builtin.casemap_full); + size_t consumed; + size_t result; + + result = unicode_strfold(dest, destsize, src, srclen, &consumed, + locale->builtin.casemap_full); + if (consumed < srclen) + report_invalid_encoding(GetDatabaseEncoding(), src + consumed, + srclen - consumed); + + return result; } static bool diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index 31ea94513bf..08421d9e5ca 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -115,6 +115,7 @@ icu_test_full(char *str) char icu_fold[BUFSZ]; UErrorCode status; size_t len = strlen(str); + size_t consumed; /* full case mapping doesn't use posix semantics */ struct WordBoundaryState wbstate = { @@ -126,10 +127,10 @@ icu_test_full(char *str) .prev_alnum = false, }; - unicode_strlower(lower, BUFSZ, str, len, true); - unicode_strtitle(title, BUFSZ, str, len, true, initcap_wbnext, &wbstate); - unicode_strupper(upper, BUFSZ, str, len, true); - unicode_strfold(fold, BUFSZ, str, len, true); + unicode_strlower(lower, BUFSZ, str, len, &consumed, true); + unicode_strtitle(title, BUFSZ, str, len, &consumed, true, initcap_wbnext, &wbstate); + unicode_strupper(upper, BUFSZ, str, len, &consumed, true); + unicode_strfold(fold, BUFSZ, str, len, &consumed, true); status = U_ZERO_ERROR; ucasemap_utf8ToLower(casemap, icu_lower, BUFSZ, str, len, &status); status = U_ZERO_ERROR; @@ -260,13 +261,16 @@ static size_t tfunc_lower(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strlower(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strlower(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_title(char *dst, size_t dstsize, const char *src, size_t srclen) { + size_t consumed; struct WordBoundaryState wbstate = { .str = src, .len = srclen, @@ -275,28 +279,33 @@ tfunc_title(char *dst, size_t dstsize, const char *src, .prev_alnum = false, }; - return unicode_strtitle(dst, dstsize, src, srclen, true, initcap_wbnext, - &wbstate); + return unicode_strtitle(dst, dstsize, src, srclen, &consumed, true, + initcap_wbnext, &wbstate); } static size_t tfunc_upper(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strupper(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strupper(dst, dstsize, src, srclen, &consumed, true); } static size_t tfunc_fold(char *dst, size_t dstsize, const char *src, size_t srclen) { - return unicode_strfold(dst, dstsize, src, srclen, true); + size_t consumed; + + return unicode_strfold(dst, dstsize, src, srclen, &consumed, true); } static void test_convert_case(void) { size_t needed; + size_t consumed; /* test string with no case changes */ test_convert(tfunc_lower, "√∞", "√∞"); @@ -323,11 +332,11 @@ test_convert_case(void) test_convert(tfunc_title, "\uFF11a", "\uFF11a"); /* invalid UTF8: truncated multibyte sequence */ - needed = unicode_strfold(NULL, 0, "abc\xCE", 4, false); - Assert(needed == 3); - /* invalid UTF8: invalid byte */ - needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, false); - Assert(needed == 3); + needed = unicode_strfold(NULL, 0, "abc\xCE", 4, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: leading byte invalid length */ + needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index dd5b3ba86d0..24753aaab09 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -40,8 +40,8 @@ static const char32_t *const casekind_map[NCaseKind] = static char32_t find_case_map(char32_t ucs, const char32_t *map); static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate); + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate); static enum CaseMapResult casemap(char32_t u1, CaseKind casekind, bool full, const char *src, size_t srclen, size_t srcoff, char32_t *simple, const char32_t **special); @@ -82,7 +82,8 @@ unicode_casefold_simple(char32_t code) * unicode_strlower() * * Convert src to lowercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -98,17 +99,18 @@ unicode_casefold_simple(char32_t code) */ size_t unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseLower, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseLower, full, + NULL, NULL); } /* * unicode_strtitle() * * Convert src to titlecase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -134,17 +136,19 @@ unicode_strlower(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full, WordBoundaryNext wbnext, void *wbstate) + size_t *pconsumed, bool full, WordBoundaryNext wbnext, + void *wbstate) { - return convert_case(dst, dstsize, src, srclen, CaseTitle, full, wbnext, - wbstate); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseTitle, full, + wbnext, wbstate); } /* * unicode_strupper() * * Convert src to uppercase, and return the result length (not including - * terminating NUL). + * terminating NUL). Sets *pconsumed to the amount of src successfully + * consumed; if less than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -160,17 +164,18 @@ unicode_strtitle(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseUpper, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseUpper, full, + NULL, NULL); } /* * unicode_strfold() * * Case fold src, and return the result length (not including terminating - * NUL). + * NUL). Sets *pconsumed to the amount of src successfully consumed; if less + * than srclen, indicates a decoding error. * * String src must be encoded in UTF-8. * @@ -183,10 +188,10 @@ unicode_strupper(char *dst, size_t dstsize, const char *src, size_t srclen, */ size_t unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, - bool full) + size_t *pconsumed, bool full) { - return convert_case(dst, dstsize, src, srclen, CaseFold, full, NULL, - NULL); + return convert_case(dst, dstsize, src, srclen, pconsumed, CaseFold, full, + NULL, NULL); } /* local version of pg_utf_mblen() to be inlinable */ @@ -223,8 +228,8 @@ utf8_mblen(const unsigned char *s) */ static size_t convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, - CaseKind str_casekind, bool full, WordBoundaryNext wbnext, - void *wbstate) + size_t *pconsumed, CaseKind str_casekind, bool full, + WordBoundaryNext wbnext, void *wbstate) { /* character CaseKind varies while titlecasing */ CaseKind chr_casekind = str_casekind; @@ -315,6 +320,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, if (result_len < dstsize) dst[result_len] = '\0'; + *pconsumed = srcoff; return result_len; } diff --git a/src/include/common/unicode_case.h b/src/include/common/unicode_case.h index 03add78cabe..1cbc0c14bc2 100644 --- a/src/include/common/unicode_case.h +++ b/src/include/common/unicode_case.h @@ -21,13 +21,13 @@ char32_t unicode_titlecase_simple(char32_t code); char32_t unicode_uppercase_simple(char32_t code); char32_t unicode_casefold_simple(char32_t code); size_t unicode_strlower(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strtitle(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full, + size_t srclen, size_t *pconsumed, bool full, WordBoundaryNext wbnext, void *wbstate); size_t unicode_strupper(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); size_t unicode_strfold(char *dst, size_t dstsize, const char *src, - size_t srclen, bool full); + size_t srclen, size_t *pconsumed, bool full); #endif /* UNICODE_CASE_H */ -- 2.43.0 [text/x-patch] v5-0004-Validating-iterator-friendly-UTF8-encoder-decoder.patch (4.8K, ../../[email protected]/5-v5-0004-Validating-iterator-friendly-UTF8-encoder-decoder.patch) download | inline diff: From 08acf6743051710355b6c8c6c9102a7922a3d163 Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Thu, 4 Jun 2026 12:08:51 -0700 Subject: [PATCH v5 4/5] Validating, iterator-friendly UTF8 encoder/decoder API. Discussion: https://postgr.es/m/[email protected] --- src/include/mb/pg_wchar.h | 139 +++++++++++++++++++++++++++++++++++++- 1 file changed, 137 insertions(+), 2 deletions(-) diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h index deee2a832c3..bc445be0678 100644 --- a/src/include/mb/pg_wchar.h +++ b/src/include/mb/pg_wchar.h @@ -27,6 +27,11 @@ */ typedef unsigned int pg_wchar; +/* + * Returned for decoding failures in utf8decode() and utf8_to_unicode(). + */ +#define PG_INVALID_CODEPOINT 0xFFFFFFFF + /* * Maximum byte length of multibyte characters in any backend encoding */ @@ -392,11 +397,140 @@ surrogate_pair_to_codepoint(char16_t first, char16_t second) return ((first & 0x3FF) << 10) + 0x10000 + (second & 0x3FF); } +/* + * Encode the codepoint as UTF8 and return the number of bytes required. If + * the number of bytes required exceeds dstsize, just return the number of + * bytes required without modifying dst. If dstsize is zero, dst may be + * NULL. If codepoint is not a valid Unicode Scalar, return -1. + */ +static inline int +utf8encode(unsigned char *dst, size_t dstsize, char32_t codepoint) +{ + int nbytes; + + if (codepoint <= 0x7F) + nbytes = 1; + else if (codepoint <= 0x7FF) + nbytes = 2; + else if (codepoint <= 0xFFFF) + { + /* surrogate halves not valid for UTF8 */ + if (codepoint >= 0xD800 && codepoint <= 0xDFFF) + return -1; + nbytes = 3; + } + else if (codepoint <= 0x10FFFF) + nbytes = 4; + else + return -1; + + if ((size_t) nbytes > dstsize) + return nbytes; + + if (codepoint <= 0x7F) + { + dst[0] = codepoint; + } + else if (codepoint <= 0x7FF) + { + dst[0] = 0xC0 | ((codepoint >> 6) & 0x1F); + dst[1] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0xFFFF) + { + dst[0] = 0xE0 | ((codepoint >> 12) & 0x0F); + dst[1] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[2] = 0x80 | (codepoint & 0x3F); + } + else if (codepoint <= 0x10FFFF) + { + dst[0] = 0xF0 | ((codepoint >> 18) & 0x07); + dst[1] = 0x80 | ((codepoint >> 12) & 0x3F); + dst[2] = 0x80 | ((codepoint >> 6) & 0x3F); + dst[3] = 0x80 | (codepoint & 0x3F); + } + + return nbytes; +} + +/* + * Decode the next Unicode codepoint from UTF8 at src, reading no more than + * srclen bytes (which must be at least 1). On success, *pcodepoint will be a + * valid Unicode Scalar; otherwise it will be set to PG_INVALID_CODEPOINT. + * + * Returns the number of bytes consumed. If srclen is not large enough + * (i.e. src is truncated in the middle of a sequence), returns 0. If invalid, + * returns -1. + */ +static inline int +utf8decode(char32_t *pcodepoint, const unsigned char *src, size_t srclen) +{ + int nbytes; + char32_t codepoint; + char32_t min; + + Assert(srclen >= 1); + + if ((*src & 0x80) == 0) + { + *pcodepoint = (char32_t) src[0]; + return 1; + } + else if ((*src & 0xe0) == 0xc0) + { + nbytes = 2; + min = 0x80; + codepoint = (char32_t) src[0] & 0x1f; + } + else if ((*src & 0xf0) == 0xe0) + { + nbytes = 3; + min = 0x800; + codepoint = (char32_t) src[0] & 0x0f; + } + else if ((*src & 0xf8) == 0xf0) + { + nbytes = 4; + min = 0x10000; + codepoint = (char32_t) src[0] & 0x07; + } + else + goto invalid; /* invalid leading byte */ + + /* truncated multibyte sequence */ + if (srclen < (size_t) nbytes) + { + *pcodepoint = PG_INVALID_CODEPOINT; + return 0; + } + + for (int i = 1; i < nbytes; i++) + { + if ((src[i] & 0xc0) != 0x80) + goto invalid; + codepoint = (codepoint << 6) | (src[i] & 0x3f); + } + + /* reject overlong, surrogate, and out-of-range */ + if (codepoint < min || codepoint > 0x10FFFF || + (codepoint >= 0xD800 && codepoint <= 0xDFFF)) + goto invalid; + + *pcodepoint = codepoint; + return nbytes; + +invalid: + *pcodepoint = PG_INVALID_CODEPOINT; + return -1; +} + /* * Convert a UTF-8 character to a Unicode code point. * This is a one-character version of pg_utf2wchar_with_len. * * No error checks here, c must point to a long-enough string. + * + * XXX: Callers should consider utf8decode() instead. */ static inline char32_t utf8_to_unicode(const unsigned char *c) @@ -416,13 +550,14 @@ utf8_to_unicode(const unsigned char *c) ((c[2] & 0x3f) << 6) | (c[3] & 0x3f)); else - /* that is an invalid code on purpose */ - return 0xffffffff; + return PG_INVALID_CODEPOINT; } /* * Map a Unicode code point to UTF-8. utf8string must have at least * unicode_utf8len(c) bytes available. + * + * XXX: Callers should consider utf8encode() instead. */ static inline unsigned char * unicode_to_utf8(char32_t c, unsigned char *utf8string) -- 2.43.0 [text/x-patch] v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch (8.5K, ../../[email protected]/6-v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch) download | inline diff: From 91a0f3cac34d3a4030cd8adf8553261f33804d6d Mon Sep 17 00:00:00 2001 From: Jeff Davis <[email protected]> Date: Fri, 19 Jun 2026 14:58:47 -0700 Subject: [PATCH v5 5/5] unicode_case.c: use new utf8encode/utf8decode APIs. Discussion: https://postgr.es/m/[email protected] --- src/backend/utils/adt/pg_locale_builtin.c | 10 +-- src/common/unicode/case_test.c | 45 +++++++++---- src/common/unicode_case.c | 77 +++++++++++------------ 3 files changed, 77 insertions(+), 55 deletions(-) diff --git a/src/backend/utils/adt/pg_locale_builtin.c b/src/backend/utils/adt/pg_locale_builtin.c index 5619daf43c3..63516e7174f 100644 --- a/src/backend/utils/adt/pg_locale_builtin.c +++ b/src/backend/utils/adt/pg_locale_builtin.c @@ -62,22 +62,22 @@ initcap_wbnext(void *state) while (wbstate->offset < wbstate->len) { - int ulen = pg_utf_mblen((const unsigned char *) wbstate->str + - wbstate->offset); + int ulen; char32_t u; bool curr_alnum; size_t prev_offset = wbstate->offset; + ulen = utf8decode(&u, (const unsigned char *) wbstate->str + wbstate->offset, + wbstate->len - wbstate->offset); + /* invalid UTF8 */ - if (wbstate->offset + ulen > wbstate->len) + if (ulen <= 0) { wbstate->init = true; wbstate->offset = wbstate->len; return prev_offset; } - u = utf8_to_unicode((const unsigned char *) wbstate->str + - wbstate->offset); curr_alnum = pg_u_isalnum(u, wbstate->posix); if (!wbstate->init || curr_alnum != wbstate->prev_alnum) diff --git a/src/common/unicode/case_test.c b/src/common/unicode/case_test.c index 08421d9e5ca..9461b56742b 100644 --- a/src/common/unicode/case_test.c +++ b/src/common/unicode/case_test.c @@ -52,24 +52,35 @@ initcap_wbnext(void *state) { struct WordBoundaryState *wbstate = (struct WordBoundaryState *) state; - while (wbstate->offset < wbstate->len && - wbstate->str[wbstate->offset] != '\0') + while (wbstate->offset < wbstate->len) { - char32_t u = utf8_to_unicode((const unsigned char *) wbstate->str + - wbstate->offset); - bool curr_alnum = pg_u_isalnum(u, wbstate->posix); + int ulen; + char32_t u; + bool curr_alnum; + size_t prev_offset = wbstate->offset; - if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + ulen = utf8decode(&u, (const unsigned char *) wbstate->str + wbstate->offset, + wbstate->len - wbstate->offset); + + /* invalid UTF8 */ + if (ulen <= 0) { - size_t prev_offset = wbstate->offset; + wbstate->init = true; + wbstate->offset = wbstate->len; + return prev_offset; + } + + curr_alnum = pg_u_isalnum(u, wbstate->posix); + if (!wbstate->init || curr_alnum != wbstate->prev_alnum) + { wbstate->init = true; - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; wbstate->prev_alnum = curr_alnum; return prev_offset; } - wbstate->offset += unicode_utf8len(u); + wbstate->offset += ulen; } return wbstate->len; @@ -179,7 +190,7 @@ test_icu(void) { pg_unicode_category category = unicode_category(code); - if (category != PG_U_UNASSIGNED) + if (category != PG_U_UNASSIGNED && category != PG_U_SURROGATE) { uint8_t icu_category = u_charType(code); char code_str[5] = {0}; @@ -191,7 +202,7 @@ test_icu(void) } icu_test_simple(code); - unicode_to_utf8(code, (unsigned char *) code_str); + utf8encode((unsigned char *) code_str, 5, code); icu_test_full(code_str); successful++; @@ -337,6 +348,18 @@ test_convert_case(void) /* invalid UTF8: leading byte invalid length */ needed = unicode_strfold(NULL, 0, "abc\xF8xyz", 7, &consumed, false); Assert(needed == 3 && consumed == 3); + /* invalid UTF8: surrogates */ + needed = unicode_strfold(NULL, 0, "abc\xED\xA0\x81xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: continuation with no leading byte */ + needed = unicode_strfold(NULL, 0, "abc\x80xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: out of range */ + needed = unicode_strfold(NULL, 0, "abc\xF5\x80\x80\x80xyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); + /* invalid UTF8: overlong */ + needed = unicode_strfold(NULL, 0, "abc\xC1\xBFxyz", 7, &consumed, false); + Assert(needed == 3 && consumed == 3); #ifdef USE_ICU icu_test_full(""); diff --git a/src/common/unicode_case.c b/src/common/unicode_case.c index 24753aaab09..4d8ee71e8dc 100644 --- a/src/common/unicode_case.c +++ b/src/common/unicode_case.c @@ -194,22 +194,6 @@ unicode_strfold(char *dst, size_t dstsize, const char *src, size_t srclen, NULL, NULL); } -/* local version of pg_utf_mblen() to be inlinable */ -static int -utf8_mblen(const unsigned char *s) -{ - if ((*s & 0x80) == 0) - return 1; - else if ((*s & 0xe0) == 0xc0) - return 2; - else if ((*s & 0xf0) == 0xe0) - return 3; - else if ((*s & 0xf8) == 0xf0) - return 4; - else - return -1; -} - /* * Implement Unicode Default Case Conversion algorithm. * @@ -248,18 +232,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, while (srcoff < srclen) { - int u1len = utf8_mblen((const unsigned char *) src + srcoff); char32_t u1; + int u1len; char32_t simple = 0; const char32_t *special = NULL; enum CaseMapResult casemap_result; + u1len = utf8decode(&u1, (const unsigned char *) src + srcoff, + srclen - srcoff); + /* invalid UTF8 */ - if (u1len < 0 || srcoff + u1len > srclen) + if (u1len <= 0) break; - u1 = utf8_to_unicode((const unsigned char *) src + srcoff); - if (str_casekind == CaseTitle) { if (srcoff == boundary) @@ -280,6 +265,7 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, /* no mapping; copy bytes from src */ Assert(simple == 0); Assert(special == NULL); + if (result_len + u1len <= dstsize) memcpy(dst + result_len, src + srcoff, u1len); @@ -289,11 +275,19 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, { /* replace with single character */ char32_t u2 = simple; - char32_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } Assert(special == NULL); - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); + Assert(u2len > 0); result_len += u2len; } @@ -304,10 +298,18 @@ convert_case(char *dst, size_t dstsize, const char *src, size_t srclen, for (int i = 0; i < MAX_CASE_EXPANSION && special[i]; i++) { char32_t u2 = special[i]; - size_t u2len = unicode_utf8len(u2); + int u2len; + size_t remaining = 0; + unsigned char *p = NULL; + + if (dstsize > result_len) + { + remaining = dstsize - result_len; + p = (unsigned char *) dst + result_len; + } - if (result_len + u2len <= dstsize) - unicode_to_utf8(u2, (unsigned char *) dst + result_len); + u2len = utf8encode(p, remaining, u2); + Assert(u2len > 0); result_len += u2len; } @@ -352,13 +354,10 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) /* now at leading byte of previous sequence */ Assert((str[i] & 0x80) == 0 || (str[i] & 0xC0) == 0xC0); - ulen = utf8_mblen((const unsigned char *) str + i); - - /* invalid UTF8 */ - if (ulen < 0 || i + ulen > len) - return false; + ulen = utf8decode(&curr, (const unsigned char *) str + i, len - i); - curr = utf8_to_unicode((const unsigned char *) str + i); + if (ulen <= 0) + return false; /* invalid UTF8 */ if (!pg_u_prop_case_ignorable(curr)) { @@ -367,18 +366,18 @@ check_final_sigma(const unsigned char *str, size_t len, size_t offset) } } - ulen = utf8_mblen((const unsigned char *) str + offset); + ulen = utf8decode(&curr, (const unsigned char *) str + offset, + len - offset); + if (ulen <= 0) + return false; /* invalid UTF8 */ /* iterate forward looking for following character */ for (int i = offset + ulen; i < len;) { - ulen = utf8_mblen((const unsigned char *) str + i); - - /* invalid UTF8 */ - if (ulen < 0 || i + ulen > len) - return false; + ulen = utf8decode(&curr, (const unsigned char *) str + i, len - i); - curr = utf8_to_unicode((const unsigned char *) str + i); + if (ulen <= 0) + return false; /* invalid UTF8 */ if (!pg_u_prop_case_ignorable(curr)) { -- 2.43.0 ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-24 23:43 Jeff Davis <[email protected]> parent: Ayush Tiwari <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Jeff Davis @ 2026-06-24 23:43 UTC (permalink / raw) To: Ayush Tiwari <[email protected]>; +Cc: Chao Li <[email protected]>; pgsql-hackers On Wed, 2026-06-24 at 14:59 +0530, Ayush Tiwari wrote: > I took a look at the v5 series and tried it locally. The split > between the > backpatchable defensive change, the final-sigma fix, and the newer > utf8encode/utf8decode API work for master makes sense to me. Thank you for taking a look. > On my machine, case-check fails before reaching the new invalid-UTF8 > assertions. I think it's because PostgreSQL's Unicode tables are 17.0 > while my > system ICU is 15.1, so the exhaustive ICU comparison in test_icu() > hits a > changed mapping and exits first. Yes, that test is a bit awkward because most of it is a comparison to results from ICU, so if ICU is unavailable or based on an older version of Unicode, then the test doesn't work. > Would it be worth running test_convert_case() before > test_icu()? If we want to make those independent of ICU, I think we'd move them to a regular test suite that's exercised everywhere (not just as part of the 'update-unicode' target). But if we did so, that would be a very small test suite, because most of the results are already checked in the normal SQL tests. What makes these tests different is that they are exercising invalid UTF8 behavior, which we don't expect to happen through ordinary SQL. Regards, Jeff Davis ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-25 04:10 Chao Li <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Chao Li @ 2026-06-25 04:10 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Jun 25, 2026, at 05:57, Jeff Davis <[email protected]> wrote: > > On Wed, 2026-06-24 at 16:44 +0800, Chao Li wrote: >> There is a compile warning against pg_wchar.h in 0004: > > 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. > > Regards, > Jeff Davis > > <v5-0001-unicode_case.c-defend-against-invalid-UTF8.patch><v5-0002-pg_unicode_fast-fix-final-sigma-logic.patch><v5-0003-unicode_case.c-change-API-to-signal-UTF8-decoding.patch><v5-0004-Validating-iterator-friendly-UTF8-encoder-decoder.patch><v5-0005-unicode_case.c-use-new-utf8encode-utf8decode-APIs.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) == 0) return (char32_t) c[0]; else if ((*c & 0xe0) == 0xc0) return (char32_t) (((c[0] & 0x1f) << 6) | (c[1] & 0x3f)); else if ((*c & 0xf0) == 0xe0) return (char32_t) (((c[0] & 0x0f) << 12) | ((c[1] & 0x3f) << 6) | (c[2] & 0x3f)); else if ((*c & 0xf8) == 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) == 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/ ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-25 17:38 Jeff Davis <[email protected]> parent: Chao Li <[email protected]> 0 siblings, 1 reply; 18+ messages in thread From: Jeff Davis @ 2026-06-25 17:38 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: pgsql-hackers On Thu, 2026-06-25 at 12:10 +0800, Chao Li wrote: > 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. Correct. We don't do full UTF8 validation until the last patch in the series, which is not being backported. Trying to do full validation in the backbranches seems more likely to cause problems than prevent them. We aren't expecting invalid UTF8, but in the event it got there somehow (perhaps from an old upgraded instance), throwing errors after a minor release is probably not helpful. Even in master, I am not 100% sure we want to detect other kinds of validation errors while processing the UTF8. By the time we are using the value, maybe truncated multibyte sequences are the only thing we care about, and we just need to be sure the code can handle anything that fits in a char32_t. Another thing to consider is an embedded NUL character, which is valid UTF8 but not valid in a TEXT value. Regards, Jeff Davis ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-26 04:38 Chao Li <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 2 replies; 18+ messages in thread From: Chao Li @ 2026-06-26 04:38 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Jun 26, 2026, at 01:38, Jeff Davis <[email protected]> wrote: > > On Thu, 2026-06-25 at 12:10 +0800, Chao Li wrote: >> 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. > > Correct. We don't do full UTF8 validation until the last patch in the > series, which is not being backported. > 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? 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;) ``` As offset is of type size_t, the loop variable i is better to be size_t. 0003 - looks good. 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. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-27 14:02 Jeff Davis <[email protected]> parent: Chao Li <[email protected]> 1 sibling, 1 reply; 18+ messages in thread From: Jeff Davis @ 2026-06-27 14:02 UTC (permalink / raw) To: Chao Li <[email protected]>; +Cc: 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? Right. Would "defend against truncated byte sequences" or "defend against truncated UTF8" be better wording? Regards, Jeff Davis ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-06-28 22:29 Chao Li <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Chao Li @ 2026-06-28 22:29 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Jun 27, 2026, at 22:02, Jeff Davis <[email protected]> wrote: > > 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? > > Right. Would "defend against truncated byte sequences" or "defend > against truncated UTF8" be better wording? > > Regards, > Jeff Davis > Yes, I think they are better. “Truncated” is more specific than “invalid”. I'm slightly more keen on the second phrase. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Small patch to improve safety of utf8_to_unicode(). @ 2026-07-08 16:48 Chao Li <[email protected]> parent: Chao Li <[email protected]> 1 sibling, 0 replies; 18+ messages in thread From: Chao Li @ 2026-07-08 16:48 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: pgsql-hackers > On Jul 7, 2026, at 17:56, Jeff Davis <[email protected]> wrote: > > 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 > > <v6-0001-Validating-iterator-friendly-UTF8-encoder-decoder.patch><v6-0002-unicode_case.c-use-new-utf8encode-utf8decode-APIs.patch> V6 looks good to me. Best regards, — Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/ ^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2026-07-08 16:48 UTC | newest] Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-12-12 23:24 Small patch to improve safety of utf8_to_unicode(). Jeff Davis <[email protected]> 2025-12-13 23:22 ` Chao Li <[email protected]> 2025-12-15 20:23 ` Jeff Davis <[email protected]> 2025-12-15 23:34 ` Chao Li <[email protected]> 2025-12-17 19:37 ` Jeff Davis <[email protected]> 2026-06-19 23:22 ` Jeff Davis <[email protected]> 2026-06-23 02:02 ` Jeff Davis <[email protected]> 2026-06-24 05:45 ` Jeff Davis <[email protected]> 2026-06-24 08:44 ` Chao Li <[email protected]> 2026-06-24 21:57 ` Jeff Davis <[email protected]> 2026-06-25 04:10 ` Chao Li <[email protected]> 2026-06-25 17:38 ` Jeff Davis <[email protected]> 2026-06-26 04:38 ` Chao Li <[email protected]> 2026-06-27 14:02 ` Jeff Davis <[email protected]> 2026-06-28 22:29 ` Chao Li <[email protected]> 2026-07-08 16:48 ` Chao Li <[email protected]> 2026-06-24 09:29 ` Ayush Tiwari <[email protected]> 2026-06-24 23:43 ` Jeff Davis <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox