Skip to content

Commit ab35962

Browse files
michaelpqmichaelpq
committed
Fix SHOW ALL command for non-superusers with replication connection
Since Postgres 10, SHOW commands can be triggered with replication connections in a WAL sender context, however it missed that a transaction context is needed for syscache lookups. This commit makes sure that the syscache lookups can happen correctly by setting a transaction context when running SHOW commands in a WAL sender. Superuser-only parameters can be displayed using SHOW commands not only to superusers, but also to members of system role pg_read_all_settings, which requires a syscache lookup to check if the connected role is a member of this system role or not, or the instance crashes. Superusers do not need to check the syscache so it worked correctly in this case. New tests are added to cover this issue. Reported-by: Alexander Kukushkin Author: Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/15734-2daa8761eeed8e20@postgresql.org Backpatch-through: 10
1 parent 4543ef3 commit ab35962

File tree

3 files changed

+59
-3
lines changed

3 files changed

+59
-3
lines changed

‎src/backend/replication/walsender.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,7 +1548,10 @@ exec_replication_command(const char *cmd_string)
15481548
DestReceiver *dest = CreateDestReceiver(DestRemoteSimple);
15491549
VariableShowStmt *n = (VariableShowStmt *) cmd_node;
15501550

1551+
/* syscache access needs a transaction environment */
1552+
StartTransactionCommand();
15511553
GetPGVariable(n->name, dest);
1554+
CommitTransactionCommand();
15521555
}
15531556
break;
15541557

‎src/test/perl/PostgresNode.pm

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,8 @@ sub init
412412

413413
TestLib::system_or_bail('initdb', '-D', $pgdata, '-A', 'trust', '-N',
414414
@{ $params{extra} });
415-
TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
415+
TestLib::system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata,
416+
@{ $params{auth_extra} });
416417

417418
open my $conf, '>>', "$pgdata/postgresql.conf";
418419
print $conf "\n# Added by PostgresNode.pm\n";

‎src/test/recovery/t/001_stream_rep.pl

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,14 @@
33
use warnings;
44
use PostgresNode;
55
use TestLib;
6-
use Test::More tests => 26;
6+
use Test::More tests => 32;
77

88
# Initialize master node
99
my $node_master = get_new_node('master');
10-
$node_master->init(allows_streaming => 1);
10+
# A specific role is created to perform some tests related to replication,
11+
# and it needs proper authentication configuration.
12+
$node_master->init(allows_streaming => 1,
13+
auth_extra => ['--create-role', 'repl_role']);
1114
$node_master->start;
1215
my $backup_name = 'my_backup';
1316

@@ -115,6 +118,55 @@ sub test_target_session_attrs
115118
test_target_session_attrs($node_standby_1, $node_master, $node_standby_1,
116119
"any", 0);
117120

121+
# Test for SHOW commands using a WAL sender connection with a replication
122+
# role.
123+
note "testing SHOW commands for replication connection";
124+
125+
$node_master->psql('postgres',"
126+
CREATE ROLE repl_role REPLICATION LOGIN;
127+
GRANT pg_read_all_settings TO repl_role;");
128+
my $master_host = $node_master->host;
129+
my $master_port = $node_master->port;
130+
my $connstr_common = "host=$master_host port=$master_port user=repl_role";
131+
my $connstr_rep = "$connstr_common replication=1";
132+
my $connstr_db = "$connstr_common replication=database dbname=postgres";
133+
134+
# Test SHOW ALL
135+
my ($ret, $stdout, $stderr) =
136+
$node_master->psql('postgres', 'SHOW ALL;',
137+
on_error_die => 1,
138+
extra_params => [ '-d', $connstr_rep ]);
139+
ok($ret == 0, "SHOW ALL with replication role and physical replication");
140+
($ret, $stdout, $stderr) =
141+
$node_master->psql('postgres', 'SHOW ALL;',
142+
on_error_die => 1,
143+
extra_params => [ '-d', $connstr_db ]);
144+
ok($ret == 0, "SHOW ALL with replication role and logical replication");
145+
146+
# Test SHOW with a user-settable parameter
147+
($ret, $stdout, $stderr) =
148+
$node_master->psql('postgres', 'SHOW work_mem;',
149+
on_error_die => 1,
150+
extra_params => [ '-d', $connstr_rep ]);
151+
ok($ret == 0, "SHOW with user-settable parameter, replication role and physical replication");
152+
($ret, $stdout, $stderr) =
153+
$node_master->psql('postgres', 'SHOW work_mem;',
154+
on_error_die => 1,
155+
extra_params => [ '-d', $connstr_db ]);
156+
ok($ret == 0, "SHOW with user-settable parameter, replication role and logical replication");
157+
158+
# Test SHOW with a superuser-settable parameter
159+
($ret, $stdout, $stderr) =
160+
$node_master->psql('postgres', 'SHOW data_directory;',
161+
on_error_die => 1,
162+
extra_params => [ '-d', $connstr_rep ]);
163+
ok($ret == 0, "SHOW with superuser-settable parameter, replication role and physical replication");
164+
($ret, $stdout, $stderr) =
165+
$node_master->psql('postgres', 'SHOW data_directory;',
166+
on_error_die => 1,
167+
extra_params => [ '-d', $connstr_db ]);
168+
ok($ret == 0, "SHOW with superuser-settable parameter, replication role and logical replication");
169+
118170
note "switching to physical replication slot";
119171

120172
# Switch to using a physical replication slot. We can do this without a new

0 commit comments

Comments
 (0)