psql: fix \connect with URIs and conninfo strings

psql was already accepting conninfo strings as the first parameter in
\connect, but the way it worked wasn't sane; some of the other
parameters would get the previous connection's values, causing it to
connect to a completely unexpected server or, more likely, not finding
any server at all because of completely wrong combinations of
parameters.

Fix by explicitely checking for a conninfo-looking parameter in the
dbname position; if one is found, use its complete specification rather
than mix with the other arguments.  Also, change tab-completion to not
try to complete conninfo/URI-looking "dbnames" and document that
conninfos are accepted as first argument.

There was a weak consensus to backpatch this, because while the behavior
of using the dbname as a conninfo is nowhere documented for \connect, it
is reasonable to expect that it works because it does work in many other
contexts.  Therefore this is backpatched all the way back to 9.0.

To implement this, routines previously private to libpq have been
duplicated so that psql can decide what looks like a conninfo/URI
string.  In back branches, just duplicate the same code all the way back
to 9.2, where URIs where introduced; 9.0 and 9.1 have a simpler version.
In master, the routines are moved to src/common and renamed.

Author: David Fetter, Andrew Dunstan.  Some editorialization by me
(probably earning a Gierth's "Sloppy" badge in the process.)
Reviewers: Andrew Gierth, Erik Rijkers, Pavel Stěhule, Stephen Frost,
Robert Haas, Andrew Dunstan.

diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 15bbd7a7f6a4db8c74e459a5aeadcdb4289ec1be..bf29cb98f8e47dcc22652d133d5bda6c79e60d32 100644 (file)
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -766,23 +766,31 @@ testdb=>
       </varlistentry>
 
       <varlistentry>
-        <term><literal>\connect</literal> (or <literal>\c</literal>) <literal>[ <replaceable class="parameter">dbname</replaceable> [ <replaceable class="parameter">username</replaceable> ] [ <replaceable class="parameter">host</replaceable> ] [ <replaceable class="parameter">port</replaceable> ] ]</literal></term>
+        <term><literal>\connect</literal> (or <literal>\c</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>
         <listitem>
         <para>
         Establishes a new connection to a <productname>PostgreSQL</>
-        server. If the new connection is successfully made, the
-        previous connection is closed. If 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.
+        server.  The connection parameters to use can be specified either
+        using a positional syntax, or using <literal>conninfo</> connection
+        strings as detailed in <xref linkend="libpq-connect">.
         </para>
 
         <para>
+        When using positional parameters, if 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.
+        </para>
+
+        <para>
+        If the new connection is successfully made, the previous
+        connection is closed.
         If the connection attempt failed (wrong user name, access
         denied, etc.), the previous connection will only be kept if
         <application>psql</application> is in interactive mode. When
@@ -792,6 +800,15 @@ testdb=&gt;
         mechanism that scripts are not accidentally acting on the
         wrong database on the other hand.
         </para>
+
+        <para>
+        Examples:
+        </para>
+<programlisting>
+=&gt; \c mydb myuser host.dom 6432
+=&gt; \c service=foo
+=&gt; \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable"
+</programlisting>
         </listitem>
       </varlistentry>
 

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d2967edaa0aa53c7cd2ff960ce86d867ab3eb0b8..cd4309c46556b1270f103a6cba95a9c6aa7f2d06 100644 (file)
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1240,9 +1240,10 @@ do_connect(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;
 
-       if (!dbname)
-               dbname = PQdb(o_conn);
+       /* grab values from the old connection, unless supplied by caller */
        if (!user)
                user = PQuser(o_conn);
        if (!host)
@@ -1250,6 +1251,27 @@ do_connect(char *dbname, char *user, char *host, char *port)
        if (!port)
                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 positional
+        * syntax.
+        */
+       keep_password =
+               ((strcmp(user, PQuser(o_conn)) == 0) &&
+                (!host || strcmp(host, PQhost(o_conn)) == 0) &&
+                (strcmp(port, PQport(o_conn)) == 0) &&
+                !has_connection_string);
+
+       /*
+        * Grab dbname from old connection unless supplied by caller.  No password
+        * discard if this changes: passwords aren't (usually) database-specific.
+        */
+       if (!dbname)
+               dbname = PQdb(o_conn);
+
        /*
         * If the user asked to be prompted for a password, ask for one now. If
         * not, use the password from the old connection, provided the username
@@ -1264,7 +1286,7 @@ do_connect(char *dbname, char *user, char *host, char *port)
        {
                password = prompt_for_password(user);
        }
-       else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0)
+       else if (o_conn && keep_password)
        {
                password = strdup(PQpass(o_conn));
        }
@@ -1274,21 +1296,28 @@ do_connect(char *dbname, char *user, char *host, char *port)
 #define PARAMS_ARRAY_SIZE      7
                const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords));
                const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values));
+               int                     paramnum = 0;
 
-               keywords[0] = "host";
-               values[0] = host;
-               keywords[1] = "port";
-               values[1] = port;
-               keywords[2] = "user";
-               values[2] = user;
-               keywords[3] = "password";
-               values[3] = password;
-               keywords[4] = "dbname";
-               values[4] = dbname;
-               keywords[5] = "fallback_application_name";
-               values[5] = pset.progname;
-               keywords[6] = NULL;
-               values[6] = NULL;
+               keywords[0] = "dbname";
+               values[0] = dbname;
+
+               if (!has_connection_string)
+               {
+                       keywords[++paramnum] = "host";
+                       values[paramnum] = host;
+                       keywords[++paramnum] = "port";
+                       values[paramnum] = port;
+                       keywords[++paramnum] = "user";
+                       values[paramnum] = user;
+               }
+               keywords[++paramnum] = "password";
+               values[paramnum] = password;
+               keywords[++paramnum] = "fallback_application_name";
+               values[paramnum] = pset.progname;
+
+               /* add array terminator */
+               keywords[++paramnum] = NULL;
+               values[paramnum] = NULL;
 
                n_conn = PQconnectdbParams(keywords, values, true);
 

diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 5013ad36dfd6c2cc2bebd0b5b6d55cb738a78914..b0049b59c7f182717586ca965160663edd231248 100644 (file)
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -1574,3 +1574,15 @@ expand_tilde(char **filename)
 
        return *filename;
 }
+
+/*
+ * Recognized connection string contains a "=" in it.
+ *
+ * Must be consistent with conninfo_parse: anything for which this returns true
+ * should at least look like it's parseable by that routine.
+ */
+bool
+recognized_connection_string(const char *connstr)
+{
+       return strchr(connstr, '=') != NULL;
+}

diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 838a5526bd15495b5cfc4da295fa6dbfa53a7ab9..155771c57cfd686d0155faaded89f4c35adda120 100644 (file)
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -63,4 +63,6 @@ extern const char *session_username(void);
 
 extern char *expand_tilde(char **filename);
 
+extern bool recognized_connection_string(const char *connstr);
+
 #endif   /* COMMON_H */

diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index c31d3d6b4162cc3340865d803442e59f41cee79b..5c7bba11a1841de805064c5671cfdaf1c6a937ff 100644 (file)
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -249,8 +249,8 @@ slashUsage(unsigned short int pager)
        fprintf(output, "\n");
 
        fprintf(output, _("Connection\n"));
-       fprintf(output, _("  \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\n"
-       "                         connect to new database (currently \"%s\")\n"),
+       fprintf(output, _("  \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n"
+                                         "                         connect to new database (currently \"%s\")\n"),
                        currdb);
        fprintf(output, _("  \\encoding [ENCODING]   show or set client encoding\n"));
        fprintf(output, _("  \\password [USERNAME]   securely change the password for a user\n"));

diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index feeb76f0655df4c09287f180fe9182db76de0dec..f860e452ddd2ef4f83e1dc574a63e46900f05e34 100644 (file)
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -2451,8 +2451,10 @@ psql_completion(char *text, int start, int end)
 /* Backslash commands */
 /* TODO:  \dc \dd \dl */
        else if (strcmp(prev_wd, "\\connect") == 0 || strcmp(prev_wd, "\\c") == 0)
-               COMPLETE_WITH_QUERY(Query_for_list_of_databases);
-
+       {
+               if (!recognized_connection_string(text))
+                       COMPLETE_WITH_QUERY(Query_for_list_of_databases);
+       }
        else if (strncmp(prev_wd, "\\da", strlen("\\da")) == 0)
                COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_aggregates, NULL);
        else if (strncmp(prev_wd, "\\db", strlen("\\db")) == 0)