Fix connection string handling in src/bin/scripts/ programs.

When told to process all databases, clusterdb, reindexdb, and vacuumdb
would reconnect by replacing their --maintenance-db parameter with the
name of the target database.  If that parameter is a connstring (which
has been allowed for a long time, though we failed to document that
before this patch), we'd lose any other options it might specify, for
example SSL or GSS parameters, possibly resulting in failure to connect.
Thus, this is the same bug as commit a45bc8a4f fixed in pg_dump and
pg_restore.  We can fix it in the same way, by using libpq's rules for
handling multiple "dbname" parameters to add the target database name
separately.  I chose to apply the same refactoring approach as in that
patch, with a struct to handle the command line parameters that need to
be passed through to connectDatabase.  (Maybe someday we can unify the
very similar functions here and in pg_dump/pg_restore.)

Per Peter Eisentraut's comments on bug #16604.  Back-patch to all
supported branches.

Discussion: https://postgr.es/m/16604-933f4b8791227b15@postgresql.org

1 files changed, 53 insertions, 36 deletions

diff --git a/src/bin/scripts/common.c b/src/bin/scripts/common.c
index e987eef2343..3362221a311 100644
--- a/src/bin/scripts/common.c
+++ b/src/bin/scripts/common.c
@@ -54,7 +54,7 @@ handle_help_version_opts(int argc, char *argv[],
  * Make a database connection with the given parameters.
  *
  * An interactive password prompt is automatically issued if needed and
- * allowed by prompt_password.
+ * allowed by cparams->prompt_password.
  *
  * If allow_password_reuse is true, we will try to re-use any password
  * given during previous calls to this routine.  (Callers should not pass
@@ -62,22 +62,23 @@ handle_help_version_opts(int argc, char *argv[],
  * as before, else we might create password exposure hazards.)
  */
 PGconn *
-connectDatabase(const char *dbname, const char *pghost,
-				const char *pgport, const char *pguser,
-				enum trivalue prompt_password, const char *progname,
+connectDatabase(const ConnParams *cparams, const char *progname,
 				bool echo, bool fail_ok, bool allow_password_reuse)
 {
 	PGconn	   *conn;
 	bool		new_pass;
 	static char *password = NULL;
 
+	/* Callers must supply at least dbname; other params can be NULL */
+	Assert(cparams->dbname);
+
 	if (!allow_password_reuse && password)
 	{
 		free(password);
 		password = NULL;
 	}
 
-	if (!password && prompt_password == TRI_YES)
+	if (cparams->prompt_password == TRI_YES && password == NULL)
 		password = simple_prompt("Password: ", false);
 
 	/*
@@ -86,23 +87,35 @@ connectDatabase(const char *dbname, const char *pghost,
 	 */
 	do
 	{
-		const char *keywords[7];
-		const char *values[7];
-
-		keywords[0] = "host";
-		values[0] = pghost;
-		keywords[1] = "port";
-		values[1] = pgport;
-		keywords[2] = "user";
-		values[2] = pguser;
-		keywords[3] = "password";
-		values[3] = password;
-		keywords[4] = "dbname";
-		values[4] = dbname;
-		keywords[5] = "fallback_application_name";
-		values[5] = progname;
-		keywords[6] = NULL;
-		values[6] = NULL;
+		const char *keywords[8];
+		const char *values[8];
+		int			i = 0;
+
+		/*
+		 * If dbname is a connstring, its entries can override the other
+		 * values obtained from cparams; but in turn, override_dbname can
+		 * override the dbname component of it.
+		 */
+		keywords[i] = "host";
+		values[i++] = cparams->pghost;
+		keywords[i] = "port";
+		values[i++] = cparams->pgport;
+		keywords[i] = "user";
+		values[i++] = cparams->pguser;
+		keywords[i] = "password";
+		values[i++] = password;
+		keywords[i] = "dbname";
+		values[i++] = cparams->dbname;
+		if (cparams->override_dbname)
+		{
+			keywords[i] = "dbname";
+			values[i++] = cparams->override_dbname;
+		}
+		keywords[i] = "fallback_application_name";
+		values[i++] = progname;
+		keywords[i] = NULL;
+		values[i++] = NULL;
+		Assert(i <= lengthof(keywords));
 
 		new_pass = false;
 		conn = PQconnectdbParams(keywords, values, true);
@@ -110,7 +123,7 @@ connectDatabase(const char *dbname, const char *pghost,
 		if (!conn)
 		{
 			pg_log_error("could not connect to database %s: out of memory",
-						 dbname);
+						 cparams->dbname);
 			exit(1);
 		}
 
@@ -119,7 +132,7 @@ connectDatabase(const char *dbname, const char *pghost,
 		 */
 		if (PQstatus(conn) == CONNECTION_BAD &&
 			PQconnectionNeedsPassword(conn) &&
-			prompt_password != TRI_NO)
+			cparams->prompt_password != TRI_NO)
 		{
 			PQfinish(conn);
 			if (password)
@@ -138,10 +151,11 @@ connectDatabase(const char *dbname, const char *pghost,
 			return NULL;
 		}
 		pg_log_error("could not connect to database %s: %s",
-					 dbname, PQerrorMessage(conn));
+					 cparams->dbname, PQerrorMessage(conn));
 		exit(1);
 	}
 
+	/* Start strict; callers may override this. */
 	PQclear(executeQuery(conn, ALWAYS_SECURE_SEARCH_PATH_SQL, echo));
 
 	return conn;
@@ -149,27 +163,30 @@ connectDatabase(const char *dbname, const char *pghost,
 
 /*
  * Try to connect to the appropriate maintenance database.
+ *
+ * This differs from connectDatabase only in that it has a rule for
+ * inserting a default "dbname" if none was given (which is why cparams
+ * is not const).  Note that cparams->dbname should typically come from
+ * a --maintenance-db command line parameter.
  */
 PGconn *
-connectMaintenanceDatabase(const char *maintenance_db,
-						   const char *pghost, const char *pgport,
-						   const char *pguser, enum trivalue prompt_password,
+connectMaintenanceDatabase(ConnParams *cparams,
 						   const char *progname, bool echo)
 {
 	PGconn	   *conn;
 
 	/* If a maintenance database name was specified, just connect to it. */
-	if (maintenance_db)
-		return connectDatabase(maintenance_db, pghost, pgport, pguser,
-							   prompt_password, progname, echo, false, false);
+	if (cparams->dbname)
+		return connectDatabase(cparams, progname, echo, false, false);
 
 	/* Otherwise, try postgres first and then template1. */
-	conn = connectDatabase("postgres", pghost, pgport, pguser, prompt_password,
-						   progname, echo, true, false);
+	cparams->dbname = "postgres";
+	conn = connectDatabase(cparams, progname, echo, true, false);
 	if (!conn)
-		conn = connectDatabase("template1", pghost, pgport, pguser,
-							   prompt_password, progname, echo, false, false);
-
+	{
+		cparams->dbname = "template1";
+		conn = connectDatabase(cparams, progname, echo, false, false);
+	}
 	return conn;
 }