From 9a50261faad3a7578dab150f1c05b510285e283c Mon Sep 17 00:00:00 2001 From: Florin Irion Date: Thu, 7 Oct 2021 09:51:29 +0200 Subject: [PATCH] Reserve the prefix for each extension Throw a warning when we `SET` a variable that doesn't exist with a reserved prefix. This will protect against setting incorrect configurations for any extension, like we do with the `GUC`s. Add a regression test that checks the patch using the "plpgsql" registered prefix. --- src/backend/utils/misc/guc.c | 49 +++++++++++++++++++++++++++++++ src/test/regress/expected/guc.out | 21 +++++++++++++ src/test/regress/sql/guc.sql | 10 +++++++ 3 files changed, 80 insertions(+) diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 6652a60ec3..f19310df44 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -234,6 +234,8 @@ 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 EmitWarningsOnSetPlaceholders(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, @@ -8703,6 +8705,8 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) (superuser() ? PGC_SUSET : PGC_USERSET), PGC_S_SESSION, action, true, 0, false); + + EmitWarningsOnSetPlaceholders(stmt->name); break; case VAR_SET_MULTI: @@ -8789,6 +8793,8 @@ ExecSetVariableStmt(VariableSetStmt *stmt, bool isTopLevel) (superuser() ? PGC_SUSET : PGC_USERSET), PGC_S_SESSION, action, true, 0, false); + + EmitWarningsOnSetPlaceholders(stmt->name); break; case VAR_RESET_ALL: ResetAllOptions(); @@ -9275,6 +9281,7 @@ EmitWarningsOnPlaceholders(const char *className) { int classLen = strlen(className); int i; + MemoryContext oldcontext; for (i = 0; i < num_guc_variables; i++) { @@ -9290,8 +9297,50 @@ EmitWarningsOnPlaceholders(const char *className) var->name))); } } + oldcontext = MemoryContextSwitchTo(TopMemoryContext); + reserved_class_prefix = lappend(reserved_class_prefix, pstrdup(className)); + MemoryContextSwitchTo(oldcontext); } +/* + * Throw a warning if a user creates a placeholder with + * the "SET" command and we don't recognize it. + */ +static void +EmitWarningsOnSetPlaceholders(const char *varName) +{ + char *gucQualifierSeparator = strchr(varName, GUC_QUALIFIER_SEPARATOR); + + if (gucQualifierSeparator) + { + unsigned int classLen = gucQualifierSeparator - varName; + unsigned int varLen = strlen(varName); + ListCell *lc; + + foreach(lc, reserved_class_prefix) + { + char *rcprefix = (char *) 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 && + strncmp(varName, var->name, varLen)==0) + { + ereport(WARNING, + (errcode(ERRCODE_UNDEFINED_OBJECT), + errmsg("unrecognized configuration parameter \"%s\"", var->name), + errdetail("\"%.*s\" is a reserved prefix.", classLen, var->name), + errhint("If you need to create a custom placeholder use a different prefix."))); + } + } + } + } + } +} /* * SHOW command diff --git a/src/test/regress/expected/guc.out b/src/test/regress/expected/guc.out index 59da91ff04..e5049bed19 100644 --- a/src/test/regress/expected/guc.out +++ b/src/test/regress/expected/guc.out @@ -813,3 +813,24 @@ 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 unrecongnized 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. +HINT: If you need to create a custom placeholder use a different prefix. +SHOW plpgsql.extra_foo_warnings; -- the parameter is however 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. +HINT: If you need to create a custom placeholder use a different prefix. diff --git a/src/test/regress/sql/guc.sql b/src/test/regress/sql/guc.sql index c39c11388d..d9669d6e5a 100644 --- a/src/test/regress/sql/guc.sql +++ b/src/test/regress/sql/guc.sql @@ -311,3 +311,13 @@ 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 unrecongnized 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; -- the parameter is however set +-- cleanup +RESET foo; +RESET plpgsql.extra_foo_warnings; \ No newline at end of file -- 2.31.1