From f99313b7d0a8e3084b6709aaf69e4a7e803a8502 Mon Sep 17 00:00:00 2001 From: "okbob@github.com" Date: Fri, 13 Dec 2024 06:28:45 +0100 Subject: [PATCH 09/22] dynamic check of usage of session variable fences This patch allows to specify dynamic context where fencing of session variables is required. Possible contexts are none, nospi or all. With this check is possible to force variable fencing outside stored procedures. --- doc/src/sgml/config.sgml | 56 ++++++++++++ doc/src/sgml/ddl.sgml | 3 +- src/backend/commands/session_variable.c | 33 +++++++ src/backend/executor/execExpr.c | 4 + src/backend/executor/spi.c | 8 ++ src/backend/utils/misc/guc_tables.c | 19 ++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/commands/session_variable.h | 16 ++++ src/include/executor/spi.h | 1 + .../regress/expected/session_variables.out | 90 +++++++++++++++++++ src/test/regress/sql/session_variables.sql | 73 +++++++++++++++ 11 files changed, 303 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 65f19598c2f..b1dd7e50d0c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -10840,6 +10840,62 @@ DETAIL: Session variables can be shadowed by columns, routine's variables and r + + session_variables_use_fence_context_guard (boolean) + + session_variables_use_fence_context_guard configuration parameter + + + + + When on, an error is raised when a session variable identifier is used + inside a query without variable fence in choosed context. The default + is none. The error is raised only when variable is + used in places, where an collisions with column names is possible (and + when variable fences are possible). The allowed values of + session_variables_use_fence_context_guard are + none (allow using session variables without + variable's fences always), all (requires variable + fencing everywhere), and nospi (requires variable + fencing outside SPI contexts (outside stored procedures). + +SET session_variables_use_fence_context_guard TO 'nospi'; +CREATE VARIABLE a int; + +LET a = 10; +DO $$ +BEGIN + RAISE NOTICE 'a = %', a; +END; +$$; + +SELECT a; +SELECT VARIABLE(s); + + + +NOTICE: a = 10 + +ERROR: session variable "public.a" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". + +SELECT variable(a); + a +---- + 10 +(1 row) + + + + + When you afraid about unwanted usage of session variables, set + this feature to all or minimaly to + nospi. + + + + session_variables_use_fence_warning_guard (boolean) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index fe3f1cca006..122545511e4 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -5424,7 +5424,8 @@ SELECT VARIABLE(current_user_id); When there is a risk of possible collisions between variable identifiers and column names, then using variable fence syntax can be recommended. Usage session variable without variable fence can be detected by - warning . + warning + or by check . diff --git a/src/backend/commands/session_variable.c b/src/backend/commands/session_variable.c index 4f5fee4814d..2e36f9da800 100644 --- a/src/backend/commands/session_variable.c +++ b/src/backend/commands/session_variable.c @@ -17,6 +17,7 @@ #include "access/xact.h" #include "catalog/pg_variable.h" #include "commands/session_variable.h" +#include "executor/spi.h" #include "executor/svariableReceiver.h" #include "funcapi.h" #include "miscadmin.h" @@ -31,6 +32,13 @@ #include "utils/snapmgr.h" #include "utils/syscache.h" +/* + * The session variables use fence context allow to specify + * the contexts where using session variable fences are required. + */ +int session_variables_use_fence_context_guard = + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE; + /* * The values of session variables are stored in the backend's private memory * in the dedicated memory context SVariableMemoryContext in binary format. @@ -816,3 +824,28 @@ ResetSessionVariables(void) if (SVariableMemoryContext != NULL) MemoryContextReset(SVariableMemoryContext); } + +/* + * Raise error when unfenced variable is used in wrong context. + * This check allows to force session variable fencing outside + * stored procedures environment. + */ +void +SessionVariablesUseFenceContextCheck(Oid varid, bool is_fenced) +{ + if (is_fenced) + return; + + if (((session_variables_use_fence_context_guard == + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_ALL) && !is_fenced) || + ((session_variables_use_fence_context_guard == + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NOSPI) && + !SPI_inside_spi_context())) + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("session variable \"%s.%s\" is not used inside variable fence", + get_namespace_name(get_session_variable_namespace(varid)), + get_session_variable_name(varid)), + errdetail("There is a risk of unwanted usage of session variable."), + errhint("Use variable fence \"VARIABLE(varname) for access to variable\"."))); +} diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 9b55861ea64..2ffb56ae451 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -34,6 +34,7 @@ #include "catalog/objectaccess.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" +#include "commands/session_variable.h" #include "executor/execExpr.h" #include "executor/nodeSubplan.h" #include "funcapi.h" @@ -998,6 +999,9 @@ ExecInitExprRec(Expr *node, ExprState *state, SessionVariableValue *es_session_variables = NULL; SessionVariableValue *var; + SessionVariablesUseFenceContextCheck(param->paramvarid, + param->paramvarfenced); + if (state->parent && state->parent->state) { es_session_variables = state->parent->state->es_session_variables; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index ecb2e4ccaa1..4ce6bc5f298 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -590,6 +590,14 @@ SPI_inside_nonatomic_context(void) return true; } +/* + * Are we executed inside a SPI? + */ +bool +SPI_inside_spi_context(void) +{ + return (_SPI_current != NULL); +} /* Parse, plan, and execute a query string */ int diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 4f96788d1db..9b3d848ad95 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -40,6 +40,7 @@ #include "catalog/storage.h" #include "commands/async.h" #include "commands/event_trigger.h" +#include "commands/session_variable.h" #include "commands/tablespace.h" #include "commands/trigger.h" #include "commands/user.h" @@ -423,6 +424,13 @@ static const struct config_enum_entry debug_logical_replication_streaming_option {NULL, 0, false} }; +static const struct config_enum_entry session_variables_use_fence_context_guard_options[] = { + {"none", SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE, false}, + {"all", SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_ALL, false}, + {"nospi", SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NOSPI, false}, + {NULL, 0, false} +}; + StaticAssertDecl(lengthof(ssl_protocol_versions_info) == (PG_TLS1_3_VERSION + 2), "array length mismatch"); @@ -5244,6 +5252,17 @@ struct config_enum ConfigureNamesEnum[] = NULL, NULL, NULL }, + { + {"session_variables_use_fence_context_guard", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Chooses the context where session variables should be used inside fence."), + NULL + }, + &session_variables_use_fence_context_guard, + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE, session_variables_use_fence_context_guard_options, + NULL, NULL, NULL + }, + + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, 0, NULL, NULL, NULL, NULL diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 47041cbe80b..c3062b22915 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -718,6 +718,7 @@ #session_replication_role = 'origin' #session_variables_ambiguity_warning = off #session_variables_use_fence_warning_guard = off +#session_variables_use_fence_context_guard = 'none' #statement_timeout = 0 # in milliseconds, 0 is disabled #transaction_timeout = 0 # in milliseconds, 0 is disabled #lock_timeout = 0 # in milliseconds, 0 is disabled diff --git a/src/include/commands/session_variable.h b/src/include/commands/session_variable.h index 07d800bcc01..06ce9640085 100644 --- a/src/include/commands/session_variable.h +++ b/src/include/commands/session_variable.h @@ -31,5 +31,21 @@ extern void ExecuteLetStmt(ParseState *pstate, LetStmt *stmt, ParamListInfo para QueryEnvironment *queryEnv, QueryCompletion *qc); extern void ResetSessionVariables(void); +extern void SessionVariablesUseFenceContextCheck(Oid varid, bool is_fenced); + +/* + * Specify the dynamic context where variable fence is required + */ +typedef enum SessionVariablesUseFenceContextGuard +{ + /* Disable fence usage context guard */ + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NONE, + /* Enable fence usage context guard everywhere */ + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_ALL, + /* Enable fence usage context guard when SPI is not used */ + SESSIONVARIABLE_USE_FENCE_CONTEXT_GUARD_NOSPI, +} SessionVariablesUseFenceContextGuard; + +extern PGDLLIMPORT int session_variables_use_fence_context_guard; #endif diff --git a/src/include/executor/spi.h b/src/include/executor/spi.h index d064d1a9b76..4690a95f39a 100644 --- a/src/include/executor/spi.h +++ b/src/include/executor/spi.h @@ -203,5 +203,6 @@ extern void SPI_rollback_and_chain(void); extern void AtEOXact_SPI(bool isCommit); extern void AtEOSubXact_SPI(bool isCommit, SubTransactionId mySubid); extern bool SPI_inside_nonatomic_context(void); +extern bool SPI_inside_spi_context(void); #endif /* SPI_H */ diff --git a/src/test/regress/expected/session_variables.out b/src/test/regress/expected/session_variables.out index 174f9140dcf..309c3f36998 100644 --- a/src/test/regress/expected/session_variables.out +++ b/src/test/regress/expected/session_variables.out @@ -1996,3 +1996,93 @@ DROP SCHEMA testvar; SET session_variables_ambiguity_warning TO DEFAULT; SET session_variables_use_fence_warning_guard TO DEFAULT; SET search_path TO DEFAULT; +-- test session_variables_use_fence_context_guard +CREATE VARIABLE var1 AS int; +CREATE TABLE vartest_tab1(a int); +LET var1 = 20; +INSERT INTO vartest_tab1 VALUES(20); +SET session_variables_use_fence_context_guard TO 'none'; +-- should be ok +SELECT var1, VARIABLE(var1); + var1 | var1 +------+------ + 20 | 20 +(1 row) + +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; +NOTICE: 20 t +NOTICE: 20 t +SET session_variables_use_fence_context_guard TO 'all'; +-- should fail +SELECT var1, VARIABLE(var1); +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +-- should fail +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; +END; +$$; +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +CONTEXT: SQL statement "SELECT a FROM vartest_tab1 WHERE a = var1" +PL/pgSQL function inline_code_block line 4 at SQL statement +-- should fail +DO $$ +DECLARE t int DEFAULT 20; +BEGIN + RAISE NOTICE '% %', var1, var1 = t; +END; +$$; +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +CONTEXT: PL/pgSQL expression "var1" +PL/pgSQL function inline_code_block line 4 at RAISE +-- should be ok +SELECT VARIABLE(var1), VARIABLE(var1); + var1 | var1 +------+------ + 20 | 20 +(1 row) + +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; +NOTICE: 20 t +SET session_variables_use_fence_context_guard TO 'nospi'; +-- should fail +SELECT var1, VARIABLE(var1); +ERROR: session variable "public.var1" is not used inside variable fence +DETAIL: There is a risk of unwanted usage of session variable. +HINT: Use variable fence "VARIABLE(varname) for access to variable". +-- should be ok +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; +NOTICE: 20 t +NOTICE: 20 t +SET session_variables_use_fence_context_guard TO DEFAULT; +DROP VARIABLE var1; +DROP TABLE vartest_tab1; diff --git a/src/test/regress/sql/session_variables.sql b/src/test/regress/sql/session_variables.sql index 172918b603c..c49ceed33e1 100644 --- a/src/test/regress/sql/session_variables.sql +++ b/src/test/regress/sql/session_variables.sql @@ -1376,3 +1376,76 @@ DROP SCHEMA testvar; SET session_variables_ambiguity_warning TO DEFAULT; SET session_variables_use_fence_warning_guard TO DEFAULT; SET search_path TO DEFAULT; + +-- test session_variables_use_fence_context_guard +CREATE VARIABLE var1 AS int; +CREATE TABLE vartest_tab1(a int); + +LET var1 = 20; +INSERT INTO vartest_tab1 VALUES(20); + +SET session_variables_use_fence_context_guard TO 'none'; + +-- should be ok +SELECT var1, VARIABLE(var1); +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; + +SET session_variables_use_fence_context_guard TO 'all'; + +-- should fail +SELECT var1, VARIABLE(var1); + +-- should fail +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; +END; +$$; + +-- should fail +DO $$ +DECLARE t int DEFAULT 20; +BEGIN + RAISE NOTICE '% %', var1, var1 = t; +END; +$$; + +-- should be ok +SELECT VARIABLE(var1), VARIABLE(var1); +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; + +SET session_variables_use_fence_context_guard TO 'nospi'; + +-- should fail +SELECT var1, VARIABLE(var1); + +-- should be ok +DO $$ +DECLARE t int; +BEGIN + SELECT a FROM vartest_tab1 WHERE a = var1 INTO t; + RAISE NOTICE '% %', var1, var1 = t; + SELECT a FROM vartest_tab1 WHERE a = VARIABLE(var1) INTO t; + RAISE NOTICE '% %', VARIABLE(var1), VARIABLE(var1) = t; +END; +$$; + +SET session_variables_use_fence_context_guard TO DEFAULT; + +DROP VARIABLE var1; +DROP TABLE vartest_tab1; -- 2.47.1