From: Laurenz Albe <laurenz.albe@cybertec.at>
To: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers@postgresql.org
Cc: simon@2ndquadrant.com
Subject: Re: SHOW ALL does not honor pg_read_all_settings membership
Date: Fri, 20 Apr 2018 13:21:46 +0200
Message-ID: <1524223306.2480.6.camel@cybertec.at> (raw)
In-Reply-To: <20180417032858.GD4998@paquier.xyz>
References: <1519917758.6586.8.camel@cybertec.at>
<1523881930.3779.37.camel@cybertec.at>
<20180417032858.GD4998@paquier.xyz>
Michael Paquier wrote:
> On Mon, Apr 16, 2018 at 02:32:10PM +0200, Laurenz Albe wrote:
> > Now that the dust from the last commitfest is settling, I'll make a second
> > attempt to attract attention for this small bug fix.
> >
> > The original commit was Simon's.
>
> Thanks for the ping.
>
> This was new as of v10, so this cannot be listed as an open item still I
> have added that under the section for older bugs, because you are right
> as far as I can see.
>
> GetConfigOption is wrong by the way, as restrict_superuser means that
> all members of the group pg_read_all_settings can read
> GUC_SUPERUSER_ONLY params, and not only superusers, so the comment at
> least needs a fix, the variable ought to be renamed as well.
Thanks for the review!
I agree; here is a patch for that.
Yours,
Laurenz Albe
Attachments:0002-Fix-comments-and-a-parameter-name.patch (text/x-patch, 2.6K) download
From cfe04ef974dba324c47510b854587de6c33e39a1 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 20 Apr 2018 13:13:20 +0200
Subject: [PATCH] Fix comments and a parameter name
Commit 25fff40798fc4ac11a241bfd9ab0c45c085e2212 changed the semantics
of GetConfigOption, but did not update the comment.
Also, the parameter name should better be changed.
Noted by Michael Paquier.
---
src/backend/utils/misc/guc.c | 10 +++++-----
src/include/utils/guc.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.cindex 7acec5ea21..b5501fd949 100644--- a/src/backend/utils/misc/guc.c+++ b/src/backend/utils/misc/guc.c@@ -6869,15 +6869,15 @@ SetConfigOption(const char *name, const char *value,
* this cannot be distinguished from a string variable with a NULL value!),
* otherwise throw an ereport and don't return.
*
- * If restrict_superuser is true, we also enforce that only superusers can- * see GUC_SUPERUSER_ONLY variables. This should only be passed as true- * in user-driven calls.+ * If restrict_privileged is true, we also enforce that only superusers and+ * members of the pg_read_all_settings role can see GUC_SUPERUSER_ONLY+ * variables. This should only be passed as true in user-driven calls.
*
* The string is *not* allocated for modification and is really only
* valid until the next call to configuration related functions.
*/
const char *
-GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser)+GetConfigOption(const char *name, bool missing_ok, bool restrict_privileged)
{
struct config_generic *record;
static char buffer[256];
@@ -6892,7 +6892,7 @@ GetConfigOption(const char *name, bool missing_ok, bool restrict_superuser)
errmsg("unrecognized configuration parameter \"%s\"",
name)));
}
- if (restrict_superuser &&+ if (restrict_privileged &&
(record->flags & GUC_SUPERUSER_ONLY) &&
!is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_SETTINGS))
ereport(ERROR,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.hindex 3d13a33b94..f462eabe59 100644--- a/src/include/utils/guc.h+++ b/src/include/utils/guc.h@@ -347,7 +347,7 @@ extern void DefineCustomEnumVariable(
extern void EmitWarningsOnPlaceholders(const char *className);
extern const char *GetConfigOption(const char *name, bool missing_ok,
- bool restrict_superuser);+ bool restrict_privileged);
extern const char *GetConfigOptionResetString(const char *name);
extern int GetConfigOptionFlags(const char *name, bool missing_ok);
extern void ProcessConfigFile(GucContext context);
--
2.14.3
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: laurenz.albe@cybertec.at, michael@paquier.xyz, simon@2ndquadrant.com
Subject: Re: SHOW ALL does not honor pg_read_all_settings membership
In-Reply-To: <1524223306.2480.6.camel@cybertec.at>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox