Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit2d870b4

Browse files
committed
Allow ALTER SYSTEM to set unrecognized custom GUCs.
Previously, ALTER SYSTEM failed if the target GUC wasn't present inthe session's GUC hashtable. That is a reasonable behavior for core(single-part) GUC names, and for custom GUCs for which we have loadedan extension that's reserved the prefix. But it's unnecessarilyrestrictive otherwise, and it also causes inconsistent behavior:you can "ALTER SYSTEM SET foo.bar" only if you did "SET foo.bar"earlier in the session. That's fairly silly.Hence, refactor things so that we can execute ALTER SYSTEM evenif the variable doesn't have a GUC hashtable entry, as long as thename meets the custom-variable naming requirements and does nothave a reserved prefix. (It's safe to do this even if thevariable belongs to an extension we currently don't have loaded.A bad value will at worst cause a WARNING when the extensiondoes get loaded.)Also, adjust GRANT ON PARAMETER to have the same opinions aboutwhether to allow an unrecognized GUC name, and to throw thesame errors if not (it previously used a one-size-fits-allmessage for several distinguishable conditions). By default,only a superuser will be allowed to do ALTER SYSTEM SET on anunrecognized name, but it's possible to GRANT the ability todo it.Patch by me, pursuant to a documentation complaint fromGavin Panella. Arguably this is a bug fix, but given thelack of other complaints I'll refrain from back-patching.Discussion:https://postgr.es/m/2617358.1697501956@sss.pgh.pa.usDiscussion:https://postgr.es/m/169746329791.169914.16613647309012285391@wrigleys.postgresql.org
1 parent36a14af commit2d870b4

File tree

5 files changed

+170
-96
lines changed

5 files changed

+170
-96
lines changed

‎src/backend/catalog/pg_parameter_acl.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,7 @@ ParameterAclCreate(const char *parameter)
8282
* To prevent cluttering pg_parameter_acl with useless entries, insist
8383
* that the name be valid.
8484
*/
85-
if (!check_GUC_name_for_parameter_acl(parameter))
86-
ereport(ERROR,
87-
(errcode(ERRCODE_INVALID_NAME),
88-
errmsg("invalid parameter name \"%s\"",
89-
parameter)));
85+
check_GUC_name_for_parameter_acl(parameter);
9086

9187
/* Convert name to the form it should have in pg_parameter_acl. */
9288
parname=convert_GUC_name_for_parameter_acl(parameter);

‎src/backend/utils/misc/guc.c

Lines changed: 129 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ static void write_auto_conf_file(int fd, const char *filename, ConfigVariable *h
250250
staticvoidreplace_auto_config_value(ConfigVariable**head_p,ConfigVariable**tail_p,
251251
constchar*name,constchar*value);
252252
staticboolvalid_custom_variable_name(constchar*name);
253+
staticboolassignable_custom_variable_name(constchar*name,boolskip_errors,
254+
intelevel);
253255
staticvoiddo_serialize(char**destptr,Size*maxbytes,
254256
constchar*fmt,...)pg_attribute_printf(3,4);
255257
staticboolcall_bool_check_hook(structconfig_bool*conf,bool*newval,
@@ -1063,7 +1065,7 @@ add_guc_variable(struct config_generic *var, int elevel)
10631065
*
10641066
* It must be two or more identifiers separated by dots, where the rules
10651067
* for what is an identifier agree with scan.l. (If you change this rule,
1066-
* adjust the errdetail infind_option().)
1068+
* adjust the errdetail inassignable_custom_variable_name().)
10671069
*/
10681070
staticbool
10691071
valid_custom_variable_name(constchar*name)
@@ -1098,6 +1100,71 @@ valid_custom_variable_name(const char *name)
10981100
returnsaw_sep;
10991101
}
11001102

