diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 7885a169bb..43cf0c4434 100644 --- a/src/backend/utils/misc/guc-file.l +++ b/src/backend/utils/misc/guc-file.l @@ -282,7 +282,7 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel) * Try to find the variable; but do not create a custom placeholder if * it's not there already. */ - record = find_option(item->name, false, elevel); + record = find_option(item->name, false, true, elevel); if (record) { @@ -306,7 +306,8 @@ ProcessConfigFileInternal(GucContext context, bool applySettings, int elevel) /* Now mark it as present in file */ record->status |= GUC_IS_IN_FILE; } - else if (strchr(item->name, GUC_QUALIFIER_SEPARATOR) == NULL) + else if (strchr(item->name, GUC_QUALIFIER_SEPARATOR) == NULL || + strcspn(item->name, GUC_DISALLOWED_NAME_CHARS) != strlen(item->name)) { /* Invalid non-custom variable, so complain */ ereport(elevel, diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index eafdb1118e..959099122b 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -5289,12 +5289,23 @@ add_placeholder_variable(const char *name, int elevel) } /* - * Look up option NAME. If it exists, return a pointer to its record, - * else return NULL. If create_placeholders is true, we'll create a - * placeholder record for a valid-looking custom variable name. + * Look up option "name". If it exists, return a pointer to its record. + * Otherwise, if create_placeholders is true and name is a valid-looking + * custom variable name, we'll create and return a placeholder record. + * Otherwise, if skip_errors is true, then we silently return NULL for + * an unrecognized or invalid name. Otherwise, the error is reported at + * error level elevel (and we return NULL if that's less than ERROR). + * + * Note: internal errors, primarily out-of-memory, draw an elevel-level + * report and NULL return regardless of skip_errors. Hence, callers must + * handle a NULL return whenever elevel < ERROR, but they should not need + * to emit any additional error message. (In practice, internal errors + * can only happen when create_placeholders is true, so callers passing + * false need not think terribly hard about this.) */ static struct config_generic * -find_option(const char *name, bool create_placeholders, int elevel) +find_option(const char *name, bool create_placeholders, bool skip_errors, + int elevel) { const char **key = &name; struct config_generic **res; @@ -5322,19 +5333,38 @@ find_option(const char *name, bool create_placeholders, int elevel) for (i = 0; map_old_guc_names[i] != NULL; i += 2) { if (guc_name_compare(name, map_old_guc_names[i]) == 0) - return find_option(map_old_guc_names[i + 1], false, elevel); + return find_option(map_old_guc_names[i + 1], false, + skip_errors, elevel); } if (create_placeholders) { /* - * Check if the name is qualified, and if so, add a placeholder. + * Check if the name is valid, and if so, add a placeholder. To be + * allowed, it must contain a separator character, but not any of the + * disallowed characters. */ if (strchr(name, GUC_QUALIFIER_SEPARATOR) != NULL) - return add_placeholder_variable(name, elevel); + { + if (strcspn(name, GUC_DISALLOWED_NAME_CHARS) == strlen(name)) + return add_placeholder_variable(name, elevel); + /* A special error message seems desirable here */ + if (!skip_errors) + ereport(elevel, + (errcode(ERRCODE_INVALID_NAME), + errmsg("invalid configuration parameter name \"%s\"", + name), + errdetail("Configuration parameter names cannot contain \"=\" or \"-\"."))); + return NULL; + } } /* Unknown name */ + if (!skip_errors) + ereport(elevel, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("unrecognized configuration parameter \"%s\"", + name))); return NULL; } @@ -6354,7 +6384,7 @@ ReportChangedGUCOptions(void) { struct config_generic *record; - record = find_option("in_hot_standby", false, ERROR); + record = find_option("in_hot_standby", false, false, ERROR); Assert(record != NULL); record->status |= GUC_NEEDS_REPORT; report_needed = true; @@ -7124,14 +7154,9 @@ set_config_option(const char *name, const char *value, (errcode(ERRCODE_INVALID_TRANSACTION_STATE), errmsg("cannot set parameters during a parallel operation"))); - record = find_option(name, true, elevel); + record = find_option(name, true, false, elevel); if (record == NULL) - { - ereport(elevel, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", name))); return 0; - } /* * Check if the option can be set at this time. See guc.h for the precise @@ -7853,10 +7878,10 @@ set_config_sourcefile(const char *name, char *sourcefile, int sourceline) */ elevel = IsUnderPostmaster ? DEBUG3 : LOG; - record = find_option(name, true, elevel); + record = find_option(name, true, false, elevel); /* should not happen */ if (record == NULL) - elog(ERROR, "unrecognized configuration parameter \"%s\"", name); + return; sourcefile = guc_strdup(elevel, sourcefile); if (record->sourcefile) @@ -7905,16 +7930,9 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_privileged) struct config_generic *record; static char buffer[256]; - record = find_option(name, false, ERROR); + record = find_option(name, false, missing_ok, ERROR); if (record == NULL) - { - if (missing_ok) - return NULL; - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", - name))); - } + return NULL; if (restrict_privileged && (record->flags & GUC_SUPERUSER_ONLY) && !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)) @@ -7961,11 +7979,8 @@ GetConfigOptionResetString(const char *name) struct config_generic *record; static char buffer[256]; - record = find_option(name, false, ERROR); - if (record == NULL) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", name))); + record = find_option(name, false, false, ERROR); + Assert(record != NULL); if ((record->flags & GUC_SUPERUSER_ONLY) && !is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS)) ereport(ERROR, @@ -8009,16 +8024,9 @@ GetConfigOptionFlags(const char *name, bool missing_ok) { struct config_generic *record; - record = find_option(name, false, WARNING); + record = find_option(name, false, missing_ok, ERROR); if (record == NULL) - { - if (missing_ok) - return 0; - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", - name))); - } + return 0; return record->flags; } @@ -8050,7 +8058,7 @@ flatten_set_variable_args(const char *name, List *args) * Get flags for the variable; if it's not known, use default flags. * (Caller might throw error later, but not our business to do so here.) */ - record = find_option(name, false, WARNING); + record = find_option(name, false, true, WARNING); if (record) flags = record->flags; else @@ -8345,12 +8353,8 @@ AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt) { struct config_generic *record; - record = find_option(name, false, ERROR); - if (record == NULL) - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", - name))); + record = find_option(name, false, false, ERROR); + Assert(record != NULL); /* * Don't allow parameters that can't be set in configuration files to @@ -9366,19 +9370,12 @@ GetConfigOptionByName(const char *name, const char **varname, bool missing_ok) { struct config_generic *record; - record = find_option(name, false, ERROR); + record = find_option(name, false, missing_ok, ERROR); if (record == NULL) { - if (missing_ok) - { - if (varname) - *varname = NULL; - return NULL; - } - - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", name))); + if (varname) + *varname = NULL; + return NULL; } if ((record->flags & GUC_SUPERUSER_ONLY) && @@ -10230,7 +10227,7 @@ read_nondefault_variables(void) if ((varname = read_string_with_null(fp)) == NULL) break; - if ((record = find_option(varname, true, FATAL)) == NULL) + if ((record = find_option(varname, true, false, FATAL)) == NULL) elog(FATAL, "failed to locate variable \"%s\" in exec config params file", varname); if ((varvalue = read_string_with_null(fp)) == NULL) @@ -10801,7 +10798,7 @@ GUCArrayAdd(ArrayType *array, const char *name, const char *value) (void) validate_option_array_item(name, value, false); /* normalize name (converts obsolete GUC names to modern spellings) */ - record = find_option(name, false, WARNING); + record = find_option(name, false, true, WARNING); if (record) name = record->name; @@ -10880,7 +10877,7 @@ GUCArrayDelete(ArrayType *array, const char *name) (void) validate_option_array_item(name, NULL, false); /* normalize name (converts obsolete GUC names to modern spellings) */ - record = find_option(name, false, WARNING); + record = find_option(name, false, true, WARNING); if (record) name = record->name; @@ -11027,7 +11024,7 @@ validate_option_array_item(const char *name, const char *value, * SUSET and user is superuser). * * name is not known, but exists or can be created as a placeholder (i.e., - * it has a prefixed name). We allow this case if you're a superuser, + * it has a valid custom name). We allow this case if you're a superuser, * otherwise not. Superusers are assumed to know what they're doing. We * can't allow it for other users, because when the placeholder is * resolved it might turn out to be a SUSET variable; @@ -11036,16 +11033,11 @@ validate_option_array_item(const char *name, const char *value, * name is not known and can't be created as a placeholder. Throw error, * unless skipIfNoPermissions is true, in which case return false. */ - gconf = find_option(name, true, WARNING); + gconf = find_option(name, true, skipIfNoPermissions, ERROR); if (!gconf) { /* not known, failed to make a placeholder */ - if (skipIfNoPermissions) - return false; - ereport(ERROR, - (errcode(ERRCODE_UNDEFINED_OBJECT), - errmsg("unrecognized configuration parameter \"%s\"", - name))); + return false; } if (gconf->flags & GUC_CUSTOM_PLACEHOLDER) diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h index 5004ee4177..e4bc5052ef 100644 --- a/src/include/utils/guc.h +++ b/src/include/utils/guc.h @@ -197,8 +197,16 @@ typedef enum GUC_ACTION_SAVE /* function SET option, or temp assignment */ } GucAction; +/* Custom GUC names must contain this: */ #define GUC_QUALIFIER_SEPARATOR '.' +/* + * We disallow these characters in GUC names, as they would cause + * ParseLongOption() to do the wrong thing. If you change this list, + * adjust the error text in find_option(). + */ +#define GUC_DISALLOWED_NAME_CHARS "=-" + /* * bit values in "flags" of a GUC variable */ diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 811f80a097..63e2d31454 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -511,6 +511,29 @@ SET seq_page_cost TO 'NaN'; ERROR: invalid value for parameter "seq_page_cost": "NaN" SET vacuum_cost_delay TO '10s'; ERROR: 10000 ms is outside the valid range for parameter "vacuum_cost_delay" (0 .. 100) +SET no_such_variable TO 42; +ERROR: unrecognized configuration parameter "no_such_variable" +-- Test "custom" GUCs created on the fly (which aren't really an +-- intended feature, but many people use them). +SET custom.my_guc = 42; +SHOW custom.my_guc; + custom.my_guc +--------------- + 42 +(1 row) + +SET special."weird name" = 'foo'; +SHOW special."weird name"; + special.weird name +-------------------- + foo +(1 row) + +SET custom."bad-guc" = 42; -- disallowed because -c cannot set it +ERROR: invalid configuration parameter name "custom.bad-guc" +DETAIL: Configuration parameter names cannot contain "=" or "-". +SHOW custom."bad-guc"; +ERROR: unrecognized configuration parameter "custom.bad-guc" -- -- Test DISCARD TEMP -- diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index 43dbba3775..c8f55a43be 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -147,6 +147,16 @@ SELECT '2006-08-13 12:34:56'::timestamptz; -- Test some simple error cases SET seq_page_cost TO 'NaN'; SET vacuum_cost_delay TO '10s'; +SET no_such_variable TO 42; + +-- Test "custom" GUCs created on the fly (which aren't really an +-- intended feature, but many people use them). +SET custom.my_guc = 42; +SHOW custom.my_guc; +SET special."weird name" = 'foo'; +SHOW special."weird name"; +SET custom."bad-guc" = 42; -- disallowed because -c cannot set it +SHOW custom."bad-guc"; -- -- Test DISCARD TEMP