Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 8981ff5

Browse files
macdiceCommitfest Bot
authored and
Commitfest Bot
committed
Remove setlocale() from check_locale().
Validate locale names with newlocale() or _create_locale() instead, to avoid clobbering global state. This also removes the "canonicalization" of the locale name, which previously had two user-visible effects: 1. CREATE DATABASE ... LOCALE = '' would look up the locale from environment variables, but this was not documented behavior and doesn't seem too useful. A default will normally be inherited from the template if you just leave the option off. (Note that initdb still chooses default values from the server environment, and that would often be the original source of the template database's values.) 2. On Windows only, the setlocale() step reached by CREATE DATABASE ... LOCALE = '...' did accept a wide range of formats and do some canonicalization, when using (deprecated) pre-BCP 47 locale names, but again it seems enough to let that happen in the initdb phase only. Now, CREATE DATABASE and SET lc_XXX reject ''. CREATE COLLATION continues to accept it, as it never recorded canonicalized values so there may be system catalogs containing verbatim '' in the wild. We'll need to research the upgrade implications before banning it there. Thanks to Peter Eisentraut and Tom Lane for the suggestion that we might not really need to preserve those behaviors in the backend, in our hunt for non-thread-safe code. Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi> Discussion: https://postgr.es/m/CA%2BhUKGJqVe0%2BPv9dvC9dSums_PXxGo9SWcxYAMBguWJUGbWz-A%40mail.gmail.com Discussion: https://postgr.es/m/CA%2BhUKGK57sgUYKO03jB4VarTsswfMyScFAyJpVnYD8c%2Bg12_mg%40mail.gmail.com
1 parent b53b881 commit 8981ff5

File tree

4 files changed

+70
-52
lines changed

4 files changed

+70
-52
lines changed

src/backend/commands/dbcommands.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,6 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
730730
const char *dblocale = NULL;
731731
char *dbicurules = NULL;
732732
char dblocprovider = '\0';
733-
char *canonname;
734733
int encoding = -1;
735734
bool dbistemplate = false;
736735
bool dballowconnections = true;
@@ -1064,18 +1063,16 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt)
10641063
errmsg("invalid server encoding %d", encoding)));
10651064

10661065
/* Check that the chosen locales are valid, and get canonical spellings */
1067-
if (!check_locale(LC_COLLATE, dbcollate, &canonname))
1066+
if (!check_locale(LC_COLLATE, dbcollate))
10681067
ereport(ERROR,
10691068
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
10701069
errmsg("invalid LC_COLLATE locale name: \"%s\"", dbcollate),
10711070
errhint("If the locale name is specific to ICU, use ICU_LOCALE.")));
1072-
dbcollate = canonname;
1073-
if (!check_locale(LC_CTYPE, dbctype, &canonname))
1071+
if (!check_locale(LC_CTYPE, dbctype))
10741072
ereport(ERROR,
10751073
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
10761074
errmsg("invalid LC_CTYPE locale name: \"%s\"", dbctype),
10771075
errhint("If the locale name is specific to ICU, use ICU_LOCALE.")));
1078-
dbctype = canonname;
10791076

10801077
check_encoding_locale_matches(encoding, dbcollate, dbctype);
10811078

src/backend/utils/adt/pg_locale.c

Lines changed: 67 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,36 @@ static pg_locale_t last_collation_cache_locale = NULL;
172172
static char *IsoLocaleName(const char *);
173173
#endif
174174

175+
#ifndef WIN32
176+
/*
177+
* The newlocale() function needs LC_xxx_MASK, but sometimes we have LC_xxx,
178+
* and POSIX doesn't offer a way to translate.
179+
*/
180+
static int
181+
get_lc_category_mask(int category)
182+
{
183+
switch (category)
184+
{
185+
case LC_COLLATE:
186+
return LC_COLLATE_MASK;
187+
case LC_CTYPE:
188+
return LC_CTYPE_MASK;
189+
#ifdef LC_MESSAGES
190+
case LC_MESSAGES:
191+
return LC_MESSAGES_MASK;
192+
#endif
193+
case LC_MONETARY:
194+
return LC_MONETARY_MASK;
195+
case LC_NUMERIC:
196+
return LC_NUMERIC_MASK;
197+
case LC_TIME:
198+
return LC_TIME_MASK;
199+
default:
200+
return 0;
201+
};
202+
}
203+
#endif
204+
175205
/*
176206
* pg_perm_setlocale
177207
*
@@ -281,19 +311,11 @@ pg_perm_setlocale(int category, const char *locale)
281311

282312
/*
283313
* Is the locale name valid for the locale category?
284-
*
285-
* If successful, and canonname isn't NULL, a palloc'd copy of the locale's
286-
* canonical name is stored there. This is especially useful for figuring out
287-
* what locale name "" means (ie, the server environment value). (Actually,
288-
* it seems that on most implementations that's the only thing it's good for;
289-
* we could wish that setlocale gave back a canonically spelled version of
290-
* the locale name, but typically it doesn't.)
291314
*/
292315
bool
293-
check_locale(int category, const char *locale, char **canonname)
316+
check_locale(int category, const char *locale)
294317
{
295-
char *save;
296-
char *res;
318+
locale_t loc;
297319

298320
/* Don't let Windows' non-ASCII locale names in. */
299321
if (!pg_is_ascii(locale))
@@ -305,41 +327,42 @@ check_locale(int category, const char *locale, char **canonname)
305327
return false;
306328
}
307329

308-
if (canonname)
309-
*canonname = NULL; /* in case of failure */
310-
311-
save = setlocale(category, NULL);
312-
if (!save)
313-
return false; /* won't happen, we hope */
314-
315-
/* save may be pointing at a modifiable scratch variable, see above. */
316-
save = pstrdup(save);
317-
318-
/* set the locale with setlocale, to see if it accepts it. */
319-
res = setlocale(category, locale);
320-
321-
/* save canonical name if requested. */
322-
if (res && canonname)
323-
*canonname = pstrdup(res);
324-
325-
/* restore old value. */
326-
if (!setlocale(category, save))
327-
elog(WARNING, "failed to restore old locale \"%s\"", save);
328-
pfree(save);
330+
if (locale[0] == 0)
331+
{
332+
/*
333+
* We only accept explicit locale names, not "". We don't want to
334+
* rely on environment variables in the backend.
335+
*/
336+
return false;
337+
}
329338

330-
/* Don't let Windows' non-ASCII locale names out. */
331-
if (canonname && *canonname && !pg_is_ascii(*canonname))
339+
/*
340+
* See if we can open it. Unfortunately we can't always distinguish
341+
* out-of-memory from invalid locale name.
342+
*/
343+
errno = ENOENT;
344+
#ifdef WIN32
345+
loc = _create_locale(category, locale);
346+
if (loc == (locale_t) 0)
347+
_dosmaperr(GetLastError());
348+
#else
349+
loc = newlocale(get_lc_category_mask(category), locale, (locale_t) 0);
350+
#endif
351+
if (loc == (locale_t) 0)
332352
{
333-
ereport(WARNING,
334-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
335-
errmsg("locale name \"%s\" contains non-ASCII characters",
336-
*canonname)));
337-
pfree(*canonname);
338-
*canonname = NULL;
353+
if (errno == ENOMEM)
354+
elog(ERROR, "out of memory");
355+
356+
/* Otherwise assume the locale doesn't exist. */
339357
return false;
340358
}
359+
#ifdef WIN32
360+
_free_locale(loc);
361+
#else
362+
freelocale(loc);
363+
#endif
341364

342-
return (res != NULL);
365+
return true;
343366
}
344367

345368

@@ -357,7 +380,7 @@ check_locale(int category, const char *locale, char **canonname)
357380
bool
358381
check_locale_monetary(char **newval, void **extra, GucSource source)
359382
{
360-
return check_locale(LC_MONETARY, *newval, NULL);
383+
return check_locale(LC_MONETARY, *newval);
361384
}
362385

363386
void
@@ -369,7 +392,7 @@ assign_locale_monetary(const char *newval, void *extra)
369392
bool
370393
check_locale_numeric(char **newval, void **extra, GucSource source)
371394
{
372-
return check_locale(LC_NUMERIC, *newval, NULL);
395+
return check_locale(LC_NUMERIC, *newval);
373396
}
374397

375398
void
@@ -381,7 +404,7 @@ assign_locale_numeric(const char *newval, void *extra)
381404
bool
382405
check_locale_time(char **newval, void **extra, GucSource source)
383406
{
384-
return check_locale(LC_TIME, *newval, NULL);
407+
return check_locale(LC_TIME, *newval);
385408
}
386409

387410
void
@@ -417,7 +440,7 @@ check_locale_messages(char **newval, void **extra, GucSource source)
417440
* On Windows, we can't even check the value, so accept blindly
418441
*/
419442
#if defined(LC_MESSAGES) && !defined(WIN32)
420-
return check_locale(LC_MESSAGES, *newval, NULL);
443+
return check_locale(LC_MESSAGES, *newval);
421444
#else
422445
return true;
423446
#endif

src/bin/initdb/initdb.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2203,8 +2203,6 @@ locale_date_order(const char *locale)
22032203
* it seems that on most implementations that's the only thing it's good for;
22042204
* we could wish that setlocale gave back a canonically spelled version of
22052205
* the locale name, but typically it doesn't.)
2206-
*
2207-
* this should match the backend's check_locale() function
22082206
*/
22092207
static void
22102208
check_locale_name(int category, const char *locale, char **canonname)

src/include/utils/pg_locale.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ extern PGDLLIMPORT char *localized_full_months[];
3535
/* is the databases's LC_CTYPE the C locale? */
3636
extern PGDLLIMPORT bool database_ctype_is_c;
3737

38-
extern bool check_locale(int category, const char *locale, char **canonname);
38+
extern bool check_locale(int category, const char *locale);
3939
extern char *pg_perm_setlocale(int category, const char *locale);
4040

4141
/*

0 commit comments

Comments
 (0)