1103+
/*
1104+
* Decide whether an unrecognized variable name is allowed to be SET.
1105+
*
1106+
* It must pass the syntactic rules of valid_custom_variable_name(),
1107+
* and it must not be in any namespace already reserved by an extension.
1108+
* (We make this separate from valid_custom_variable_name() because we don't
1109+
* apply the reserved-namespace test when reading configuration files.)
1110+
*
1111+
* If valid, return true. Otherwise, return false if skip_errors is true,
1112+
* else throw a suitable error at the specified elevel (and return false
1113+
* if that's less than ERROR).
1114+
*/
1115+
staticbool
1116+
assignable_custom_variable_name(constchar*name,boolskip_errors,intelevel)
1117+
{
1118+
/* If there's no separator, it can't be a custom variable */
1119+
constchar*sep=strchr(name,GUC_QUALIFIER_SEPARATOR);
1120+
1121+
if (sep!=NULL)
1122+
{
1123+
size_tclassLen=sep-name;
1124+
ListCell*lc;
1125+
1126+
/* The name must be syntactically acceptable ... */
1127+
if (!valid_custom_variable_name(name))
1128+
{
1129+
if (!skip_errors)
1130+
ereport(elevel,
1131+
(errcode(ERRCODE_INVALID_NAME),
1132+
errmsg("invalid configuration parameter name \"%s\"",
1133+
name),
1134+
errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
1135+
return false;
1136+
}
1137+
/* ... and it must not match any previously-reserved prefix */
1138+
foreach(lc,reserved_class_prefix)
1139+
{
1140+
constchar*rcprefix=lfirst(lc);
1141+
1142+
if (strlen(rcprefix)==classLen&&
1143+
strncmp(name,rcprefix,classLen)==0)
1144+
{
1145+
if (!skip_errors)
1146+
ereport(elevel,
1147+
(errcode(ERRCODE_INVALID_NAME),
1148+
errmsg("invalid configuration parameter name \"%s\"",
1149+
name),
1150+
errdetail("\"%s\" is a reserved prefix.",
1151+
rcprefix)));
1152+
return false;
1153+
}
1154+
}
1155+
/* OK to create it */
1156+
return true;
1157+
}
1158+
1159+
/* Unrecognized single-part name */
1160+
if (!skip_errors)
1161+
ereport(elevel,
1162+
(errcode(ERRCODE_UNDEFINED_OBJECT),
1163+
errmsg("unrecognized configuration parameter \"%s\"",
1164+
name)));
1165+
return false;
1166+
}
1167+
11011168
/*
11021169
* Create and add a placeholder variable for a custom variable name.
11031170
*/
@@ -1191,52 +1258,15 @@ find_option(const char *name, bool create_placeholders, bool skip_errors,
11911258
if (create_placeholders)
11921259
{
11931260
/*
1194-
* Check if the name is valid, and if so, add a placeholder. If it
1195-
* doesn't contain a separator, don't assume that it was meant to be a
1196-
* placeholder.
1261+
* Check if the name is valid, and if so, add a placeholder.
11971262
*/
1198-
constchar*sep=strchr(name,GUC_QUALIFIER_SEPARATOR);
1199-
1200-
if (sep!=NULL)
1201-
{
1202-
size_tclassLen=sep-name;
1203-
ListCell*lc;
1204-
1205-
/* The name must be syntactically acceptable ... */
1206-
if (!valid_custom_variable_name(name))
1207-
{
1208-
if (!skip_errors)
1209-
ereport(elevel,
1210-
(errcode(ERRCODE_INVALID_NAME),
1211-
errmsg("invalid configuration parameter name \"%s\"",
1212-
name),
1213-
errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
1214-
returnNULL;
1215-
}
1216-
/* ... and it must not match any previously-reserved prefix */
1217-
foreach(lc,reserved_class_prefix)
1218-
{
1219-
constchar*rcprefix=lfirst(lc);
1220-
1221-
if (strlen(rcprefix)==classLen&&
1222-
strncmp(name,rcprefix,classLen)==0)
1223-
{
1224-
if (!skip_errors)
1225-
ereport(elevel,
1226-
(errcode(ERRCODE_INVALID_NAME),
1227-
errmsg("invalid configuration parameter name \"%s\"",
1228-
name),
1229-
errdetail("\"%s\" is a reserved prefix.",
1230-
rcprefix)));
1231-
returnNULL;
1232-
}
1233-
}
1234-
/* OK, create it */
1263+
if (assignable_custom_variable_name(name,skip_errors,elevel))
12351264
returnadd_placeholder_variable(name,elevel);
1236-
}
1265+
else
1266+
returnNULL;/* error message, if any, already emitted */
12371267
}
12381268

1239-
/* Unknown name */
1269+
/* Unknown nameand we're not supposed to make a placeholder*/
12401270
if (!skip_errors)
12411271
ereport(elevel,
12421272
(errcode(ERRCODE_UNDEFINED_OBJECT),
@@ -1369,18 +1399,16 @@ convert_GUC_name_for_parameter_acl(const char *name)
13691399
/*
13701400
* Check whether we should allow creation of a pg_parameter_acl entry
13711401
* for the given name. (This can be applied either before or after
1372-
* canonicalizing it.)
1402+
* canonicalizing it.) Throws error if not.
13731403
*/
1374-
bool
1404+
void
13751405
check_GUC_name_for_parameter_acl(constchar*name)
13761406
{
13771407
/* OK if the GUC exists. */
1378-
if (find_option(name, false, true,DEBUG1)!=NULL)
1379-
return true;
1408+
if (find_option(name, false, true,DEBUG5)!=NULL)
1409+
return;
13801410
/* Otherwise, it'd better be a valid custom GUC name. */
1381-
if (valid_custom_variable_name(name))
1382-
return true;
1383-
return false;
1411+
(void)assignable_custom_variable_name(name, false,ERROR);
13841412
}
13851413

13861414
/*
@@ -4515,52 +4543,64 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
45154543
{
45164544
structconfig_generic*record;
45174545

4518-
record=find_option(name, false, false,ERROR);
4519-
Assert(record!=NULL);
4520-
4521-
/*
4522-
* Don't allow parameters that can't be set in configuration files to
4523-
* be set in PG_AUTOCONF_FILENAME file.
4524-
*/
4525-
if ((record->context==PGC_INTERNAL)||
4526-
(record->flags&GUC_DISALLOW_IN_FILE)||
4527-
(record->flags&GUC_DISALLOW_IN_AUTO_FILE))
4528-
ereport(ERROR,
4529-
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
4530-
errmsg("parameter \"%s\" cannot be changed",
4531-
name)));
4532-
4533-
/*
4534-
* If a value is specified, verify that it's sane.
4535-
*/
4536-
if (value)
4546+
/* We don't want to create a placeholder if there's not one already */
4547+
record=find_option(name, false, true,DEBUG5);
4548+
if (record!=NULL)
45374549
{
4538-
unionconfig_var_valnewval;
4539-
void*newextra=NULL;
4540-
4541-
/* Check that it's acceptable for the indicated parameter */
4542-
if (!parse_and_validate_value(record,name,value,
4543-
PGC_S_FILE,ERROR,
4544-
&newval,&newextra))
4550+
/*
4551+
* Don't allow parameters that can't be set in configuration files
4552+
* to be set in PG_AUTOCONF_FILENAME file.
4553+
*/
4554+
if ((record->context==PGC_INTERNAL)||
4555+
(record->flags&GUC_DISALLOW_IN_FILE)||
4556+
(record->flags&GUC_DISALLOW_IN_AUTO_FILE))
45454557
ereport(ERROR,
4546-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
4547-
errmsg("invalid value for parameter \"%s\": \"%s\"",
4548-
name,value)));
4558+
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
4559+
errmsg("parameter \"%s\" cannot be changed",
4560+
name)));
4561+
4562+
/*
4563+
* If a value is specified, verify that it's sane.
4564+
*/
4565+
if (value)
4566+
{
4567+
unionconfig_var_valnewval;
4568+
void*newextra=NULL;
45494569

4550-
if (record->vartype==PGC_STRING&&newval.stringval!=NULL)
4551-
guc_free(newval.stringval);
4552-
guc_free(newextra);
4570+
if (!parse_and_validate_value(record,name,value,
4571+
PGC_S_FILE,ERROR,
4572+
&newval,&newextra))
4573+
ereport(ERROR,
4574+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
4575+
errmsg("invalid value for parameter \"%s\": \"%s\"",
4576+
name,value)));
45534577

4578+
if (record->vartype==PGC_STRING&&newval.stringval!=NULL)
4579+
guc_free(newval.stringval);
4580+
guc_free(newextra);
4581+
}
4582+
}
4583+
else
4584+
{
45544585
/*
4555-
* We must also reject values containing newlines, because the
4556-
* grammar for config files doesn't support embedded newlines in
4557-
* string literals.
4586+
* Variable not known; check we'd be allowed to create it. (We
4587+
* cannot validate the value, but that's fine. A non-core GUC in
4588+
* the config file cannot cause postmaster start to fail, so we
4589+
* don't have to be too tense about possibly installing a bad
4590+
* value.)
45584591
*/
4559-
if (strchr(value,'\n'))
4560-
ereport(ERROR,
4561-
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
4562-
errmsg("parameter value for ALTER SYSTEM must not contain a newline")));
4592+
(void)assignable_custom_variable_name(name, false,ERROR);
45634593
}
4594+
4595+
/*
4596+
* We must also reject values containing newlines, because the grammar
4597+
* for config files doesn't support embedded newlines in string
4598+
* literals.
4599+
*/
4600+
if (value&&strchr(value,'\n'))
4601+
ereport(ERROR,
4602+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
4603+
errmsg("parameter value for ALTER SYSTEM must not contain a newline")));
45644604
}
45654605

