diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l index 7885a169bb..9498bbea2f 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,7 @@ 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 (!valid_custom_variable_name(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 855076b1fd..46434950a7 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -5252,6 +5252,45 @@ add_guc_variable(struct config_generic *var, int elevel) return true; } +/* + * Decide whether a proposed custom variable name is allowed. + * + * It must be "identifier.identifier", where the rules for what is an + * identifier agree with scan.l. + */ +static bool +valid_custom_variable_name(const char *name) +{ + int num_sep = 0; + bool name_start = true; + + for (const char *p = name; *p; p++) + { + if (*p == GUC_QUALIFIER_SEPARATOR) + { + if (name_start) + return false; /* empty name component */ + num_sep++; + name_start = true; + } + else if (strchr("ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz", *p) != NULL || + IS_HIGHBIT_SET(*p)) + { + /* okay as first or non-first character */ + name_start = false; + } + else if (!name_start && strchr("0123456789_$", *p) != NULL) + /* okay as non-first character */ ; + else + return false; + } + if (name_start) + return false; /* empty name component */ + /* OK if we had exactly one separator */ + return (num_sep == 1); +} + /* * Create and add a placeholder variable for a custom variable name. */ @@ -5299,12 +5338,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; @@ -5332,19 +5382,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. If it + * doesn't contain a separator, don't assume that it was meant to be a + * placeholder. */ if (strchr(name, GUC_QUALIFIER_SEPARATOR) != NULL) - return add_placeholder_variable(name, elevel); + { + if (valid_custom_variable_name(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("Custom parameter names must be of the form \"identifier.identifier\"."))); + return NULL; + } } /* Unknown name */ + if (!skip_errors) + ereport(elevel, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("unrecognized configuration parameter \"%s\"", + name))); return NULL; } @@ -6362,7 +6431,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; @@ -7132,14 +7201,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 @@ -7861,10 +7925,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) @@ -7913,16 +7977,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)) @@ -7969,11 +8026,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, @@ -8017,16 +8071,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; } @@ -8058,7 +8105,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 @@ -8353,12 +8400,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 @@ -9374,19 +9417,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) && @@ -10238,7 +10274,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) @@ -10809,7 +10845,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; @@ -10888,7 +10924,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; @@ -11035,7 +11071,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; @@ -11044,16 +11080,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/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 811f80a097..c55871a972 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -511,6 +511,27 @@ 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 custom."bad-guc" = 42; -- disallowed because -c cannot set this name +ERROR: invalid configuration parameter name "custom.bad-guc" +DETAIL: Custom parameter names must be of the form "identifier.identifier". +SHOW custom."bad-guc"; +ERROR: unrecognized configuration parameter "custom.bad-guc" +SET special."weird name" = 'foo'; -- could be allowed, but we choose not to +ERROR: invalid configuration parameter name "special.weird name" +DETAIL: Custom parameter names must be of the form "identifier.identifier". +SHOW special."weird name"; +ERROR: unrecognized configuration parameter "special.weird name" -- -- Test DISCARD TEMP -- diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index 43dbba3775..3650188d9d 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 custom."bad-guc" = 42; -- disallowed because -c cannot set this name +SHOW custom."bad-guc"; +SET special."weird name" = 'foo'; -- could be allowed, but we choose not to +SHOW special."weird name"; -- -- Test DISCARD TEMP