From 107e96e12dd25c8360470bd4b65f975925300d86 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 25 May 2022 05:14:43 -0500
Subject: [PATCH 2/2] WIP: test GUC default values in postgresql.conf.sample

This tests for consistency between the default values written in
postgresql.conf.sample and defined in guc.c.

Kyotaro Horiguchi and Justin Pryzby

//-os-only: freebsd, macos
---
 src/backend/utils/misc/guc.c                  | 80 +++++++++++++++
 src/include/catalog/pg_proc.dat               |  5 +
 src/test/modules/test_misc/t/003_check_guc.pl | 99 +++++++++++++++++--
 .../unsafe_tests/expected/rolenames.out       | 14 +++
 .../modules/unsafe_tests/sql/rolenames.sql    |  7 ++
 5 files changed, 199 insertions(+), 6 deletions(-)

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 53d1d9a06a7..9eef85a4fa4 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3239,6 +3239,86 @@ parse_and_validate_value(struct config_generic *record,
 	return true;
 }
 
+/*
+ * Helper for pg_config_unitless_value
+ *
+ * Return the the value of the enum at the given index, or NULL if not found.
+ */
+static const char *
+config_enum_lookup_by_value_soft(struct config_enum *record, int val)
+{
+	const struct config_enum_entry *entry;
+
+	for (entry = record->options; entry && entry->name; entry++)
+	{
+		if (entry->val == val)
+			return entry->name;
+	}
+
+	return NULL;
+}
+
+/*
+ * Convert value to unitless value according to the specified GUC variable
+ */
+Datum
+pg_config_unitless_value(PG_FUNCTION_ARGS)
+{
+	char	   *name;
+	char	   *value;
+	struct config_generic *record;
+	const char *result = "";
+	void	   *extra;
+	union config_var_val val;
+	char		buffer[256];
+
+	name = text_to_cstring(PG_GETARG_TEXT_PP(0));
+	value = text_to_cstring(PG_GETARG_TEXT_PP(1));
+
+	record = find_option(name, true, false, ERROR);
+
+	/*
+	 * This function doesn't reveal values of the variables, but be consistent
+	 * with similar functions.
+	 */
+	if ((record->flags & GUC_SUPERUSER_ONLY) &&
+		!ConfigOptionIsVisible(record))
+		ereport(ERROR,
+				errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				errmsg("permission denied to examine \"%s\"", name),
+				errdetail("Only roles with privileges of the \"%s\" role may examine this parameter.",
+						  "pg_read_all_settings"));
+
+	parse_and_validate_value(record, name, value, PGC_S_TEST, WARNING,
+							 &val, &extra);
+
+	switch (record->vartype)
+	{
+		case PGC_BOOL:
+			result = (val.boolval ? "on" : "off");
+			break;
+		case PGC_INT:
+			snprintf(buffer, sizeof(buffer), "%d", val.intval);
+			result = buffer;
+			break;
+		case PGC_REAL:
+			snprintf(buffer, sizeof(buffer), "%g", val.realval);
+			result = buffer;
+			break;
+		case PGC_STRING:
+			result = val.stringval;
+			break;
+		case PGC_ENUM:
+			result = config_enum_lookup_by_value_soft((struct config_enum *) record,
+												 val.intval);
+			break;
+	}
+
+	if (result == NULL)
+		PG_RETURN_NULL();
+
+	PG_RETURN_TEXT_P(cstring_to_text(result));
+}
 
 /*
  * set_config_option: sets option `name' to given value.
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index aa293c43b6a..fe82629b1f6 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -6242,6 +6242,11 @@
   proname => 'pg_settings_get_flags', provolatile => 's', prorettype => '_text',
   proargtypes => 'text', prosrc => 'pg_settings_get_flags' },
 
+{ oid => '9956', descr => 'normalize value to the unit of specified GUC',
+  proname => 'pg_config_unitless_value', provolatile => 's',
+  prorettype => 'text', proargtypes => 'text text',
+  proargnames => '{varname,value}', prosrc => 'pg_config_unitless_value' },
+
 { oid => '3329', descr => 'show config file settings',
   proname => 'pg_show_all_file_settings', prorows => '1000', proretset => 't',
   provolatile => 'v', prorettype => 'record', proargtypes => '',
diff --git a/src/test/modules/test_misc/t/003_check_guc.pl b/src/test/modules/test_misc/t/003_check_guc.pl
index e9f33f3c775..ce2ec1bf9ba 100644
--- a/src/test/modules/test_misc/t/003_check_guc.pl
+++ b/src/test/modules/test_misc/t/003_check_guc.pl
@@ -11,6 +11,26 @@ my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
 $node->start;
 
+# These are non-variables but that are mistakenly parsed as variable
+# settings in the loop below.
+my %skip_names =
+  map { $_ => 1 } ('include', 'include_dir', 'include_if_exists');
+
+# The following parameters have defaults which are
+# environment-dependent and may not match the default
+# values written in the sample config file.
+my %ignore_parameters =
+  map { $_ => 1 } (
+	  'data_directory',
+	  'hba_file',
+	  'ident_file',
+	  'krb_server_keyfile',
+	  'timezone_abbreviations',
+	  'lc_messages',
+	  'max_stack_depth', # XXX
+	  'wal_buffers', # XXX
+	  );
+
 # Grab the names of all the parameters that can be listed in the
 # configuration sample file.  config_file is an exception, it is not
 # in postgresql.conf.sample but is part of the lists from guc_tables.c.
@@ -42,7 +62,7 @@ my @gucs_in_file;
 
 # Read the sample file line-by-line, checking its contents to build a list
 # of everything known as a GUC.
-my $num_tests = 0;
+my @file_vals = ();
 open(my $contents, '<', $sample_file)
   || die "Could not open $sample_file: $!";
 while (my $line = <$contents>)
@@ -52,19 +72,28 @@ while (my $line = <$contents>)
 	# file.
 	# - Valid configuration options are followed immediately by " = ",
 	# with one space before and after the equal sign.
-	if ($line =~ m/^#?([_[:alpha:]]+) = .*/)
+	if ($line =~ m/^#?([_[:alpha:]]+) = (.*)$/)
 	{
 		# Lower-case conversion matters for some of the GUCs.
 		my $param_name = lc($1);
 
-		# Ignore some exceptions.
-		next if $param_name eq "include";
-		next if $param_name eq "include_dir";
-		next if $param_name eq "include_if_exists";
+		# extract value
+		my $file_value = $2;
+		$file_value =~ s/\s*#.*$//;		# strip trailing comment
+		$file_value =~ s/^'(.*)'$/$1/;	# strip quotes
+
+		next if (defined $skip_names{$param_name});
 
 		# Update the list of GUCs found in the sample file, for the
 		# follow-up tests.
 		push @gucs_in_file, $param_name;
+
+		# Update the list of GUCs whose value is checked for consistency
+		# between the sample file and pg_setting.boot_val
+		if (!defined $ignore_parameters{$param_name})
+		{
+			push(@file_vals, [$param_name, $file_value]);
+		}
 	}
 }
 
