Re: BUG #15025: PSQL CLI - inconsistency when both -d and -U supplies a username

Bruce Momjian <bruce@momjian.us> writes:
> On Sun, Jan 28, 2018 at 02:38:46PM -0500, Tom Lane wrote:
>> Isn't it possible to get the URI parse
>> results back out of libpq?

> Well, there is PQuser(), but you need to pass a connection struct to
> that, and before you connect you don't have one.

Yeah, but we normally don't prompt for password till after a failed
connection attempt, at which point we can get the info.  So I propose
something like the attached.

There's room for debate about what we ought to do when -W (--password) is
specified, but I think that that's not really that exciting because the
only real use-cases for it are noninteractive applications that aren't
going to care what the prompt is.  So in the startup.c case I have it
just offering the neutral "Password: " prompt always.  In the \c case,
I left it using the same initial username as it was before, because the
odds that that's right seem considerably higher with \c.  You can still
fool it by giving a URI dbname to \c, so maybe there's an argument for
lobotomizing the initial prompt in \c too, but I didn't do that here.

            regards, tom lane

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 015c391..63a6f99 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -2829,7 +2829,7 @@ prompt_for_password(const char *username)
 {
     char        buf[100];

-    if (username == NULL)
+    if (username == NULL || username[0] == '\0')
         simple_prompt("Password: ", buf, sizeof(buf), false);
     else
     {
@@ -2957,6 +2957,11 @@ do_connect(enum trivalue reuse_previous_specification,
      * XXX: this behavior leads to spurious connection attempts recorded in
      * the postmaster's log.  But libpq offers no API that would let us obtain
      * a password and then continue with the first connection attempt.
+     *
+     * XXX: prompting with "user" might be the wrong thing, if the dbname is a
+     * connstring or URI that overrides that.  But getPassword == TRI_YES is a
+     * seldom-used option, so it doesn't seem worth sweating over.  The normal
+     * prompting path below gets it right.
      */
     if (pset.getPassword == TRI_YES)
     {
@@ -3026,8 +3031,12 @@ do_connect(enum trivalue reuse_previous_specification,
          */
         if (!password && PQconnectionNeedsPassword(n_conn) && pset.getPassword != TRI_NO)
         {
+            /*
+             * Prompt for password using the username we actually connected
+             * with --- it might've come out of "dbname" rather than "user".
+             */
+            password = prompt_for_password(PQuser(n_conn));
             PQfinish(n_conn);
-            password = prompt_for_password(user);
             continue;
         }

diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index ec6ae45..be57574 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -101,7 +101,6 @@ main(int argc, char *argv[])
     int            successResult;
     bool        have_password = false;
     char        password[100];
-    char       *password_prompt = NULL;
     bool        new_pass;

     set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
@@ -205,15 +204,14 @@ main(int argc, char *argv[])
         pset.popt.topt.recordSep.separator_zero = false;
     }

-    if (options.username == NULL)
-        password_prompt = pg_strdup(_("Password: "));
-    else
-        password_prompt = psprintf(_("Password for user %s: "),
-                                   options.username);
-
     if (pset.getPassword == TRI_YES)
     {
-        simple_prompt(password_prompt, password, sizeof(password), false);
+        /*
+         * We can't be sure yet of the username that will be used, so don't
+         * offer a potentially wrong one.  Typical uses of this option are
+         * noninteractive anyway.
+         */
+        simple_prompt("Password: ", password, sizeof(password), false);
         have_password = true;
     }

@@ -252,15 +250,28 @@ main(int argc, char *argv[])
             !have_password &&
             pset.getPassword != TRI_NO)
         {
+            /*
+             * Before closing the old PGconn, extract the user name that was
+             * actually connected with --- it might've come out of a URI or
+             * connstring "database name" rather than options.username.
+             */
+            const char *realusername = PQuser(pset.db);
+            char       *password_prompt;
+
+            if (realusername && realusername[0])
+                password_prompt = psprintf(_("Password for user %s: "),
+                                           realusername);
+            else
+                password_prompt = pg_strdup(_("Password: "));
             PQfinish(pset.db);
+
             simple_prompt(password_prompt, password, sizeof(password), false);
+            free(password_prompt);
             have_password = true;
             new_pass = true;
         }
     } while (new_pass);

-    free(password_prompt);
-
     if (PQstatus(pset.db) == CONNECTION_BAD)
     {
         fprintf(stderr, "%s: %s", pset.progname, PQerrorMessage(pset.db));