Introduce a psql "\connect -reuse-previous=on|off" option.

nmisch · nmisch · commit dfb2d8039eb7 · 2016-08-08T10:07:51.000-04:00
The decision to reuse values of parameters from a previous connection has been based on whether the new target is a conninfo string. Add this means of overriding that default. This feature arose as one component of a fix for security vulnerabilities in pg_dump, pg_dumpall, and pg_upgrade, so back-patch to 9.1 (all supported versions). In 9.3 and later, comment paragraphs that required update had already-incorrect claims about behavior when no connection is open; fix those problems. Security: CVE-2016-5424
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
@@ -775,7 +775,7 @@ testdb=&gt;
       </varlistentry>
 
       <varlistentry>
-        <term><literal>\c</literal> or <literal>\connect</literal> <literal>[ <replaceable class="parameter">dbname</replaceable> [ <replaceable class="parameter">username</replaceable> ] [ <replaceable class="parameter">host</replaceable> ] [ <replaceable class="parameter">port</replaceable> ] ] | <replaceable class="parameter">conninfo</replaceable> </literal></term>
+        <term><literal>\c</literal> or <literal>\connect [ -reuse-previous=<replaceable class="parameter">on|off</replaceable> ] [ <replaceable class="parameter">dbname</replaceable> [ <replaceable class="parameter">username</replaceable> ] [ <replaceable class="parameter">host</replaceable> ] [ <replaceable class="parameter">port</replaceable> ] | <replaceable class="parameter">conninfo</replaceable> ]</literal></term>
         <listitem>
         <para>
         Establishes a new connection to a <productname>PostgreSQL</>
@@ -785,16 +785,19 @@ testdb=&gt;
         </para>
 
         <para>
-        When using positional parameters, if any of
-        <replaceable class="parameter">dbname</replaceable>,
+        Where the command omits database name, user, host, or port, the new
+        connection can reuse values from the previous connection.  By default,
+        values from the previous connection are reused except when processing
+        a <literal>conninfo</> string.  Passing a first argument
+        of <literal>-reuse-previous=on</>
+        or <literal>-reuse-previous=off</literal> overrides that default.
+        When the command neither specifies nor reuses a particular parameter,
+        the <application>libpq</application> default is used.  Specifying any
+        of <replaceable class="parameter">dbname</replaceable>,
         <replaceable class="parameter">username</replaceable>,
         <replaceable class="parameter">host</replaceable> or
-        <replaceable class="parameter">port</replaceable> are omitted or
-        specified as <literal>-</literal>, the value of that parameter from
-        the previous connection is used; if there is no previous connection,
-        the <application>libpq</application> default for the parameter's value
-        is used.  When using <literal>conninfo</> strings, no values from the
-        previous connection are used for the new connection.
+        <replaceable class="parameter">port</replaceable>
+        as <literal>-</literal> is equivalent to omitting that parameter.
         </para>
 
         <para>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
@@ -59,7 +59,8 @@ static backslashResult exec_command(const char *cmd,
 			 PQExpBuffer query_buf);
 static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
 		int lineno, bool *edited);
-static bool do_connect(char *dbname, char *user, char *host, char *port);
+static bool do_connect(enum trivalue reuse_previous_specification,
+		   char *dbname, char *user, char *host, char *port);
 static bool do_shell(const char *command);
 static bool do_watch(PQExpBuffer query_buf, long sleep);
 static bool lookup_function_oid(PGconn *conn, const char *desc, Oid *foid);
