rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Rethink handling of settings with a prefix reserved by an extension. 

Commit 75d22069e made SET print a warning if you tried to set an
unrecognized parameter within namespace previously reserved by an
extension.  It seems better for that to be an outright error though,
for the same reason that we don't let you set unrecognized unqualified
parameter names.  In any case, the preceding implementation was
inefficient and erroneous.  Perform the check in a more appropriate
spot, and be more careful about prefix-match cases.

Discussion: https://postgr.es/m/116024.1640111629@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2021-12-27 14:35:50 -05:00
parent 86d9888d2e
commit 2ed8a8cc5b
3 changed files with 68 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

96
src/backend/utils/misc/guc.c
View File

@ -148,6 +148,8 @@ extern bool optimize_bounded_sort;
static int GUC_check_errcode_value;
static List *reserved_class_prefix = NIL;
/* global variables for check hook support */
char *GUC_check_errmsg_string;
char *GUC_check_errdetail_string;
@ -235,8 +237,6 @@ static bool check_recovery_target_lsn(char **newval, void **extra, GucSource sou
static void assign_recovery_target_lsn(const char *newval, void *extra);
static bool check_primary_slot_name(char **newval, void **extra, GucSource source);
static bool check_default_with_oids(bool *newval, void **extra, GucSource source);
static void check_reserved_prefixes(const char *varName);
static List *reserved_class_prefix = NIL;
/* Private functions in guc-file.l that need to be called from guc.c */
static ConfigVariable *ProcessConfigFileInternal(GucContext context,
@ -5569,18 +5569,44 @@ find_option(const char *name, bool create_placeholders, bool skip_errors,
* doesn't contain a separator, don't assume that it was meant to be a
* placeholder.
*/
if (strchr(name, GUC_QUALIFIER_SEPARATOR) != NULL)
const char *sep = strchr(name, GUC_QUALIFIER_SEPARATOR);
if (sep != NULL)
{
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 two or more simple identifiers separated by dots.")));
return NULL;
size_t classLen = sep - name;
ListCell *lc;
/* The name must be syntactically acceptable ... */
if (!valid_custom_variable_name(name))
{
if (!skip_errors)
ereport(elevel,
(errcode(ERRCODE_INVALID_NAME),
errmsg("invalid configuration parameter name \"%s\"",
name),
errdetail("Custom parameter names must be two or more simple identifiers separated by dots.")));
return NULL;
}
/* ... and it must not match any previously-reserved prefix */
foreach(lc, reserved_class_prefix)
{
const char *rcprefix = lfirst(lc);
if (strlen(rcprefix) == classLen &&
strncmp(name, rcprefix, classLen) == 0)
{
if (!skip_errors)
ereport(elevel,
(errcode(ERRCODE_INVALID_NAME),
errmsg("invalid configuration parameter name \"%s\"",
name),
errdetail("\"%s\" is a reserved prefix.",
rcprefix)));
return NULL;
}
}
/* OK, create it */
return add_placeholder_variable(name, elevel);
}
}
@ -8764,7 +8790,6 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
(superuser() ? PGC_SUSET : PGC_USERSET),
PGC_S_SESSION,
action, true, 0, false);
check_reserved_prefixes(stmt->name);
break;
case VAR_SET_MULTI:
@ -8850,8 +8875,6 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel)
(superuser() ? PGC_SUSET : PGC_USERSET),
PGC_S_SESSION,
action, true, 0, false);
check_reserved_prefixes(stmt->name);
break;
case VAR_RESET_ALL:
ResetAllOptions();
@ -9345,8 +9368,9 @@ EmitWarningsOnPlaceholders(const char *className)
{
int classLen = strlen(className);
int i;
MemoryContext oldcontext;
MemoryContext oldcontext;
/* Check for existing placeholders. */
for (i = 0; i < num_guc_variables; i++)
{
struct config_generic *var = guc_variables[i];
@ -9362,48 +9386,12 @@ EmitWarningsOnPlaceholders(const char *className)
}
}
/* And remember the name so we can prevent future mistakes. */
oldcontext = MemoryContextSwitchTo(TopMemoryContext);
reserved_class_prefix = lappend(reserved_class_prefix, pstrdup(className));
MemoryContextSwitchTo(oldcontext);
}
/*
* Check a setting name against prefixes previously reserved by
* EmitWarningsOnPlaceholders() and throw a warning if matching.
*/
static void
check_reserved_prefixes(const char *varName)
{
char *sep = strchr(varName, GUC_QUALIFIER_SEPARATOR);
if (sep)
{
size_t classLen = sep - varName;
ListCell *lc;
foreach(lc, reserved_class_prefix)
{
char *rcprefix = lfirst(lc);
if (strncmp(varName, rcprefix, classLen) == 0)
{
for (int i = 0; i < num_guc_variables; i++)
{
struct config_generic *var = guc_variables[i];
if ((var->flags & GUC_CUSTOM_PLACEHOLDER) != 0 &&
strcmp(varName, var->name) == 0)
{
ereport(WARNING,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("unrecognized configuration parameter \"%s\"", var->name),
errdetail("\"%.*s\" is a reserved prefix.", (int) classLen, var->name)));
}
}
}
}
}
}
/*
* SHOW command

36
src/test/regress/expected/guc.out
View File

@ -548,6 +548,23 @@ ERROR: invalid configuration parameter name "special.weird name"
DETAIL: Custom parameter names must be two or more simple identifiers separated by dots.
SHOW special."weird name";
ERROR: unrecognized configuration parameter "special.weird name"
-- Check what happens when you try to set a "custom" GUC within the
-- namespace of an extension.
SET plpgsql.bogus_setting = 42; -- allowed if plpgsql is not loaded yet
LOAD 'plpgsql'; -- this will now warn about it
WARNING: unrecognized configuration parameter "plpgsql.bogus_setting"
SET plpgsql.extra_foo_warnings = false; -- but now, it's an error
ERROR: invalid configuration parameter name "plpgsql.extra_foo_warnings"
DETAIL: "plpgsql" is a reserved prefix.
SHOW plpgsql.extra_foo_warnings;
ERROR: unrecognized configuration parameter "plpgsql.extra_foo_warnings"
SET plpgsql.bogus_setting = 43; -- you can still use the pre-existing variable
SHOW plpgsql.bogus_setting;
plpgsql.bogus_setting
-----------------------
43
(1 row)
--
-- Test DISCARD TEMP
--
@ -813,22 +830,3 @@ set default_with_oids to f;
-- Should not allow to set it to true.
set default_with_oids to t;
ERROR: tables declared WITH OIDS are not supported
-- test SET unrecognized parameter
SET foo = false; -- no such setting
ERROR: unrecognized configuration parameter "foo"
-- test setting a parameter with a registered prefix (plpgsql)
SET plpgsql.extra_foo_warnings = false; -- no such setting
WARNING: unrecognized configuration parameter "plpgsql.extra_foo_warnings"
DETAIL: "plpgsql" is a reserved prefix.
SHOW plpgsql.extra_foo_warnings; -- but the parameter is set
plpgsql.extra_foo_warnings
----------------------------
false
(1 row)
-- cleanup
RESET foo;
ERROR: unrecognized configuration parameter "foo"
RESET plpgsql.extra_foo_warnings;
WARNING: unrecognized configuration parameter "plpgsql.extra_foo_warnings"
DETAIL: "plpgsql" is a reserved prefix.

20
src/test/regress/sql/guc.sql
View File

@ -163,6 +163,15 @@ SHOW custom."bad-guc";
SET special."weird name" = 'foo'; -- could be allowed, but we choose not to
SHOW special."weird name";
-- Check what happens when you try to set a "custom" GUC within the
-- namespace of an extension.
SET plpgsql.bogus_setting = 42; -- allowed if plpgsql is not loaded yet
LOAD 'plpgsql'; -- this will now warn about it
SET plpgsql.extra_foo_warnings = false; -- but now, it's an error
SHOW plpgsql.extra_foo_warnings;
SET plpgsql.bogus_setting = 43; -- you can still use the pre-existing variable
SHOW plpgsql.bogus_setting;
--
-- Test DISCARD TEMP
--
@ -311,14 +320,3 @@ reset check_function_bodies;
set default_with_oids to f;
-- Should not allow to set it to true.
set default_with_oids to t;
-- test SET unrecognized parameter
SET foo = false; -- no such setting
-- test setting a parameter with a registered prefix (plpgsql)
SET plpgsql.extra_foo_warnings = false; -- no such setting
SHOW plpgsql.extra_foo_warnings; -- but the parameter is set
-- cleanup
RESET foo;
RESET plpgsql.extra_foo_warnings;