From 54d60bbd075a61c7dd2ef384dc930d726d68ee64 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 3 Oct 2009 18:04:57 +0000
Subject: [PATCH] Fix a couple of issues in recent patch to print updates to
postgresql.conf settings: avoid calling superuser() in contexts where it's
not defined, don't leak the transient copies of GetConfigOption output, and
avoid the whole exercise in postmaster child processes.
I found that actually no current caller of GetConfigOption has any use for
its internal check of GUC_SUPERUSER_ONLY. But rather than just remove
that entirely, it seemed better to add a parameter indicating whether to
enforce the check.
Per report from Simon and subsequent testing.
---
src/backend/utils/misc/guc-file.l | 19 ++++++++++++-------
src/backend/utils/misc/guc.c | 12 +++++++++---
src/include/utils/guc.h | 4 ++--
src/timezone/pgtz.c | 6 +++---
4 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index a425cd48ac0..424caea13f5 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -4,7 +4,7 @@
*
* Copyright (c) 2000-2009, PostgreSQL Global Development Group
*
- * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.61 2009/09/17 21:15:18 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/guc-file.l,v 1.62 2009/10/03 18:04:57 tgl Exp $
*/
%{
@@ -312,21 +312,26 @@ ProcessConfigFile(GucContext context)
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
- char *pre_value = NULL;
+ char *pre_value = NULL;
- if (context == PGC_SIGHUP)
- pre_value = pstrdup(GetConfigOption(item->name));
+ /* In SIGHUP cases in the postmaster, report changes */
+ if (context == PGC_SIGHUP && !IsUnderPostmaster)
+ pre_value = pstrdup(GetConfigOption(item->name, false));
if (set_config_option(item->name, item->value, context,
PGC_S_FILE, GUC_ACTION_SET, true))
{
- if (pre_value && strcmp(pre_value, GetConfigOption(item->name)) != 0)
+ set_config_sourcefile(item->name, item->filename,
+ item->sourceline);
+ if (pre_value &&
+ strcmp(pre_value, GetConfigOption(item->name, false)) != 0)
ereport(elevel,
(errmsg("parameter \"%s\" changed to \"%s\"",
item->name, item->value)));
- set_config_sourcefile(item->name, item->filename,
- item->sourceline);
}
+
+ if (pre_value)
+ pfree(pre_value);
}
/* Remember when we last successfully loaded the config file. */
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index cb743bb55ef..4d5aeee85ce 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
* Written by Peter Eisentraut <peter_e@gmx.net>.
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.519 2009/09/22 23:43:38 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.520 2009/10/03 18:04:57 tgl Exp $
*
*--------------------------------------------------------------------
*/
@@ -5197,11 +5197,15 @@ SetConfigOption(const char *name, const char *value,
* Fetch the current value of the option `name'. If the option doesn't exist,
* 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.
+ *
* 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)
+GetConfigOption(const char *name, bool restrict_superuser)
{
struct config_generic *record;
static char buffer[256];
@@ -5211,7 +5215,9 @@ GetConfigOption(const char *name)
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("unrecognized configuration parameter \"%s\"", name)));
- if ((record->flags & GUC_SUPERUSER_ONLY) && !superuser())
+ if (restrict_superuser &&
+ (record->flags & GUC_SUPERUSER_ONLY) &&
+ !superuser())
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("must be superuser to examine \"%s\"", name)));
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 435a722aa0b..ef93d0f27cd 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -7,7 +7,7 @@
* Copyright (c) 2000-2009, PostgreSQL Global Development Group
* Written by Peter Eisentraut <peter_e@gmx.net>.
*
- * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.105 2009/09/22 23:43:41 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/utils/guc.h,v 1.106 2009/10/03 18:04:57 tgl Exp $
*--------------------------------------------------------------------
*/
#ifndef GUC_H
@@ -249,7 +249,7 @@ extern void DefineCustomEnumVariable(
extern void EmitWarningsOnPlaceholders(const char *className);
-extern const char *GetConfigOption(const char *name);
+extern const char *GetConfigOption(const char *name, bool restrict_superuser);
extern const char *GetConfigOptionResetString(const char *name);
extern void ProcessConfigFile(GucContext context);
extern void InitializeGUCOptions(void);
diff --git a/src/timezone/pgtz.c b/src/timezone/pgtz.c
index 82a02438780..ea26bbd3d39 100644
--- a/src/timezone/pgtz.c
+++ b/src/timezone/pgtz.c
@@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/timezone/pgtz.c,v 1.63 2009/06/11 14:49:15 momjian Exp $
+ * $PostgreSQL: pgsql/src/timezone/pgtz.c,v 1.64 2009/10/03 18:04:57 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -1366,7 +1366,7 @@ pg_timezone_initialize(void)
pg_tz *def_tz = NULL;
/* Do we need to try to figure the session timezone? */
- if (pg_strcasecmp(GetConfigOption("timezone"), "UNKNOWN") == 0)
+ if (pg_strcasecmp(GetConfigOption("timezone", false), "UNKNOWN") == 0)
{
/* Select setting */
def_tz = select_default_timezone();
@@ -1377,7 +1377,7 @@ pg_timezone_initialize(void)
}
/* What about the log timezone? */
- if (pg_strcasecmp(GetConfigOption("log_timezone"), "UNKNOWN") == 0)
+ if (pg_strcasecmp(GetConfigOption("log_timezone", false), "UNKNOWN") == 0)
{
/* Select setting, but don't duplicate work */
if (!def_tz)
--
GitLab