Re: pg_parameter_aclcheck() and trusted extensions - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Re: pg_parameter_aclcheck() and trusted extensions |
| Date | |
| Msg-id | 3126292.1657828950@sss.pgh.pa.us Whole thread Raw |
| In response to | Re: pg_parameter_aclcheck() and trusted extensions (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: pg_parameter_aclcheck() and trusted extensions
|
| List | pgsql-hackers |
I wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> Looks like a bug to me, so I have added an open item assigned to Tom.
> Yeah. So the fix here seems pretty obvious: rather than applying the
> permissions check using bare GetUserId(), we need to remember the role
> OID that originally applied the setting, and use that.
Here's a draft patch for that. I initially ran around and changed all
the set_config_option callers as I threatened before, but as I did it
I could not help observing that they were all changing in exactly the
same way: basically, they were passing GetUserId() if the GucContext
is PGC_S_SESSION and BOOTSTRAP_SUPERUSERID otherwise. Not counting
guc.c internal call sites, there is a grand total of one caller that
fails to fit the pattern. So that brought me around to liking the idea
of keeping set_config_option's API stable by making it a thin wrapper
around another function with an explicit role argument. The result,
attached, poses far less of an API/ABI hazard than I was anticipating.
If you're not poking into the GUC tables you have little to fear.
Most of the bulk of this is mechanical changes to pass the source
role around properly in guc.c's data structures. That's all basically
copy-and-paste from the code to track the source context (scontext).
I noted something that ought to be looked at separately:
validate_option_array_item() seems like it needs to be taught about
grantable permissions on GUCs. I think that right now it may report
permissions failures in some cases where it should succeed.
regards, tom lane
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 3db859c3ea..6b6720c690 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -912,6 +912,9 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
* We use the equivalent of a function SET option to allow the setting to
* persist for exactly the duration of the script execution. guc.c also
* takes care of undoing the setting on error.
+ *
+ * log_min_messages can't be set by ordinary users, so for that one we
+ * pretend to be superuser.
*/
save_nestlevel = NewGUCNestLevel();
@@ -920,9 +923,10 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
PGC_USERSET, PGC_S_SESSION,
GUC_ACTION_SAVE, true, 0, false);
if (log_min_messages < WARNING)
- (void) set_config_option("log_min_messages", "warning",
- PGC_SUSET, PGC_S_SESSION,
- GUC_ACTION_SAVE, true, 0, false);
+ (void) set_config_option_ext("log_min_messages", "warning",
+ PGC_SUSET, PGC_S_SESSION,
+ BOOTSTRAP_SUPERUSERID,
+ GUC_ACTION_SAVE, true, 0, false);
/*
* Similarly disable check_function_bodies, to ensure that SQL functions
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 0328029d43..9e6cb590c7 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -5172,7 +5172,8 @@ static void reapply_stacked_values(struct config_generic *variable,
struct config_string *pHolder,
GucStack *stack,
const char *curvalue,
- GucContext curscontext, GucSource cursource);
+ GucContext curscontext, GucSource cursource,
+ Oid cursrole);
static void ShowGUCConfigOption(const char *name, DestReceiver *dest);
static void ShowAllGUCConfig(DestReceiver *dest);
static char *_ShowOption(struct config_generic *record, bool use_units);
@@ -5897,10 +5898,10 @@ InitializeWalConsistencyChecking(void)
check_wal_consistency_checking_deferred = false;
- set_config_option("wal_consistency_checking",
- wal_consistency_checking_string,
- PGC_POSTMASTER, guc->source,
- GUC_ACTION_SET, true, ERROR, false);
+ set_config_option_ext("wal_consistency_checking",
+ wal_consistency_checking_string,
+ guc->scontext, guc->source, guc->srole,
+ GUC_ACTION_SET, true, ERROR, false);
/* checking should not be deferred again */
Assert(!check_wal_consistency_checking_deferred);
@@ -5979,6 +5980,8 @@ InitializeOneGUCOption(struct config_generic *gconf)
gconf->reset_source = PGC_S_DEFAULT;
gconf->scontext = PGC_INTERNAL;
gconf->reset_scontext = PGC_INTERNAL;
+ gconf->srole = BOOTSTRAP_SUPERUSERID;
+ gconf->reset_srole = BOOTSTRAP_SUPERUSERID;
gconf->stack = NULL;
gconf->extra = NULL;
gconf->last_reported = NULL;
@@ -6356,6 +6359,7 @@ ResetAllOptions(void)
gconf->source = gconf->reset_source;
gconf->scontext = gconf->reset_scontext;
+ gconf->srole = gconf->reset_srole;
if (gconf->flags & GUC_REPORT)
{
@@ -6401,6 +6405,7 @@ push_old_value(struct config_generic *gconf, GucAction action)
{
/* SET followed by SET LOCAL, remember SET's value */
stack->masked_scontext = gconf->scontext;
+ stack->masked_srole = gconf->srole;
set_stack_value(gconf, &stack->masked);
stack->state = GUC_SET_LOCAL;
}
@@ -6439,6 +6444,7 @@ push_old_value(struct config_generic *gconf, GucAction action)
}
stack->source = gconf->source;
stack->scontext = gconf->scontext;
+ stack->srole = gconf->srole;
set_stack_value(gconf, &stack->prior);
gconf->stack = stack;
@@ -6583,6 +6589,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
{
/* LOCAL migrates down */
prev->masked_scontext = stack->scontext;
+ prev->masked_srole = stack->srole;
prev->masked = stack->prior;
prev->state = GUC_SET_LOCAL;
}
@@ -6598,6 +6605,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
discard_stack_value(gconf, &stack->prior);
/* copy down the masked state */
prev->masked_scontext = stack->masked_scontext;
+ prev->masked_srole = stack->masked_srole;
if (prev->state == GUC_SET_LOCAL)
discard_stack_value(gconf, &prev->masked);
prev->masked = stack->masked;
@@ -6614,18 +6622,21 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
config_var_value newvalue;
GucSource newsource;
GucContext newscontext;
+ Oid newsrole;
if (restoreMasked)
{
newvalue = stack->masked;
newsource = PGC_S_SESSION;
newscontext = stack->masked_scontext;
+ newsrole = stack->masked_srole;
}
else
{
newvalue = stack->prior;
newsource = stack->source;
newscontext = stack->scontext;
+ newsrole = stack->srole;
}
switch (gconf->vartype)
@@ -6740,6 +6751,7 @@ AtEOXact_GUC(bool isCommit, int nestLevel)
/* And restore source information */
gconf->source = newsource;
gconf->scontext = newscontext;
+ gconf->srole = newsrole;
}
/* Finish popping the state stack */
@@ -7520,7 +7532,7 @@ parse_and_validate_value(struct config_generic *record,
/*
- * Sets option `name' to given value.
+ * set_config_option: sets option `name' to given value.
*
* The value should be a string, which will be parsed and converted to
* the appropriate data type. The context and source parameters indicate
@@ -7563,6 +7575,46 @@ set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
GucAction action, bool changeVal, int elevel,
bool is_reload)
+{
+ Oid srole;
+
+ /*
+ * Non-interactive sources should be treated as having all privileges,
+ * except for PGC_S_CLIENT. Note in particular that this is true for
+ * pg_db_role_setting sources (PGC_S_GLOBAL etc): we assume a suitable
+ * privilege check was done when the pg_db_role_setting entry was made.
+ */
+ if (source >= PGC_S_INTERACTIVE || source == PGC_S_CLIENT)
+ srole = GetUserId();
+ else
+ srole = BOOTSTRAP_SUPERUSERID;
+
+ return set_config_option_ext(name, value,
+ context, source, srole,
+ action, changeVal, elevel,
+ is_reload);
+}
+
+/*
+ * set_config_option_ext: sets option `name' to given value.
+ *
+ * This API adds the ability to explicitly specify which role OID
+ * is considered to be setting the value. Most external callers can use
+ * set_config_option() and let it determine that based on the GucSource,
+ * but there are a few that are supplying a value that was determined
+ * in some special way and need to override the decision. Also, when
+ * restoring a previously-assigned value, it's important to supply the
+ * same role OID that set the value originally; so all guc.c callers
+ * that are doing that type of thing need to call this directly.
+ *
+ * Generally, srole should be GetUserId() when the source is a SQL operation,
+ * or BOOTSTRAP_SUPERUSERID if the source is a config file or similar.
+ */
+int
+set_config_option_ext(const char *name, const char *value,
+ GucContext context, GucSource source, Oid srole,
+ GucAction action, bool changeVal, int elevel,
+ bool is_reload)
{
struct config_generic *record;
union config_var_val newval_union;
@@ -7667,12 +7719,12 @@ set_config_option(const char *name, const char *value,
if (context == PGC_BACKEND)
{
/*
- * Check whether the current user has been granted privilege
- * to set this GUC.
+ * Check whether the requesting user has been granted
+ * privilege to set this GUC.
*/
AclResult aclresult;
- aclresult = pg_parameter_aclcheck(name, GetUserId(), ACL_SET);
+ aclresult = pg_parameter_aclcheck(name, srole, ACL_SET);
if (aclresult != ACLCHECK_OK)
{
/* No granted privilege */
@@ -7725,12 +7777,12 @@ set_config_option(const char *name, const char *value,
if (context == PGC_USERSET || context == PGC_BACKEND)
{
/*
- * Check whether the current user has been granted privilege
- * to set this GUC.
+ * Check whether the requesting user has been granted
+ * privilege to set this GUC.
*/
AclResult aclresult;
- aclresult = pg_parameter_aclcheck(name, GetUserId(), ACL_SET);
+ aclresult = pg_parameter_aclcheck(name, srole, ACL_SET);
if (aclresult != ACLCHECK_OK)
{
/* No granted privilege */
@@ -7847,6 +7899,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra;
source = conf->gen.reset_source;
context = conf->gen.reset_scontext;
+ srole = conf->gen.reset_srole;
}
if (prohibitValueChange)
@@ -7881,6 +7934,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.source = source;
conf->gen.scontext = context;
+ conf->gen.srole = srole;
}
if (makeDefault)
{
@@ -7893,6 +7947,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.reset_source = source;
conf->gen.reset_scontext = context;
+ conf->gen.reset_srole = srole;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
@@ -7903,6 +7958,7 @@ set_config_option(const char *name, const char *value,
newextra);
stack->source = source;
stack->scontext = context;
+ stack->srole = srole;
}
}
}
@@ -7941,6 +7997,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra;
source = conf->gen.reset_source;
context = conf->gen.reset_scontext;
+ srole = conf->gen.reset_srole;
}
if (prohibitValueChange)
@@ -7975,6 +8032,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.source = source;
conf->gen.scontext = context;
+ conf->gen.srole = srole;
}
if (makeDefault)
{
@@ -7987,6 +8045,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.reset_source = source;
conf->gen.reset_scontext = context;
+ conf->gen.reset_srole = srole;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
@@ -7997,6 +8056,7 @@ set_config_option(const char *name, const char *value,
newextra);
stack->source = source;
stack->scontext = context;
+ stack->srole = srole;
}
}
}
@@ -8035,6 +8095,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra;
source = conf->gen.reset_source;
context = conf->gen.reset_scontext;
+ srole = conf->gen.reset_srole;
}
if (prohibitValueChange)
@@ -8069,6 +8130,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.source = source;
conf->gen.scontext = context;
+ conf->gen.srole = srole;
}
if (makeDefault)
{
@@ -8081,6 +8143,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.reset_source = source;
conf->gen.reset_scontext = context;
+ conf->gen.reset_srole = srole;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
@@ -8091,6 +8154,7 @@ set_config_option(const char *name, const char *value,
newextra);
stack->source = source;
stack->scontext = context;
+ stack->srole = srole;
}
}
}
@@ -8145,6 +8209,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra;
source = conf->gen.reset_source;
context = conf->gen.reset_scontext;
+ srole = conf->gen.reset_srole;
}
if (prohibitValueChange)
@@ -8189,6 +8254,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.source = source;
conf->gen.scontext = context;
+ conf->gen.srole = srole;
}
if (makeDefault)
@@ -8202,6 +8268,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.reset_source = source;
conf->gen.reset_scontext = context;
+ conf->gen.reset_srole = srole;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
@@ -8213,6 +8280,7 @@ set_config_option(const char *name, const char *value,
newextra);
stack->source = source;
stack->scontext = context;
+ stack->srole = srole;
}
}
}
@@ -8254,6 +8322,7 @@ set_config_option(const char *name, const char *value,
newextra = conf->reset_extra;
source = conf->gen.reset_source;
context = conf->gen.reset_scontext;
+ srole = conf->gen.reset_srole;
}
if (prohibitValueChange)
@@ -8288,6 +8357,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.source = source;
conf->gen.scontext = context;
+ conf->gen.srole = srole;
}
if (makeDefault)
{
@@ -8300,6 +8370,7 @@ set_config_option(const char *name, const char *value,
newextra);
conf->gen.reset_source = source;
conf->gen.reset_scontext = context;
+ conf->gen.reset_srole = srole;
}
for (stack = conf->gen.stack; stack; stack = stack->prev)
{
@@ -8310,6 +8381,7 @@ set_config_option(const char *name, const char *value,
newextra);
stack->source = source;
stack->scontext = context;
+ stack->srole = srole;
}
}
}
@@ -9354,17 +9426,19 @@ define_custom_variable(struct config_generic *variable)
/* First, apply the reset value if any */
if (pHolder->reset_val)
- (void) set_config_option(name, pHolder->reset_val,
- pHolder->gen.reset_scontext,
- pHolder->gen.reset_source,
- GUC_ACTION_SET, true, WARNING, false);
+ (void) set_config_option_ext(name, pHolder->reset_val,
+ pHolder->gen.reset_scontext,
+ pHolder->gen.reset_source,
+ pHolder->gen.reset_srole,
+ GUC_ACTION_SET, true, WARNING, false);
/* That should not have resulted in stacking anything */
Assert(variable->stack == NULL);
/* Now, apply current and stacked values, in the order they were stacked */
reapply_stacked_values(variable, pHolder, pHolder->gen.stack,
*(pHolder->variable),
- pHolder->gen.scontext, pHolder->gen.source);
+ pHolder->gen.scontext, pHolder->gen.source,
+ pHolder->gen.srole);
/* Also copy over any saved source-location information */
if (pHolder->gen.sourcefile)
@@ -9395,7 +9469,8 @@ reapply_stacked_values(struct config_generic *variable,
struct config_string *pHolder,
GucStack *stack,
const char *curvalue,
- GucContext curscontext, GucSource cursource)
+ GucContext curscontext, GucSource cursource,
+ Oid cursrole)
{
const char *name = variable->name;
GucStack *oldvarstack = variable->stack;
@@ -9405,43 +9480,45 @@ reapply_stacked_values(struct config_generic *variable,
/* First, recurse, so that stack items are processed bottom to top */
reapply_stacked_values(variable, pHolder, stack->prev,
stack->prior.val.stringval,
- stack->scontext, stack->source);
+ stack->scontext, stack->source, stack->srole);
/* See how to apply the passed-in value */
switch (stack->state)
{
case GUC_SAVE:
- (void) set_config_option(name, curvalue,
- curscontext, cursource,
- GUC_ACTION_SAVE, true,
- WARNING, false);
+ (void) set_config_option_ext(name, curvalue,
+ curscontext, cursource, cursrole,
+ GUC_ACTION_SAVE, true,
+ WARNING, false);
break;
case GUC_SET:
- (void) set_config_option(name, curvalue,
- curscontext, cursource,
- GUC_ACTION_SET, true,
- WARNING, false);
+ (void) set_config_option_ext(name, curvalue,
+ curscontext, cursource, cursrole,
+ GUC_ACTION_SET, true,
+ WARNING, false);
break;
case GUC_LOCAL:
- (void) set_config_option(name, curvalue,
- curscontext, cursource,
- GUC_ACTION_LOCAL, true,
- WARNING, false);
+ (void) set_config_option_ext(name, curvalue,
+ curscontext, cursource, cursrole,
+ GUC_ACTION_LOCAL, true,
+ WARNING, false);
break;
case GUC_SET_LOCAL:
/* first, apply the masked value as SET */
- (void) set_config_option(name, stack->masked.val.stringval,
- stack->masked_scontext, PGC_S_SESSION,
- GUC_ACTION_SET, true,
- WARNING, false);
+ (void) set_config_option_ext(name, stack->masked.val.stringval,
+ stack->masked_scontext,
+ PGC_S_SESSION,
+ stack->masked_srole,
+ GUC_ACTION_SET, true,
+ WARNING, false);
/* then apply the current value as LOCAL */
- (void) set_config_option(name, curvalue,
- curscontext, cursource,
- GUC_ACTION_LOCAL, true,
- WARNING, false);
+ (void) set_config_option_ext(name, curvalue,
+ curscontext, cursource, cursrole,
+ GUC_ACTION_LOCAL, true,
+ WARNING, false);
break;
}
@@ -9461,11 +9538,12 @@ reapply_stacked_values(struct config_generic *variable,
*/
if (curvalue != pHolder->reset_val ||
curscontext != pHolder->gen.reset_scontext ||
- cursource != pHolder->gen.reset_source)
+ cursource != pHolder->gen.reset_source ||
+ cursrole != pHolder->gen.reset_srole)
{
- (void) set_config_option(name, curvalue,
- curscontext, cursource,
- GUC_ACTION_SET, true, WARNING, false);
+ (void) set_config_option_ext(name, curvalue,
+ curscontext, cursource, cursrole,
+ GUC_ACTION_SET, true, WARNING, false);
variable->stack = NULL;
}
}
@@ -10577,6 +10655,7 @@ _ShowOption(struct config_generic *record, bool use_units)
* variable sourceline, integer
* variable source, integer
* variable scontext, integer
+* variable srole, OID
*/
static void
write_one_nondefault_variable(FILE *fp, struct config_generic *gconf)
@@ -10643,6 +10722,7 @@ write_one_nondefault_variable(FILE *fp, struct config_generic *gconf)
fwrite(&gconf->sourceline, 1, sizeof(gconf->sourceline), fp);
fwrite(&gconf->source, 1, sizeof(gconf->source), fp);
fwrite(&gconf->scontext, 1, sizeof(gconf->scontext), fp);
+ fwrite(&gconf->srole, 1, sizeof(gconf->srole), fp);
}
void
@@ -10738,6 +10818,7 @@ read_nondefault_variables(void)
int varsourceline;
GucSource varsource;
GucContext varscontext;
+ Oid varsrole;
/*
* Open file
@@ -10774,10 +10855,12 @@ read_nondefault_variables(void)
elog(FATAL, "invalid format of exec config params file");
if (fread(&varscontext, 1, sizeof(varscontext), fp) != sizeof(varscontext))
elog(FATAL, "invalid format of exec config params file");
+ if (fread(&varsrole, 1, sizeof(varsrole), fp) != sizeof(varsrole))
+ elog(FATAL, "invalid format of exec config params file");
- (void) set_config_option(varname, varvalue,
- varscontext, varsource,
- GUC_ACTION_SET, true, 0, true);
+ (void) set_config_option_ext(varname, varvalue,
+ varscontext, varsource, varsrole,
+ GUC_ACTION_SET, true, 0, true);
if (varsourcefile[0])
set_config_sourcefile(varname, varsourcefile, varsourceline);
@@ -10931,6 +11014,7 @@ estimate_variable_size(struct config_generic *gconf)
size = add_size(size, sizeof(gconf->source));
size = add_size(size, sizeof(gconf->scontext));
+ size = add_size(size, sizeof(gconf->srole));
return size;
}
@@ -11076,6 +11160,8 @@ serialize_variable(char **destptr, Size *maxbytes,
sizeof(gconf->source));
do_serialize_binary(destptr, maxbytes, &gconf->scontext,
sizeof(gconf->scontext));
+ do_serialize_binary(destptr, maxbytes, &gconf->srole,
+ sizeof(gconf->srole));
}
/*
@@ -11177,6 +11263,7 @@ RestoreGUCState(void *gucstate)
int varsourceline;
GucSource varsource;
GucContext varscontext;
+ Oid varsrole;
char *srcptr = (char *) gucstate;
char *srcend;
Size len;
@@ -11304,12 +11391,15 @@ RestoreGUCState(void *gucstate)
&varsource, sizeof(varsource));
read_gucstate_binary(&srcptr, srcend,
&varscontext, sizeof(varscontext));
+ read_gucstate_binary(&srcptr, srcend,
+ &varsrole, sizeof(varsrole));
error_context_name_and_value[0] = varname;
error_context_name_and_value[1] = varvalue;
error_context_callback.arg = &error_context_name_and_value[0];
- result = set_config_option(varname, varvalue, varscontext, varsource,
- GUC_ACTION_SET, true, ERROR, true);
+ result = set_config_option_ext(varname, varvalue,
+ varscontext, varsource, varsrole,
+ GUC_ACTION_SET, true, ERROR, true);
if (result <= 0)
ereport(ERROR,
(errcode(ERRCODE_INTERNAL_ERROR),
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 4d0920c42e..e734493a48 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -388,6 +388,11 @@ extern int set_config_option(const char *name, const char *value,
GucContext context, GucSource source,
GucAction action, bool changeVal, int elevel,
bool is_reload);
+extern int set_config_option_ext(const char *name, const char *value,
+ GucContext context, GucSource source,
+ Oid srole,
+ GucAction action, bool changeVal, int elevel,
+ bool is_reload);
extern void AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt);
extern char *GetConfigOptionByName(const char *name, const char **varname,
bool missing_ok);
diff --git a/src/include/utils/guc_tables.h b/src/include/utils/guc_tables.h
index ba44f7437b..067d82ada9 100644
--- a/src/include/utils/guc_tables.h
+++ b/src/include/utils/guc_tables.h
@@ -120,6 +120,8 @@ typedef struct guc_stack
/* masked value's source must be PGC_S_SESSION, so no need to store it */
GucContext scontext; /* context that set the prior value */
GucContext masked_scontext; /* context that set the masked value */
+ Oid srole; /* role that set the prior value */
+ Oid masked_srole; /* role that set the masked value */
config_var_value prior; /* previous value of variable */
config_var_value masked; /* SET value in a GUC_SET_LOCAL entry */
} GucStack;
@@ -131,6 +133,10 @@ typedef struct guc_stack
* applications may use the long description as well, and will append
* it to the short description. (separated by a newline or '. ')
*
+ * srole is the role that set the current value, or BOOTSTRAP_SUPERUSERID
+ * if the value came from an internal source or the config file. Similarly
+ * for reset_srole (which is usually BOOTSTRAP_SUPERUSERID, but not always).
+ *
* Note that sourcefile/sourceline are kept here, and not pushed into stacked
* values, although in principle they belong with some stacked value if the
* active value is session- or transaction-local. This is to avoid bloating
@@ -152,6 +158,8 @@ struct config_generic
GucSource reset_source; /* source of the reset_value */
GucContext scontext; /* context that set the current value */
GucContext reset_scontext; /* context that set the reset value */
+ Oid srole; /* role that set the current value */
+ Oid reset_srole; /* role that set the reset value */
GucStack *stack; /* stacked prior values */
void *extra; /* "extra" pointer for current actual value */
char *last_reported; /* if variable is GUC_REPORT, value last sent
diff --git a/src/pl/plperl/expected/plperl_init.out b/src/pl/plperl/expected/plperl_init.out
index 133828e9f3..4b7e925419 100644
--- a/src/pl/plperl/expected/plperl_init.out
+++ b/src/pl/plperl/expected/plperl_init.out
@@ -1,4 +1,4 @@
--- test plperl.on_plperl_init errors are fatal
+-- test plperl.on_plperl_init
-- This test tests setting on_plperl_init after loading plperl
LOAD 'plperl';
SET SESSION plperl.on_plperl_init = ' system("/nonesuch"); ';
@@ -12,3 +12,29 @@ DO $$ warn 42 $$ language plperl;
ERROR: 'system' trapped by operation mask at line 1.
CONTEXT: while executing plperl.on_plperl_init
PL/Perl anonymous code block
+--
+-- Reconnect (to unload plperl), then test setting on_plperl_init
+-- as an unprivileged user
+--
+\c -
+CREATE ROLE regress_plperl_user;
+SET ROLE regress_plperl_user;
+-- this succeeds, since the GUC isn't known yet
+SET SESSION plperl.on_plperl_init = 'test';
+RESET ROLE;
+LOAD 'plperl';
+WARNING: permission denied to set parameter "plperl.on_plperl_init"
+SHOW plperl.on_plperl_init;
+ plperl.on_plperl_init
+-----------------------
+
+(1 row)
+
+DO $$ warn 42 $$ language plperl;
+WARNING: 42 at line 1.
+-- now we won't be allowed to set it in the first place
+SET ROLE regress_plperl_user;
+SET SESSION plperl.on_plperl_init = 'test';
+ERROR: permission denied to set parameter "plperl.on_plperl_init"
+RESET ROLE;
+DROP ROLE regress_plperl_user;
diff --git a/src/pl/plperl/sql/plperl_init.sql b/src/pl/plperl/sql/plperl_init.sql
index 4ebf3f86eb..2aa3811033 100644
--- a/src/pl/plperl/sql/plperl_init.sql
+++ b/src/pl/plperl/sql/plperl_init.sql
@@ -1,4 +1,4 @@
--- test plperl.on_plperl_init errors are fatal
+-- test plperl.on_plperl_init
-- This test tests setting on_plperl_init after loading plperl
LOAD 'plperl';
@@ -8,3 +8,34 @@ SET SESSION plperl.on_plperl_init = ' system("/nonesuch"); ';
SHOW plperl.on_plperl_init;
DO $$ warn 42 $$ language plperl;
+
+--
+-- Reconnect (to unload plperl), then test setting on_plperl_init
+-- as an unprivileged user
+--
+
+\c -
+
+CREATE ROLE regress_plperl_user;
+
+SET ROLE regress_plperl_user;
+
+-- this succeeds, since the GUC isn't known yet
+SET SESSION plperl.on_plperl_init = 'test';
+
+RESET ROLE;
+
+LOAD 'plperl';
+
+SHOW plperl.on_plperl_init;
+
+DO $$ warn 42 $$ language plperl;
+
+-- now we won't be allowed to set it in the first place
+SET ROLE regress_plperl_user;
+
+SET SESSION plperl.on_plperl_init = 'test';
+
+RESET ROLE;
+
+DROP ROLE regress_plperl_user;
pgsql-hackers by date: