Skip to content

Commit ceeaaed

Browse files
jeff-davisjeff-davis
committed
Tighten up make_libc_collator() and make_icu_collator().
Ensure that error paths within these functions do not leak a collator, and return the result rather than using an out parameter. (Error paths in the caller may still result in a leaked collator, which will be addressed separately.) In make_libc_collator(), if the first newlocale() succeeds and the second one fails, close the first locale_t object. The function make_icu_collator() doesn't have any external callers, so change it to be static. Discussion: https://postgr.es/m/54d20e812bd6c3e44c10eddcd757ec494ebf1803.camel@j-davis.com
1 parent 59f0eea commit ceeaaed

File tree

2 files changed

+111
-69
lines changed

2 files changed

+111
-69
lines changed

‎src/backend/utils/adt/pg_locale.c

+111-65
Original file line numberDiff line numberDiff line change
@@ -1297,14 +1297,15 @@ report_newlocale_failure(const char *localename)
12971297
}
12981298

12991299
/*
1300-
* Initialize the locale_t field.
1300+
* Create a locale_t with the given collation and ctype.
13011301
*
1302-
* The "C" and "POSIX" locales are not actually handled by libc, so set the
1303-
* locale_t to zero in that case.
1302+
* The "C" and "POSIX" locales are not actually handled by libc, so return
1303+
* NULL.
1304+
*
1305+
* Ensure that no path leaks a locale_t.
13041306
*/
1305-
static void
1306-
make_libc_collator(const char *collate, const char *ctype,
1307-
pg_locale_t result)
1307+
static locale_t
1308+
make_libc_collator(const char *collate, const char *ctype)
13081309
{
13091310
locale_t loc = 0;
13101311

@@ -1343,7 +1344,11 @@ make_libc_collator(const char *collate, const char *ctype,
13431344
errno = 0;
13441345
loc = newlocale(LC_CTYPE_MASK, ctype, loc1);
13451346
if (!loc)
1347+
{
1348+
if (loc1)
1349+
freelocale(loc1);
13461350
report_newlocale_failure(ctype);
1351+
}
13471352
}
13481353
else
13491354
loc = loc1;
@@ -1360,60 +1365,78 @@ make_libc_collator(const char *collate, const char *ctype,
13601365
#endif
13611366
}
13621367

1363-
result->info.lt = loc;
1368+
return loc;
13641369
}
13651370

