From a8da929f5961c169b359ad93f31f59b8baac00c8 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Wed, 7 Aug 2024 11:05:46 -0700 Subject: [PATCH v2 2/6] Tighten up make_libc_collator() and make_icu_collator(). Return the result rather than using an out parameter, and make it the caller's responsibility to copy it into the right context. Ensure that no paths leak a collator. The function make_icu_collator() doesn't have any external callers, so change it to be static. Also, when re-opening with rules, close the previous collator before there's a chance for errors. In make_libc_collator(), if the first newlocale() succeeds and the second one fails, close the first locale_t object. --- src/backend/utils/adt/pg_locale.c | 75 +++++++++++++++++++------------ src/include/utils/pg_locale.h | 4 -- 2 files changed, 47 insertions(+), 32 deletions(-) diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c index 1668236e779..5a3dccf757b 100644 --- a/src/backend/utils/adt/pg_locale.c +++ b/src/backend/utils/adt/pg_locale.c @@ -1302,14 +1302,15 @@ report_newlocale_failure(const char *localename) } /* - * Initialize the locale_t field. + * Create a locale_t with the given collation and ctype. * - * The "C" and "POSIX" locales are not actually handled by libc, so set the - * locale_t to zero in that case. + * The "C" and "POSIX" locales are not actually handled by libc, so return + * NULL. + * + * Ensure that no path leaks a locale_t. */ -static void -make_libc_collator(const char *collate, const char *ctype, - pg_locale_t result) +static locale_t +make_libc_collator(const char *collate, const char *ctype) { locale_t loc = 0; @@ -1348,7 +1349,11 @@ make_libc_collator(const char *collate, const char *ctype, errno = 0; loc = newlocale(LC_CTYPE_MASK, ctype, loc1); if (!loc) + { + if (loc1) + freelocale(loc1); report_newlocale_failure(ctype); + } } else loc = loc1; @@ -1365,15 +1370,18 @@ make_libc_collator(const char *collate, const char *ctype, #endif } - result->info.lt = loc; + return loc; } -void -make_icu_collator(const char *iculocstr, - const char *icurules, - struct pg_locale_struct *resultp) -{ +/* + * Create a UCollator with the given locale string and rules. + * + * Ensure that no path leaks a UCollator. + */ #ifdef USE_ICU +static UCollator * +make_icu_collator(const char *iculocstr, const char *icurules) +{ UCollator *collator; collator = pg_ucol_open(iculocstr); @@ -1391,14 +1399,14 @@ make_icu_collator(const char *iculocstr, int32_t length; default_rules = ucol_getRules(collator, &length); + ucol_close(collator); + icu_to_uchar(&my_rules, icurules, strlen(icurules)); agg_rules = palloc_array(UChar, u_strlen(default_rules) + u_strlen(my_rules) + 1); u_strcpy(agg_rules, default_rules); u_strcat(agg_rules, my_rules); - ucol_close(collator); - status = U_ZERO_ERROR; collator = ucol_openRules(agg_rules, u_strlen(agg_rules), UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH, NULL, &status); @@ -1409,16 +1417,9 @@ make_icu_collator(const char *iculocstr, iculocstr, icurules, u_errorName(status)))); } - /* We will leak this string if the caller errors later :-( */ - resultp->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr); - resultp->info.icu.ucol = collator; -#else /* not USE_ICU */ - /* could get here if a collation was created by a build with ICU */ - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("ICU is not supported in this build"))); -#endif /* not USE_ICU */ + return collator; } +#endif /* not USE_ICU */ bool @@ -1436,7 +1437,6 @@ init_database_collation(void) HeapTuple tup; Form_pg_database dbform; Datum datum; - bool isnull; /* Fetch our pg_database row normally, via syscache */ tup = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId)); @@ -1461,8 +1461,10 @@ init_database_collation(void) } else if (dbform->datlocprovider == COLLPROVIDER_ICU) { +#ifdef USE_ICU char *datlocale; char *icurules; + bool isnull; datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datlocale); datlocale = TextDatumGetCString(datum); @@ -1476,7 +1478,14 @@ init_database_collation(void) else icurules = NULL; - make_icu_collator(datlocale, icurules, &default_locale); + default_locale.info.icu.ucol = make_icu_collator(datlocale, icurules); + default_locale.info.icu.locale = MemoryContextStrdup(TopMemoryContext, datlocale); +#else /* not USE_ICU */ + /* could get here if a collation was created by a build with ICU */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("ICU is not supported in this build"))); +#endif /* not USE_ICU */ } else { @@ -1495,7 +1504,7 @@ init_database_collation(void) default_locale.ctype_is_c = (strcmp(datctype, "C") == 0) || (strcmp(datctype, "POSIX") == 0); - make_libc_collator(datcollate, datctype, &default_locale); + default_locale.info.lt = make_libc_collator(datcollate, datctype); } default_locale.provider = dbform->datlocprovider; @@ -1568,10 +1577,11 @@ create_pg_locale(Oid collid) result.ctype_is_c = (strcmp(collctype, "C") == 0) || (strcmp(collctype, "POSIX") == 0); - make_libc_collator(collcollate, collctype, &result); + result.info.lt = make_libc_collator(collcollate, collctype); } else if (collform->collprovider == COLLPROVIDER_ICU) { +#ifdef USE_ICU const char *iculocstr; const char *icurules; @@ -1587,7 +1597,14 @@ create_pg_locale(Oid collid) else icurules = NULL; - make_icu_collator(iculocstr, icurules, &result); + result.info.icu.ucol = make_icu_collator(iculocstr, icurules); + result.info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr); +#else /* not USE_ICU */ + /* could get here if a collation was created by a build with ICU */ + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("ICU is not supported in this build"))); +#endif /* not USE_ICU */ } datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion, @@ -2530,6 +2547,8 @@ builtin_validate_locale(int encoding, const char *locale) /* * Wrapper around ucol_open() to handle API differences for older ICU * versions. + * + * Ensure that no path leaks a UCollator. */ static UCollator * pg_ucol_open(const char *loc_str) diff --git a/src/include/utils/pg_locale.h b/src/include/utils/pg_locale.h index f41d33975be..1ca0b7fa74b 100644 --- a/src/include/utils/pg_locale.h +++ b/src/include/utils/pg_locale.h @@ -107,10 +107,6 @@ struct pg_locale_struct typedef struct pg_locale_struct *pg_locale_t; -extern void make_icu_collator(const char *iculocstr, - const char *icurules, - struct pg_locale_struct *resultp); - extern bool pg_locale_deterministic(pg_locale_t locale); extern void init_database_collation(void); extern pg_locale_t pg_newlocale_from_collation(Oid collid); -- 2.34.1