psql: fix \connect with URIs and conninfo strings

This is the second try at this, after fcef161729 failed miserably and
had to be reverted: as it turns out, libpq cannot depend on libpgcommon
after all. Instead of shuffling code in the master branch, make that one
just like 9.4 and accept the duplication.  (This was all my own mistake,
not the patch submitter's).

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.

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.
This commit is contained in:
Alvaro Herrera 2015-04-02 12:30:57 -03:00
parent f272098e91
commit e146ca6820
7 changed files with 142 additions and 39 deletions

View File

@ -796,23 +796,31 @@ testdb=>
</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> ] ]</literal></term>
<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>
<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-connstring">.
</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
@ -822,6 +830,16 @@ 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"
=&gt; \c postgresql://tom@localhost/mydb?application_name=myapp
</programlisting>
</listitem>
</varlistentry>

View File

@ -1608,6 +1608,8 @@ 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 (!o_conn && (!dbname || !user || !host || !port))
{
@ -1621,8 +1623,7 @@ do_connect(char *dbname, char *user, char *host, char *port)
return false;
}
if (!dbname)
dbname = PQdb(o_conn);
/* grab values from the old connection, unless supplied by caller */
if (!user)
user = PQuser(o_conn);
if (!host)
@ -1630,6 +1631,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
@ -1644,9 +1666,13 @@ 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 = pg_strdup(PQpass(o_conn));
password = PQpass(o_conn);
if (password && *password)
password = pg_strdup(password);
else
password = NULL;
}
while (true)
@ -1654,32 +1680,39 @@ 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;
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] = "client_encoding";
values[6] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
keywords[7] = NULL;
values[7] = 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;
keywords[++paramnum] = "client_encoding";
values[paramnum] = (pset.notty || getenv("PGCLIENTENCODING")) ? NULL : "auto";
/* add array terminator */
keywords[++paramnum] = NULL;
values[paramnum] = NULL;
n_conn = PQconnectdbParams(keywords, values, true);
free(keywords);
free(values);
pg_free(keywords);
pg_free(values);
/* We can immediately discard the password -- no longer needed */
if (password)
free(password);
pg_free(password);
if (PQstatus(n_conn) == CONNECTION_OK)
break;

View File

@ -1846,3 +1846,44 @@ expand_tilde(char **filename)
return;
}
/*
* Checks if connection string starts with either of the valid URI prefix
* designators.
*
* Returns the URI prefix length, 0 if the string doesn't contain a URI prefix.
*
* XXX This is a duplicate of the eponymous libpq function.
*/
static int
uri_prefix_length(const char *connstr)
{
/* The connection URI must start with either of the following designators: */
static const char uri_designator[] = "postgresql://";
static const char short_uri_designator[] = "postgres://";
if (strncmp(connstr, uri_designator,
sizeof(uri_designator) - 1) == 0)
return sizeof(uri_designator) - 1;
if (strncmp(connstr, short_uri_designator,
sizeof(short_uri_designator) - 1) == 0)
return sizeof(short_uri_designator) - 1;
return 0;
}
/*
* Recognized connection string either starts with a valid URI prefix or
* contains a "=" in it.
*
* Must be consistent with parse_connection_string: anything for which this
* returns true should at least look like it's parseable by that routine.
*
* XXX This is a duplicate of the eponymous libpq function.
*/
bool
recognized_connection_string(const char *connstr)
{
return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
}

View File

@ -46,4 +46,6 @@ extern const char *session_username(void);
extern void expand_tilde(char **filename);
extern bool recognized_connection_string(const char *connstr);
#endif /* COMMON_H */

View File

@ -260,11 +260,11 @@ slashUsage(unsigned short int pager)
fprintf(output, _("Connection\n"));
if (currdb)
fprintf(output, _(" \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\n"
fprintf(output, _(" \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n"
" connect to new database (currently \"%s\")\n"),
currdb);
else
fprintf(output, _(" \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\n"
fprintf(output, _(" \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n"
" connect to new database (currently no connection)\n"));
fprintf(output, _(" \\encoding [ENCODING] show or set client encoding\n"));
fprintf(output, _(" \\password [USERNAME] securely change the password for a user\n"));

View File

@ -3707,10 +3707,15 @@ psql_completion(const char *text, int start, int end)
COMPLETE_WITH_LIST_CS(my_list);
}
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 (strcmp(prev2_wd, "\\connect") == 0 || strcmp(prev2_wd, "\\c") == 0)
COMPLETE_WITH_QUERY(Query_for_list_of_roles);
{
if (!recognized_connection_string(prev_wd))
COMPLETE_WITH_QUERY(Query_for_list_of_roles);
}
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)

View File

@ -4197,6 +4197,8 @@ parse_connection_string(const char *connstr, PQExpBuffer errorMessage,
* designators.
*
* Returns the URI prefix length, 0 if the string doesn't contain a URI prefix.
*
* XXX this is duplicated in psql/common.c.
*/
static int
uri_prefix_length(const char *connstr)
@ -4218,6 +4220,8 @@ uri_prefix_length(const char *connstr)
*
* Must be consistent with parse_connection_string: anything for which this
* returns true should at least look like it's parseable by that routine.
*
* XXX this is duplicated in psql/common.c
*/
static bool
recognized_connection_string(const char *connstr)