@@ -218,12 +219,9 @@ exec_command(const char *cmd,
 	/*
 	 * \c or \connect -- connect to database using the specified parameters.
 	 *
-	 * \c dbname user host port
+	 * \c [-reuse-previous=BOOL] dbname user host port
 	 *
-	 * If any of these parameters are omitted or specified as '-', the current
-	 * value of the parameter will be used instead. If the parameter has no
-	 * current value, the default value for that parameter will be used. Some
-	 * examples:
+	 * Specifying a parameter as '-' is equivalent to omitting it.  Examples:
 	 *
 	 * \c - - hst		Connect to current database on current port of host
 	 * "hst" as current user. \c - usr - prt   Connect to current database on
@@ -232,17 +230,31 @@ exec_command(const char *cmd,
 	 */
 	else if (strcmp(cmd, "c") == 0 || strcmp(cmd, "connect") == 0)
 	{
+		static const char prefix[] = "-reuse-previous=";
 		char	   *opt1,
 				   *opt2,
 				   *opt3,
 				   *opt4;
+		enum trivalue reuse_previous;
 
 		opt1 = read_connect_arg(scan_state);
+		if (opt1 != NULL && strncmp(opt1, prefix, sizeof(prefix) - 1) == 0)
+		{
+			reuse_previous =
+				ParseVariableBool(opt1 + sizeof(prefix) - 1, prefix) ?
+				TRI_YES : TRI_NO;
+
+			free(opt1);
+			opt1 = read_connect_arg(scan_state);
+		}
+		else
+			reuse_previous = TRI_DEFAULT;
+
 		opt2 = read_connect_arg(scan_state);
 		opt3 = read_connect_arg(scan_state);
 		opt4 = read_connect_arg(scan_state);
 
-		success = do_connect(opt1, opt2, opt3, opt4);
+		success = do_connect(reuse_previous, opt1, opt2, opt3, opt4);
 
 		free(opt1);
 		free(opt2);
@@ -1545,22 +1557,25 @@ param_is_newly_set(const char *old_val, const char *new_val)
 /*
  * do_connect -- handler for \connect
  *
- * Connects to a database with given parameters. If there exists an
- * established connection, NULL values will be replaced with the ones
- * in the current connection. Otherwise NULL will be passed for that
- * parameter to PQconnectdbParams(), so the libpq defaults will be used.
+ * Connects to a database with given parameters. Absent an established
+ * connection, all parameters are required. Given -reuse-previous=off or a
+ * connection string without -reuse-previous=on, NULL values will pass through
+ * to PQconnectdbParams(), so the libpq defaults will be used. Otherwise, NULL
+ * values will be replaced with the ones in the current connection.
  *
  * In interactive mode, if connection fails with the given parameters,
  * the old connection will be kept.
  */
 static bool
-do_connect(char *dbname, char *user, char *host, char *port)
+do_connect(enum trivalue reuse_previous_specification,
+		   char *dbname, char *user, char *host, char *port)
 {
 	PGconn	   *o_conn = pset.db,
 			   *n_conn;
 	char	   *password = NULL;
 	bool		keep_password;
 	bool		has_connection_string;
+	bool		reuse_previous;
 
 	if (!o_conn && (!dbname || !user || !host || !port))
 	{
@@ -1574,17 +1589,36 @@ do_connect(char *dbname, char *user, char *host, char *port)
 		return false;
 	}
 
-	/* grab values from the old connection, unless supplied by caller */
-	if (!user)
+	has_connection_string = dbname ?
+		recognized_connection_string(dbname) : false;
+	switch (reuse_previous_specification)
+	{
+		case TRI_YES:
+			reuse_previous = true;
+			break;
+		case TRI_NO:
+			reuse_previous = false;
+			break;
+		default:
+			reuse_previous = !has_connection_string;
+			break;
+	}
+	/* Silently ignore arguments subsequent to a connection string. */
+	if (has_connection_string)
+	{
+		user = NULL;
+		host = NULL;
+		port = NULL;
+	}
+
+	/* grab missing values from the old connection */
+	if (!user && reuse_previous)
 		user = PQuser(o_conn);
-	if (!host)
+	if (!host && reuse_previous)
 		host = PQhost(o_conn);
-	if (!port)
+	if (!port && reuse_previous)
 		port = PQport(o_conn);
 
-	has_connection_string =
-		dbname ? recognized_connection_string(dbname) : false;
-
 	/*
 	 * Any change in the parameters read above makes us discard the password.
 	 * We also discard it if we're to use a conninfo rather than the
@@ -1601,10 +1635,10 @@ do_connect(char *dbname, char *user, char *host, char *port)
 			(port && PQport(o_conn) && strcmp(port, PQport(o_conn)) == 0);
 
 	/*
-	 * Grab dbname from old connection unless supplied by caller.  No password
-	 * discard if this changes: passwords aren't (usually) database-specific.
+	 * Grab missing dbname from old connection.  No password discard if this
+	 * changes: passwords aren't (usually) database-specific.
 	 */
-	if (!dbname)
+	if (!dbname && reuse_previous)
 		dbname = PQdb(o_conn);
 
 	/*
@@ -1635,20 +1669,27 @@ do_connect(char *dbname, char *user, char *host, char *port)
 #define PARAMS_ARRAY_SIZE	8
 		const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
 		const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
-		int			paramnum = 0;
+		int			paramnum = -1;
 
-		keywords[0] = "dbname";
-		values[0] = dbname;
+		keywords[++paramnum] = "host";
+		values[paramnum] = host;
+		keywords[++paramnum] = "port";
+		values[paramnum] = port;
+		keywords[++paramnum] = "user";
+		values[paramnum] = user;
 
-		if (!has_connection_string)
-		{
-			keywords[++paramnum] = "host";
-			values[paramnum] = host;
-			keywords[++paramnum] = "port";
-			values[paramnum] = port;
-			keywords[++paramnum] = "user";
-			values[paramnum] = user;
-		}
+		/*
+		 * Position in the array matters when the dbname is a connection
+		 * string, because settings in a connection string override earlier
+		 * array entries only.  Thus, user= in the connection string always
+		 * takes effect, but client_encoding= often will not.
+		 *
+		 * If you change this code, also change the initial-connection code in
+		 * main().  For no good reason, a connection string password= takes
+		 * precedence in main() but not here.
+		 */
+		keywords[++paramnum] = "dbname";
+		values[paramnum] = dbname;
 		keywords[++paramnum] = "password";
 		values[paramnum] = password;
 		keywords[++paramnum] = "fallback_application_name";
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
@@ -210,7 +210,7 @@ main(int argc, char *argv[])
 		values[2] = options.username;
 		keywords[3] = "password";
 		values[3] = password;
-		keywords[4] = "dbname";
+		keywords[4] = "dbname"; /* see do_connect() */
 		values[4] = (options.action == ACT_LIST_DB &&
 					 options.dbname == NULL) ?
 			"postgres" : options.dbname;