1366-
void
1367-
make_icu_collator(const char *iculocstr,
1368-
const char *icurules,
1369-
struct pg_locale_struct *resultp)
1370-
{
1371+
/*
1372+
* Create a UCollator with the given locale string and rules.
1373+
*
1374+
* Ensure that no path leaks a UCollator.
1375+
*/
13711376
#ifdef USE_ICU
1372-
UCollator *collator;
1373-
1374-
collator = pg_ucol_open(iculocstr);
1375-
1376-
/*
1377-
* If rules are specified, we extract the rules of the standard collation,
1378-
* add our own rules, and make a new collator with the combined rules.
1379-
*/
1380-
if (icurules)
1377+
static UCollator *
1378+
make_icu_collator(const char *iculocstr, const char *icurules)
1379+
{
1380+
if (!icurules)
1381+
{
1382+
/* simple case without rules */
1383+
return pg_ucol_open(iculocstr);
1384+
}
1385+
else
13811386
{
1382-
const UChar *default_rules;
1383-
UChar *agg_rules;
1387+
UCollator *collator_std_rules;
1388+
UCollator *collator_all_rules;
1389+
const UChar *std_rules;
13841390
UChar *my_rules;
1385-
UErrorCode status;
1391+
UChar *all_rules;
13861392
int32_t length;
1393+
int32_t total;
1394+
UErrorCode status;
13871395

1388-
default_rules = ucol_getRules(collator, &length);
1396+
/*
1397+
* If rules are specified, we extract the rules of the standard
1398+
* collation, add our own rules, and make a new collator with the
1399+
* combined rules.
1400+
*/
13891401
icu_to_uchar(&my_rules, icurules, strlen(icurules));
13901402

1391-
agg_rules = palloc_array(UChar, u_strlen(default_rules) + u_strlen(my_rules) + 1);
1392-
u_strcpy(agg_rules, default_rules);
1393-
u_strcat(agg_rules, my_rules);
1403+
collator_std_rules = pg_ucol_open(iculocstr);
13941404

1395-
ucol_close(collator);
1405+
std_rules = ucol_getRules(collator_std_rules, &length);
1406+
1407+
total = u_strlen(std_rules) + u_strlen(my_rules) + 1;
1408+
1409+
/* avoid leaking collator on OOM */
1410+
all_rules = palloc_extended(sizeof(UChar) * total, MCXT_ALLOC_NO_OOM);
1411+
if (!all_rules)
1412+
{
1413+
ucol_close(collator_std_rules);
1414+
ereport(ERROR,
1415+
(errcode(ERRCODE_OUT_OF_MEMORY),
1416+
errmsg("out of memory")));
1417+
}
1418+
1419+
u_strcpy(all_rules, std_rules);
1420+
u_strcat(all_rules, my_rules);
1421+
1422+
ucol_close(collator_std_rules);
13961423

13971424
status = U_ZERO_ERROR;
1398-
collator = ucol_openRules(agg_rules, u_strlen(agg_rules),
1399-
UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH, NULL, &status);
1425+
collator_all_rules = ucol_openRules(all_rules, u_strlen(all_rules),
1426+
UCOL_DEFAULT, UCOL_DEFAULT_STRENGTH,
1427+
NULL, &status);
14001428
if (U_FAILURE(status))
1429+
{
14011430
ereport(ERROR,
14021431
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
14031432
errmsg("could not open collator for locale \"%s\" with rules \"%s\": %s",
14041433
iculocstr, icurules, u_errorName(status))));
1405-
}
1434+
}
14061435

1407-
/* We will leak this string if the caller errors later :-( */
1408-
resultp->info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
1409-
resultp->info.icu.ucol = collator;
1410-
#else /* not USE_ICU */
1411-
/* could get here if a collation was created by a build with ICU */
1412-
ereport(ERROR,
1413-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1414-
errmsg("ICU is not supported in this build")));
1415-
#endif /* not USE_ICU */
1436+
return collator_all_rules;
1437+
}
14161438
}
1439+
#endif /* not USE_ICU */
14171440

14181441
/*
14191442
* Initialize default_locale with database locale settings.
@@ -1424,7 +1447,6 @@ init_database_collation(void)
14241447
HeapTuple tup;
14251448
Form_pg_database dbform;
14261449
Datum datum;
1427-
bool isnull;
14281450

14291451
/* Fetch our pg_database row normally, via syscache */
14301452
tup = SearchSysCache1(DATABASEOID, ObjectIdGetDatum(MyDatabaseId));
@@ -1449,8 +1471,10 @@ init_database_collation(void)
14491471
}
14501472
else if (dbform->datlocprovider == COLLPROVIDER_ICU)
14511473
{
1474+
#ifdef USE_ICU
14521475
char *datlocale;
14531476
char *icurules;
1477+
bool isnull;
14541478

14551479
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datlocale);
14561480
datlocale = TextDatumGetCString(datum);
@@ -1464,15 +1488,20 @@ init_database_collation(void)
14641488
else
14651489
icurules = NULL;
14661490

1467-
make_icu_collator(datlocale, icurules, &default_locale);
1491+
default_locale.info.icu.locale = MemoryContextStrdup(TopMemoryContext, datlocale);
1492+
default_locale.info.icu.ucol = make_icu_collator(datlocale, icurules);
1493+
#else
1494+
/* could get here if a collation was created by a build with ICU */
1495+
ereport(ERROR,
1496+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1497+
errmsg("ICU is not supported in this build")));
1498+
#endif
14681499
}
1469-
else
1500+
else if (dbform->datlocprovider == COLLPROVIDER_LIBC)
14701501
{
14711502
const char *datcollate;
14721503
const char *datctype;
14731504

1474-
Assert(dbform->datlocprovider == COLLPROVIDER_LIBC);
1475-
14761505
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datcollate);
14771506
datcollate = TextDatumGetCString(datum);
14781507
datum = SysCacheGetAttrNotNull(DATABASEOID, tup, Anum_pg_database_datctype);
@@ -1483,8 +1512,12 @@ init_database_collation(void)
14831512
default_locale.ctype_is_c = (strcmp(datctype, "C") == 0) ||
14841513
(strcmp(datctype, "POSIX") == 0);
14851514

