From ee3ee2e4f6a4723e2206cd6947715409c4a5f30d 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 65f19598c2..b1dd7e50d0 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 be3f93b2cb..8eaf41e074 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 2c3e59e1c2..5d923eac66 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 3a93a0c733..59315c4961 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -35,6 +35,7 @@ #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "catalog/pg_variable.h" +#include "commands/session_variable.h" #include "executor/execExpr.h" #include "executor/nodeSubplan.h" #include "funcapi.h" @@ -999,6 +1000,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 c1d8fd08c6..659bae0fbc 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 853a80d79a..2e3679b65b 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 47041cbe80..c3062b2291 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 39499d114f..90e9bc1a83 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 48b87730ea..4ea8bda931 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 824f1feab0..8e4b44763d 100644 --- a/src/test/regress/expected/session_variables.out +++ b/src/test/regress/expected/session_variables.out @@ -1860,3 +1860,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 61651e9674..447b042e70 100644 --- a/src/test/regress/sql/session_variables.sql +++ b/src/test/regress/sql/session_variables.sql @@ -1255,3 +1255,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