On Wed, Dec 17, 2014 at 08:14:04AM -0500, Andrew Dunstan wrote:
>
> On 12/17/2014 04:11 AM, Heikki Linnakangas wrote:
> >On 12/17/2014 10:03 AM, Albe Laurenz wrote:
> >>David Fetter wrote:
> >>>I've noticed that psql's \c function handles service= requests in a
> >>>way that I can only characterize as broken.
> >>>
> >>>This came up in the context of connecting to a cloud hosting service
> >>>named after warriors or a river or something, whose default hostnames
> >>>are long, confusing, and easy to typo, so I suspect that service= may
> >>>come up more often going forward than it has until now.
> >>>
> >>>For example, when I try to use
> >>>
> >>>\c "service=foo"
> >>>
> >>>It will correctly figure out which database I'm trying to connect to,
> >>>but fail to notice that it's on a different host, port, etc., and
> >>>hence fail to connect with a somewhat unhelpful error message.
> >>>
> >>>I can think of a few approaches for fixing this:
> >>>
> >>>0. Leave it broken.
> >>>1. Disable "service=" requests entirely in \c context, and error out
> >>>if attempted.
> >>>2. Ensure that \c actually uses all of the available information.
> >>>
> >>>Is there another one I missed?
> >>>
> >>>If not, which of the approaches seems reasonable?
> >>
> >>#2 is the correct solution, #1 a band aid.
> >
> >It would be handy, if \c "service=foo" actually worked. We should do #3.
> >If the database name is actually a connection string, or a service
> >specification, it should not re-use the hostname and port from previous
> >connection, but use the values from the connection string or service file.
>
> Yeah, that's the correct solution. It should not be terribly difficult to
> create a test for a conninfo string in the dbname parameter. That's what
> libpq does after all. We certainly don't want psql to have to try to
> interpret the service file. psql just needs to let libpq do its work in this
> situation.
This took a little longer to get time to polish than I'd hoped, but
please find attached a patch which:
- Correctly connects to service= and postgres(ql)?:// with \c
- Disallows tab completion in the above cases
I'd like to see about having tab completion actually work correctly in
at least the service= case, but that's a matter for a follow-on patch.
Thanks to Andrew Dunstan for the original patch, and to Andrew Gierth
for his help getting it into shape.
Cheers,
David.
--
David Fetter <[email protected]> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: [email protected]
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c7a17d7..4ca50b3 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1610,6 +1610,8 @@ do_connect(char *dbname, char *user, char *host, char
*port)
PGconn *o_conn = pset.db,
*n_conn;
char *password = NULL;
+ bool keep_password = true;
+ bool has_connection_string = false;
if (!o_conn && (!dbname || !user || !host || !port))
{
@@ -1623,14 +1625,32 @@ do_connect(char *dbname, char *user, char *host, char
*port)
return false;
}
- if (!dbname)
- dbname = PQdb(o_conn);
if (!user)
user = PQuser(o_conn);
+ else if (strcmp(user, PQuser(o_conn)) != 0)
+ keep_password = false;
+
if (!host)
host = PQhost(o_conn);
+ else if (strcmp(host, PQhost(o_conn)) != 0)
+ keep_password = false;
+
if (!port)
port = PQport(o_conn);
+ else if (strcmp(port, PQport(o_conn)) != 0)
+ keep_password = false;
+
+ has_connection_string = recognized_connection_string(dbname);
+
+ if (has_connection_string)
+ keep_password = false;
+
+ /*
+ * Unlike the previous stanzas, changing only the dbname shouldn't
+ * trigger throwing away the password.
+ */
+ if (!dbname)
+ dbname = PQdb(o_conn);
/*
* If the user asked to be prompted for a password, ask for one now. If
@@ -1646,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)
@@ -1656,23 +1680,28 @@ 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";
+ keywords[++paramnum] = NULL;
+ values[paramnum] = NULL;
n_conn = PQconnectdbParams(keywords, values, true);
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 2001d31..19a5fd4 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -49,4 +49,41 @@ extern const char *session_username(void);
extern void expand_tilde(char **filename);
+static const char uri_designator[] = "postgresql://";
+static const char short_uri_designator[] = "postgres://";
+
+/*
+ * 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.
+ *
+ * copied from libpq/fe-connect.c
+ */
+static int
+uri_prefix_length(const char *connstr)
+{
+ 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.
+ *
+ * copied from libpq/fe-connect.c
+ */
+static bool
+recognized_connection_string(const char *connstr)
+{
+ return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL;
+}
+
#endif /* COMMON_H */
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 82c926d..e4e4356 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -69,6 +69,10 @@ extern char *filename_completion_function();
/* word break characters */
#define WORD_BREAKS "\t\n@$><=;|&{() "
+/* XXX Until we can tab complete for URIs and services, each of which
+ * should populate their own lists */
+static const char *const empty_list[] = {NULL};
+
/*
* This struct is used to define "schema queries", which are custom-built
* to obtain possibly-schema-qualified names of database objects. There is
@@ -3705,10 +3709,22 @@ 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);
+ {
+ /* URI/service completion. Nothing for now */
+ if (recognized_connection_string(text))
+ COMPLETE_WITH_LIST(empty_list);
+ else
+ 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_LIST(empty_list);
+ else
+ 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);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