1486-
make_libc_collator(datcollate, datctype, &default_locale);
1515+
default_locale.info.lt = make_libc_collator(datcollate, datctype);
14871516
}
1517+
else
1518+
/* shouldn't happen */
1519+
PGLOCALE_SUPPORT_ERROR(dbform->datlocprovider);
1520+
14881521

14891522
default_locale.provider = dbform->datlocprovider;
14901523

@@ -1557,25 +1590,9 @@ pg_newlocale_from_collation(Oid collid)
15571590
result.info.builtin.locale = MemoryContextStrdup(TopMemoryContext,
15581591
locstr);
15591592
}
1560-
else if (collform->collprovider == COLLPROVIDER_LIBC)
1561-
{
1562-
const char *collcollate;
1563-
const char *collctype;
1564-
1565-
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collcollate);
1566-
collcollate = TextDatumGetCString(datum);
1567-
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
1568-
collctype = TextDatumGetCString(datum);
1569-
1570-
result.collate_is_c = (strcmp(collcollate, "C") == 0) ||
1571-
(strcmp(collcollate, "POSIX") == 0);
1572-
result.ctype_is_c = (strcmp(collctype, "C") == 0) ||
1573-
(strcmp(collctype, "POSIX") == 0);
1574-
1575-
make_libc_collator(collcollate, collctype, &result);
1576-
}
15771593
else if (collform->collprovider == COLLPROVIDER_ICU)
15781594
{
1595+
#ifdef USE_ICU
15791596
const char *iculocstr;
15801597
const char *icurules;
15811598

@@ -1591,8 +1608,35 @@ pg_newlocale_from_collation(Oid collid)
15911608
else
15921609
icurules = NULL;
15931610

1594-
make_icu_collator(iculocstr, icurules, &result);
1611+
result.info.icu.locale = MemoryContextStrdup(TopMemoryContext, iculocstr);
1612+
result.info.icu.ucol = make_icu_collator(iculocstr, icurules);
1613+
#else
1614+
/* could get here if a collation was created by a build with ICU */
1615+
ereport(ERROR,
1616+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
1617+
errmsg("ICU is not supported in this build")));
1618+
#endif
15951619
}
1620+
else if (collform->collprovider == COLLPROVIDER_LIBC)
1621+
{
1622+
const char *collcollate;
1623+
const char *collctype;
1624+
1625+
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collcollate);
1626+
collcollate = TextDatumGetCString(datum);
1627+
datum = SysCacheGetAttrNotNull(COLLOID, tp, Anum_pg_collation_collctype);
1628+
collctype = TextDatumGetCString(datum);
1629+
1630+
result.collate_is_c = (strcmp(collcollate, "C") == 0) ||
1631+
(strcmp(collcollate, "POSIX") == 0);
1632+
result.ctype_is_c = (strcmp(collctype, "C") == 0) ||
1633+
(strcmp(collctype, "POSIX") == 0);
1634+
1635+
result.info.lt = make_libc_collator(collcollate, collctype);
1636+
}
1637+
else
1638+
/* shouldn't happen */
1639+
PGLOCALE_SUPPORT_ERROR(collform->collprovider);
15961640

15971641
datum = SysCacheGetAttr(COLLOID, tp, Anum_pg_collation_collversion,
15981642
&isnull);
@@ -2500,6 +2544,8 @@ builtin_validate_locale(int encoding, const char *locale)
25002544
/*
25012545
* Wrapper around ucol_open() to handle API differences for older ICU
25022546
* versions.
2547+
*
2548+
* Ensure that no path leaks a UCollator.
25032549
*/
25042550
static UCollator *
25052551
pg_ucol_open(const char *loc_str)

‎src/include/utils/pg_locale.h

-4
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,6 @@ struct pg_locale_struct
104104

105105
typedef struct pg_locale_struct *pg_locale_t;
106106

107-
extern void make_icu_collator(const char *iculocstr,
108-
const char *icurules,
109-
struct pg_locale_struct *resultp);
110-
111107
extern void init_database_collation(void);
112108
extern pg_locale_t pg_newlocale_from_collation(Oid collid);
113109

0 commit comments

Comments
 (0)