Fix recently-introduced breakage in psql's \connect command.

Through my misreading of what the existing code actually did,
commits 85c54287a et al. broke psql's behavior for the case where
"\c connstring" provides a password in the connstring.  We should
use that password in such a case, but as of 85c54287a we ignored it
(and instead, prompted for a password).

Commit 94929f1cf fixed that in HEAD, but since I thought it was
cleaning up a longstanding misbehavior and not one I'd just created,
I didn't back-patch it.

Hence, back-patch the portions of 94929f1cf having to do with
password management.  In addition to fixing the introduced bug,
this means that "\c -reuse-previous=on connstring" will allow
re-use of an existing connection's password if the connstring
doesn't change user/host/port.  That didn't happen before, but
it seems like a bug fix, and anyway I'm loath to have significant
differences in this code across versions.

Also fix an error with the same root cause about whether or not to
override a connstring's setting of client_encoding.  As of 85c54287a
we always did so; restore the previous behavior of overriding only
when stdin/stdout are a terminal and there's no environment setting
of PGCLIENTENCODING.  (I find that definition a bit surprising, but
right now doesn't seem like the time to revisit it.)

Per bug #16746 from Krzysztof Gradek.  As with the previous patch,
back-patch to all supported branches.

Discussion: https://postgr.es/m/16746-44b30e2edf4335d4@postgresql.org

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index b26de20f5dd282ed17aec7a7a4956ad99f22a864..3509abc0f761f8a59866f02d6545d2348df5c9af 100644 (file)
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -844,6 +844,8 @@ testdb=>
         is changed from its previous value using the positional syntax,
         any <replaceable>hostaddr</replaceable> setting present in the
         existing connection's parameters is dropped.
+        Also, any password used for the existing connection will be re-used
+        only if the user, host, and port settings are not changed.
         When the command neither specifies nor reuses a particular parameter,
         the <application>libpq</application> default is used.
         </para>

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 2c5ad9e3052775657e641211a8c962b40f8ecdd2..4fcdc0b5ac4241f3a1cced2d439c1e51048bf06a 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1636,6 +1636,7 @@ do_connect(enum trivalue reuse_previous_specification,
        int                     nconnopts = 0;
        bool            same_host = false;
        char       *password = NULL;
+       char       *client_encoding;
        bool            success = true;
        bool            keep_password = true;
        bool            has_connection_string;
@@ -1706,6 +1707,7 @@ do_connect(enum trivalue reuse_previous_specification,
                        {
                                PQconninfoOption *ci;
                                PQconninfoOption *replci;
+                               bool            have_password = false;
 
                                for (ci = cinfo, replci = replcinfo;
                                         ci->keyword && replci->keyword;
@@ -1724,6 +1726,26 @@ do_connect(enum trivalue reuse_previous_specification,
 
                                                replci->val = ci->val;
                                                ci->val = swap;
+
+                                               /*
+                                                * Check whether connstring provides options affecting
+                                                * password re-use.  While any change in user, host,
+                                                * hostaddr, or port causes us to ignore the old
+                                                * connection's password, we don't force that for
+                                                * dbname, since passwords aren't database-specific.
+                                                */
+                                               if (replci->val == NULL ||
+                                                       strcmp(ci->val, replci->val) != 0)
+                                               {
+                                                       if (strcmp(replci->keyword, "user") == 0 ||
+                                                               strcmp(replci->keyword, "host") == 0 ||
+                                                               strcmp(replci->keyword, "hostaddr") == 0 ||
+                                                               strcmp(replci->keyword, "port") == 0)
+                                                               keep_password = false;
+                                               }
+                                               /* Also note whether connstring contains a password. */
+                                               if (strcmp(replci->keyword, "password") == 0)
+                                                       have_password = true;
                                        }
                                }
                                Assert(ci->keyword == NULL && replci->keyword == NULL);
@@ -1733,8 +1755,13 @@ do_connect(enum trivalue reuse_previous_specification,
 
                                PQconninfoFree(replcinfo);
 
-                               /* We never re-use a password with a conninfo string. */
-                               keep_password = false;
+                               /*
+                                * If the connstring contains a password, tell the loop below
+                                * that we may use it, regardless of other settings (i.e.,
+                                * cinfo's password is no longer an "old" password).
+                                */
+                               if (have_password)
+                                       keep_password = true;
 
                                /* Don't let code below try to inject dbname into params. */
                                dbname = NULL;
@@ -1815,14 +1842,16 @@ do_connect(enum trivalue reuse_previous_specification,
        {
                password = prompt_for_password(user);
        }
-       else if (o_conn && keep_password)
-       {
-               password = PQpass(o_conn);
-               if (password && *password)
-                       password = pg_strdup(password);
-               else
-                       password = NULL;
-       }
+
+       /*
+        * Consider whether to force client_encoding to "auto" (overriding
+        * anything in the connection string).  We do so if we have a terminal
+        * connection and there is no PGCLIENTENCODING environment setting.
+        */
+       if (pset.notty || getenv("PGCLIENTENCODING"))
+               client_encoding = NULL;
+       else
+               client_encoding = "auto";
 
        /* Loop till we have a connection or fail, which we might've already */
        while (success)
@@ -1834,12 +1863,12 @@ do_connect(enum trivalue reuse_previous_specification,
 
                /*
                 * Copy non-default settings into the PQconnectdbParams parameter
-                * arrays; but override any values specified old-style, as well as the
-                * password and a couple of fields we want to set forcibly.
+                * arrays; but inject any values specified old-style, as well as any
+                * interactively-obtained password, and a couple of fields we want to
+                * set forcibly.
                 *
                 * If you change this code, see also the initial-connection code in
-                * main().  For no good reason, a connection string password= takes
-                * precedence in main() but not here.
+                * main().
                 */
                for (ci = cinfo; ci->keyword; ci++)
                {
@@ -1858,12 +1887,15 @@ do_connect(enum trivalue reuse_previous_specification,
                        }
                        else if (port && strcmp(ci->keyword, "port") == 0)
                                values[paramnum++] = port;
-                       else if (strcmp(ci->keyword, "password") == 0)
+                       /* If !keep_password, we unconditionally drop old password */
+                       else if ((password || !keep_password) &&
+                                        strcmp(ci->keyword, "password") == 0)
                                values[paramnum++] = password;
                        else if (strcmp(ci->keyword, "fallback_application_name") == 0)
                                values[paramnum++] = pset.progname;
-                       else if (strcmp(ci->keyword, "client_encoding") == 0)
-                               values[paramnum++] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
+                       else if (client_encoding &&
+                                        strcmp(ci->keyword, "client_encoding") == 0)
+                               values[paramnum++] = client_encoding;
                        else if (ci->val)
                                values[paramnum++] = ci->val;
                        /* else, don't bother making libpq parse this keyword */