45664606
/*

‎src/include/utils/guc.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ extern const char *GetConfigOptionResetString(const char *name);
363363
externintGetConfigOptionFlags(constchar*name,boolmissing_ok);
364364
externvoidProcessConfigFile(GucContextcontext);
365365
externchar*convert_GUC_name_for_parameter_acl(constchar*name);
366-
externboolcheck_GUC_name_for_parameter_acl(constchar*name);
366+
externvoidcheck_GUC_name_for_parameter_acl(constchar*name);
367367
externvoidInitializeGUCOptions(void);
368368
externboolSelectConfigFiles(constchar*userDoption,constchar*progname);
369369
externvoidResetAllOptions(void);

‎src/test/modules/unsafe_tests/expected/guc_privs.out

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,31 @@ SELECT 1 FROM pg_parameter_acl WHERE parname = 'none.such';
220220
----------
221221
(0 rows)
222222

223+
-- Superuser should be able to ALTER SYSTEM SET a non-existent custom GUC.
224+
ALTER SYSTEM SET none.such = 'whiz bang';
225+
-- None of the above should have created a placeholder GUC for none.such.
226+
SHOW none.such; -- error
227+
ERROR: unrecognized configuration parameter "none.such"
228+
-- However, if we reload ...
229+
SELECT pg_reload_conf();
230+
pg_reload_conf
231+
----------------
232+
t
233+
(1 row)
234+
235+
-- and start a new session to avoid race condition ...
236+
\c -
237+
SET SESSION AUTHORIZATION regress_admin;
238+
-- then it should be there.
239+
SHOW none.such;
240+
none.such
241+
-----------
242+
whiz bang
243+
(1 row)
244+
223245
-- Can't grant on a non-existent core GUC.
224246
GRANT ALL ON PARAMETER no_such_guc TO regress_host_resource_admin; -- fail
225-
ERROR:invalid parameter name "no_such_guc"
247+
ERROR:unrecognized configuration parameter "no_such_guc"
226248
-- Initially there are no privileges and no catalog entry for this GUC.
227249
SELECT has_parameter_privilege('regress_host_resource_admin', 'enable_material', 'SET');
228250
has_parameter_privilege
@@ -459,6 +481,8 @@ SELECT set_config ('temp_buffers', '8192', false); -- ok
459481
ALTER SYSTEM RESET autovacuum_work_mem; -- ok, privileges have been granted
460482
ALTER SYSTEM RESET ALL; -- fail, insufficient privileges
461483
ERROR: permission denied to perform ALTER SYSTEM RESET ALL
484+
ALTER SYSTEM SET none.such2 = 'whiz bang'; -- fail, not superuser
485+
ERROR: permission denied to set parameter "none.such2"
462486
ALTER ROLE regress_host_resource_admin SET lc_messages = 'POSIX'; -- fail
463487
ERROR: permission denied to set parameter "lc_messages"
464488
ALTER ROLE regress_host_resource_admin SET max_stack_depth = '1MB'; -- ok

‎src/test/modules/unsafe_tests/sql/guc_privs.sql

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,19 @@ GRANT ALL ON PARAMETER none.such TO regress_host_resource_admin;
9898
SELECT1FROM pg_parameter_aclWHERE parname='none.such';
9999
REVOKE ALLON PARAMETER"None.Such"FROM regress_host_resource_admin;
100100
SELECT1FROM pg_parameter_aclWHERE parname='none.such';
101+
102+
-- Superuser should be able to ALTER SYSTEM SET a non-existent custom GUC.
103+
ALTER SYSTEMSETnone.such='whiz bang';
104+
-- None of the above should have created a placeholder GUC for none.such.
105+
SHOWnone.such;-- error
106+
-- However, if we reload ...
107+
SELECT pg_reload_conf();
108+
-- and start a new session to avoid race condition ...
109+
\c-
110+
SET SESSION AUTHORIZATION regress_admin;
111+
-- then it should be there.
112+
SHOWnone.such;
113+
101114
-- Can't grant on a non-existent core GUC.
102115
GRANT ALLON PARAMETER no_such_guc TO regress_host_resource_admin;-- fail
103116

@@ -190,6 +203,7 @@ ALTER SYSTEM RESET lc_messages; -- fail, insufficient privileges
190203
SELECT set_config ('temp_buffers','8192', false);-- ok
191204
ALTER SYSTEM RESET autovacuum_work_mem;-- ok, privileges have been granted
192205
ALTER SYSTEM RESET ALL;-- fail, insufficient privileges
206+
ALTER SYSTEMSETnone.such2='whiz bang';-- fail, not superuser
193207
ALTER ROLE regress_host_resource_adminSET lc_messages='POSIX';-- fail
194208
ALTER ROLE regress_host_resource_adminSET max_stack_depth='1MB';-- ok
195209
SELECT setconfigFROM pg_db_role_setting

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp