0001-pg_createsubscriber-Fix-cases-which-connection-param.patch

From be447a10990fcd235e8d6361b82ba3a41a4caf0f Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Date: Mon, 24 Jun 2024 03:05:12 +0000
Subject: [PATCH 1/2] pg_createsubscriber: Fix cases which connection
 parameters contain a space

appendPQExpBuffer() is used to construct a connection string, but we missed the
case when parameters contain a space. To support such cases, appendConnStrVal()
is used to quote strings appropriately. A test code is also updated.
---
 src/bin/pg_basebackup/pg_createsubscriber.c   |  13 +-
 .../t/040_pg_createsubscriber.pl              | 111 ++++++++++--------
 2 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 1138c20e56..8ed10f010b 100644
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -26,6 +26,7 @@
 #include "common/restricted_token.h"
 #include "fe_utils/recovery_gen.h"
 #include "fe_utils/simple_list.h"
+#include "fe_utils/string_utils.h"
 #include "getopt_long.h"
 
 #define	DEFAULT_SUB_PORT	"50432"
@@ -307,10 +308,14 @@ get_sub_conninfo(const struct CreateSubscriberOptions *opt)
 
 	appendPQExpBuffer(buf, "port=%s", opt->sub_port);
 #if !defined(WIN32)
-	appendPQExpBuffer(buf, " host=%s", opt->socket_dir);
+	appendPQExpBuffer(buf, " host=");
+	appendConnStrVal(buf, opt->socket_dir);
 #endif
 	if (opt->sub_username != NULL)
-		appendPQExpBuffer(buf, " user=%s", opt->sub_username);
+	{
+		appendPQExpBuffer(buf, " user=");
+		appendConnStrVal(buf, opt->sub_username);
+	}
 	appendPQExpBuffer(buf, " fallback_application_name=%s", progname);
 
 	ret = pg_strdup(buf->data);
@@ -401,8 +406,8 @@ concat_conninfo_dbname(const char *conninfo, const char *dbname)
 
 	Assert(conninfo != NULL);
 
-	appendPQExpBufferStr(buf, conninfo);
-	appendPQExpBuffer(buf, " dbname=%s", dbname);
+	appendPQExpBuffer(buf, "%s dbname=", conninfo);
+	appendConnStrVal(buf, dbname);
 
 	ret = pg_strdup(buf->data);
 	destroyPQExpBuffer(buf);
diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
index 0516d4e17e..a50771dbdf 100644
--- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
+++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl
@@ -9,6 +9,27 @@ use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
 
+# Generate a database with a name made of a range of ASCII characters.
+# Mostly Ported from 002_pg_upgrade.pl, but this returns a generated dbname.
+sub generate_db
+{
+	my ($node, $prefix, $from_char, $to_char, $suffix) = @_;
+
+	my $dbname = $prefix;
+	for my $i ($from_char .. $to_char)
+	{
+		next if $i == 7 || $i == 10 || $i == 13;    # skip BEL, LF, and CR
+		$dbname = $dbname . sprintf('%c', $i);
+	}
+
+	$dbname .= $suffix;
+	$node->command_ok(
+		[ 'createdb', $dbname ],
+		"created database with ASCII characters from $from_char to $to_char");
+
+	return $dbname;
+}
+
 program_help_ok('pg_createsubscriber');
 program_version_ok('pg_createsubscriber');
 program_options_handling_ok('pg_createsubscriber');
@@ -104,16 +125,14 @@ $node_f->init(force_initdb => 1, allows_streaming => 'logical');
 # - create test tables
 # - insert a row
 # - create a physical replication slot
-$node_p->safe_psql(
-	'postgres', q(
-	CREATE DATABASE pg1;
-	CREATE DATABASE pg2;
-));
-$node_p->safe_psql('pg1', 'CREATE TABLE tbl1 (a text)');
-$node_p->safe_psql('pg1', "INSERT INTO tbl1 VALUES('first row')");
-$node_p->safe_psql('pg2', 'CREATE TABLE tbl2 (a text)');
+my $db1 = generate_db($node_p, 'regression\\"\\', 1, 45, '\\\\"\\\\\\');
+my $db2 = generate_db($node_p, 'regression', 46, 90, '');
+
+$node_p->safe_psql($db1, 'CREATE TABLE tbl1 (a text)');
+$node_p->safe_psql($db1, "INSERT INTO tbl1 VALUES('first row')");
+$node_p->safe_psql($db2, 'CREATE TABLE tbl2 (a text)');
 my $slotname = 'physical_slot';
-$node_p->safe_psql('pg2',
+$node_p->safe_psql($db2,
 	"SELECT pg_create_physical_replication_slot('$slotname')");
 
 # Set up node S as standby linking to node P
@@ -143,11 +162,11 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_t->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_t->host, '--subscriber-port',
 		$node_t->port, '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'target server is not in recovery');
 
@@ -157,11 +176,11 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_s->host, '--subscriber-port',
 		$node_s->port, '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'standby is up and running');
 
@@ -170,11 +189,11 @@ command_fails(
 	[
 		'pg_createsubscriber', '--verbose',
 		'--pgdata', $node_f->data_dir,
-		'--publisher-server', $node_p->connstr('pg1'),
+		'--publisher-server', $node_p->connstr($db1),
 		'--socket-directory', $node_f->host,
 		'--subscriber-port', $node_f->port,
-		'--database', 'pg1',
-		'--database', 'pg2'
+		'--database', $db1,
+		'--database', $db2
 	],
 	'subscriber data directory is not a copy of the source database cluster');
 
@@ -191,16 +210,16 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_c->data_dir, '--publisher-server',
-		$node_s->connstr('pg1'), '--socket-directory',
+		$node_s->connstr($db1), '--socket-directory',
 		$node_c->host, '--subscriber-port',
 		$node_c->port, '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'primary server is in recovery');
 
 # Insert another row on node P and wait node S to catch up
-$node_p->safe_psql('pg1', "INSERT INTO tbl1 VALUES('second row')");
+$node_p->safe_psql($db1, "INSERT INTO tbl1 VALUES('second row')");
 $node_p->wait_for_replay_catchup($node_s);
 
 # Check some unmet conditions on node P
@@ -218,11 +237,11 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_s->host, '--subscriber-port',
 		$node_s->port, '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'primary contains unmet conditions on node P');
 # Restore default settings here but only apply it after testing standby. Some
@@ -247,11 +266,11 @@ command_fails(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_s->host, '--subscriber-port',
 		$node_s->port, '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'standby contains unmet conditions on node S');
 $node_s->append_conf(
@@ -265,7 +284,7 @@ $node_p->restart;
 
 # Create failover slot to test its removal
 my $fslotname = 'failover_slot';
-$node_p->safe_psql('pg1',
+$node_p->safe_psql($db1,
 	"SELECT pg_create_logical_replication_slot('$fslotname', 'pgoutput', false, false, true)");
 $node_s->start;
 $node_s->safe_psql('postgres', "SELECT pg_sync_replication_slots()");
@@ -280,15 +299,15 @@ command_ok(
 		'--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_s->host, '--subscriber-port',
 		$node_s->port, '--publication',
 		'pub1', '--publication',
 		'pub2', '--subscription',
 		'sub1', '--subscription',
 		'sub2', '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'run pg_createsubscriber --dry-run on node S');
 
@@ -304,7 +323,7 @@ command_ok(
 		'pg_createsubscriber', '--verbose',
 		'--dry-run', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_s->host, '--subscriber-port',
 		$node_s->port, '--replication-slot',
 		'replslot1'
@@ -318,20 +337,20 @@ command_ok(
 		'--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default",
 		'--verbose', '--pgdata',
 		$node_s->data_dir, '--publisher-server',
-		$node_p->connstr('pg1'), '--socket-directory',
+		$node_p->connstr($db1), '--socket-directory',
 		$node_s->host, '--subscriber-port',
 		$node_s->port, '--publication',
 		'pub1', '--publication',
 		'Pub2', '--replication-slot',
 		'replslot1', '--replication-slot',
 		'replslot2', '--database',
-		'pg1', '--database',
-		'pg2'
+		$db1, '--database',
+		$db2
 	],
 	'run pg_createsubscriber on node S');
 
 # Confirm the physical replication slot has been removed
-$result = $node_p->safe_psql('pg1',
+$result = $node_p->safe_psql($db1,
 	"SELECT count(*) FROM pg_replication_slots WHERE slot_name = '$slotname'"
 );
 is($result, qq(0),
@@ -339,8 +358,8 @@ is($result, qq(0),
 );
 
 # Insert rows on P
-$node_p->safe_psql('pg1', "INSERT INTO tbl1 VALUES('third row')");
-$node_p->safe_psql('pg2', "INSERT INTO tbl2 VALUES('row 1')");
+$node_p->safe_psql($db1, "INSERT INTO tbl1 VALUES('third row')");
+$node_p->safe_psql($db2, "INSERT INTO tbl2 VALUES('row 1')");
 
 # Start subscriber
 $node_s->start;
@@ -357,20 +376,20 @@ $node_s->wait_for_subscription_sync($node_p, $subnames[0]);
 $node_s->wait_for_subscription_sync($node_p, $subnames[1]);
 
 # Confirm the failover slot has been removed
-$result = $node_s->safe_psql('pg1',
+$result = $node_s->safe_psql($db1,
 	"SELECT count(*) FROM pg_replication_slots WHERE slot_name = '$fslotname'");
 is($result, qq(0), 'failover slot was removed');
 
-# Check result on database pg1
-$result = $node_s->safe_psql('pg1', 'SELECT * FROM tbl1');
+# Check result on database $db1
+$result = $node_s->safe_psql($db1, 'SELECT * FROM tbl1');
 is( $result, qq(first row
 second row
 third row),
-	'logical replication works on database pg1');
+	'logical replication works on database $db1');
 
-# Check result on database pg2
-$result = $node_s->safe_psql('pg2', 'SELECT * FROM tbl2');
-is($result, qq(row 1), 'logical replication works on database pg2');
+# Check result on database $db2
+$result = $node_s->safe_psql($db2, 'SELECT * FROM tbl2');
+is($result, qq(row 1), 'logical replication works on database $db2');
 
 # Different system identifier?
 my $sysid_p = $node_p->safe_psql('postgres',
-- 
2.43.0