@@ -106,4 +135,62 @@ foreach my $param (@sample_intersect)
 	);
 }
 
+# Test that GUCs in postgresql.conf.sample show the correct default values
+my $check_defaults = $node->safe_psql(
+	'postgres',
+	"
+	CREATE TABLE sample_conf AS
+	SELECT m[1] AS name, COALESCE(m[3], m[5]) AS sample_value
+	FROM (SELECT regexp_split_to_table(pg_read_file('$sample_file'), '\n') AS ln) conf,
+	regexp_match(ln, '^#?([_[:alpha:]]+) (= ''([^'']*)''|(= ([^[:space:]]*))).*') AS m
+	WHERE ln ~ '^#?[[:alpha:]]'
+	");
+
+$check_defaults = $node->safe_psql(
+	'postgres',
+	"
+	SELECT name, sc.sample_value, boot_val
+	FROM pg_settings ps
+	JOIN sample_conf sc USING(name)
+        JOIN pg_settings_get_flags(ps.name) AS flags ON true
+	WHERE boot_val != sc.sample_value -- same value
+	AND boot_val != pg_config_unitless_value(name, sc.sample_value) -- same value with different units
+	AND 'DEFAULT_COMPILE' != ALL(flags) -- dynamically-set defaults
+	AND 'DEFAULT_INITDB' != ALL(flags) -- dynamically-set defaults
+	AND name NOT IN ('max_stack_depth', 'krb_server_keyfile'); -- exceptions
+	");
+
+my @check_defaults_array = split("\n", lc($check_defaults));
+
+is (@check_defaults_array, 0,
+	"check for consistency of defaults in postgresql.conf.sample: $check_defaults");
+
+# XXX An alternative, perl implementation of the same thing:
+
+# Check if GUC values in config-file and boot value match
+my $values = $node->safe_psql(
+	'postgres',
+	'SELECT f.n, pg_config_unitless_value(f.n, f.v), s.boot_val, \'!\' '.
+	'FROM (VALUES '.
+	join(',', map { "('${$_}[0]','${$_}[1]')" } @file_vals).
+	') f(n,v) '.
+	"JOIN pg_settings s ON (s.name = f.n)".
+	"JOIN pg_settings_get_flags(s.name) AS flags ON true ".
+	"AND 'DEFAULT_COMPILE' != ALL(flags) -- dynamically-set defaults ".
+	"AND 'DEFAULT_INITDB' != ALL(flags) -- dynamically-set defaults");
+
+my $fails = "";
+foreach my $l (split("\n", $values))
+{
+	# $l: <varname>|<fileval>|<boot_val>|!
+	my @t = split("\\|", $l);
+	if ($t[1] ne $t[2])
+	{
+		$fails .= "\n" if ($fails ne "");
+		$fails .= "$t[0]: file \"$t[1]\" != boot_val \"$t[2]\"";
+	}
+}
+
+is($fails, "", "check if GUC values in .sample and boot value match");
+
 done_testing();
diff --git a/src/test/modules/unsafe_tests/expected/rolenames.out b/src/test/modules/unsafe_tests/expected/rolenames.out
index 61396b2a805..1bba9c87239 100644
--- a/src/test/modules/unsafe_tests/expected/rolenames.out
+++ b/src/test/modules/unsafe_tests/expected/rolenames.out
@@ -1082,6 +1082,20 @@ DETAIL:  Only roles with privileges of the "pg_read_all_settings" role may exami
 RESET SESSION AUTHORIZATION;
 ERROR:  current transaction is aborted, commands ignored until end of transaction block
 ROLLBACK;
+BEGIN;
+SET SESSION AUTHORIZATION regress_role_haspriv;
+-- passes with role member of pg_read_all_settings
+SELECT pg_config_unitless_value('session_preload_libraries', 'val');
+ pg_config_unitless_value 
+--------------------------
+ val
+(1 row)
+
+SET SESSION AUTHORIZATION regress_role_nopriv;
+SELECT pg_config_unitless_value('session_preload_libraries', 'val');
+ERROR:  permission denied to examine "session_preload_libraries"
+DETAIL:  Only roles with privileges of the "pg_read_all_settings" role may examine this parameter.
+ROLLBACK;
 REVOKE pg_read_all_settings FROM regress_role_haspriv;
 -- clean up
 \c
diff --git a/src/test/modules/unsafe_tests/sql/rolenames.sql b/src/test/modules/unsafe_tests/sql/rolenames.sql
index adac36536db..355aa32c2ac 100644
--- a/src/test/modules/unsafe_tests/sql/rolenames.sql
+++ b/src/test/modules/unsafe_tests/sql/rolenames.sql
@@ -492,6 +492,13 @@ SET SESSION AUTHORIZATION regress_role_nopriv;
 SHOW session_preload_libraries;
 RESET SESSION AUTHORIZATION;
 ROLLBACK;
+BEGIN;
+SET SESSION AUTHORIZATION regress_role_haspriv;
+-- passes with role member of pg_read_all_settings
+SELECT pg_config_unitless_value('session_preload_libraries', 'val');
+SET SESSION AUTHORIZATION regress_role_nopriv;
+SELECT pg_config_unitless_value('session_preload_libraries', 'val');
+ROLLBACK;
 REVOKE pg_read_all_settings FROM regress_role_haspriv;
 
 -- clean up
-- 
2.34.1

