From 107e96e12dd25c8360470bd4b65f975925300d86 Mon Sep 17 00:00:00 2001 From: Justin Pryzby 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: |||! + 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