From 4cd639baf4bd35dd7fc924009203349b81bdcd68 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Thu, 2 Apr 2015 10:10:22 -0400 Subject: [PATCH] Revert "psql: fix \connect with URIs and conninfo strings" This reverts commit fcef1617295c074f2684c887627184d2fc26ac04, about which both the buildfarm and my local machine are very unhappy. --- doc/src/sgml/ref/psql-ref.sgml | 40 +++++----------- src/bin/psql/command.c | 80 +++++++++---------------------- src/bin/psql/help.c | 4 +- src/bin/psql/tab-complete.c | 13 +---- src/common/Makefile | 2 +- src/common/connstrings.c | 53 -------------------- src/include/common/connstrings.h | 16 ------- src/interfaces/libpq/fe-connect.c | 46 +++++++++++++++--- 8 files changed, 79 insertions(+), 175 deletions(-) delete mode 100644 src/common/connstrings.c delete mode 100644 src/include/common/connstrings.h diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 62a3b21209..1f29615f83 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -796,31 +796,23 @@ testdb=> - \c or \connect [ dbname [ username ] [ host ] [ port ] ] | conninfo + \c or \connect [ dbname [ username ] [ host ] [ port ] ] Establishes a new connection to a PostgreSQL - server. The connection parameters to use can be specified either - using a positional syntax, or using conninfo connection - strings as detailed in . + server. If the new connection is successfully made, the + previous connection is closed. If any of dbname, username, host or port are omitted or specified + as -, the value of that parameter from the + previous connection is used. If there is no previous + connection, the libpq default for + the parameter's value is used. - When using positional parameters, if any of - dbname, - username, - host or - port are omitted or - specified as -, the value of that parameter from - the previous connection is used; if there is no previous connection, - the libpq default for the parameter's value - is used. When using conninfo strings, no values from the - previous connection are used for the new connection. - - - - 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 psql is in interactive mode. When @@ -830,16 +822,6 @@ testdb=> mechanism that scripts are not accidentally acting on the wrong database on the other hand. - - - Examples: - - -=> \c mydb myuser host.dom 6432 -=> \c service=foo -=> \c "host=localhost port=5432 dbname=mydb connect_timeout=10 sslmode=disable" -=> \c postgresql://tom@localhost/mydb?application_name=myapp - diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 754fba2ee7..916f1c6301 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -31,7 +31,6 @@ #include /* for stat() */ #endif -#include "common/connstrings.h" #include "portability/instr_time.h" #include "libpq-fe.h" @@ -1609,8 +1608,6 @@ 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)) { @@ -1624,7 +1621,8 @@ do_connect(char *dbname, char *user, char *host, char *port) return false; } - /* grab values from the old connection, unless supplied by caller */ + if (!dbname) + dbname = PQdb(o_conn); if (!user) user = PQuser(o_conn); if (!host) @@ -1632,27 +1630,6 @@ do_connect(char *dbname, char *user, char *host, char *port) if (!port) port = PQport(o_conn); - has_connection_string = - dbname ? libpq_connstring_is_recognized(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 @@ -1667,13 +1644,9 @@ do_connect(char *dbname, char *user, char *host, char *port) { password = prompt_for_password(user); } - else if (o_conn && keep_password) + else if (o_conn && user && strcmp(PQuser(o_conn), user) == 0) { - password = PQpass(o_conn); - if (password && *password) - password = pg_strdup(password); - else - password = NULL; + password = pg_strdup(PQpass(o_conn)); } while (true) @@ -1681,39 +1654,32 @@ 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] = "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; + 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; n_conn = PQconnectdbParams(keywords, values, true); - pg_free(keywords); - pg_free(values); + free(keywords); + free(values); /* We can immediately discard the password -- no longer needed */ if (password) - pg_free(password); + free(password); if (PQstatus(n_conn) == CONNECTION_OK) break; diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index ea05c3e37b..f58f5e52f3 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -260,11 +260,11 @@ slashUsage(unsigned short int pager) fprintf(output, _("Connection\n")); if (currdb) - fprintf(output, _(" \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n" + fprintf(output, _(" \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\n" " connect to new database (currently \"%s\")\n"), currdb); else - fprintf(output, _(" \\c[onnect] {[DBNAME|- USER|- HOST|- PORT|-] | conninfo}\n" + fprintf(output, _(" \\c[onnect] [DBNAME|- USER|- HOST|- PORT|-]\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")); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index dd15f10452..dfcb66ba6d 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -52,7 +52,6 @@ #include "libpq-fe.h" #include "pqexpbuffer.h" #include "common.h" -#include "common/connstrings.h" #include "settings.h" #include "stringutils.h" @@ -3707,18 +3706,10 @@ 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) - { - if (!libpq_connstring_is_recognized(text)) - COMPLETE_WITH_QUERY(Query_for_list_of_databases); - /* TODO: URI/service completion. Nothing for now */ - } + COMPLETE_WITH_QUERY(Query_for_list_of_databases); else if (strcmp(prev2_wd, "\\connect") == 0 || strcmp(prev2_wd, "\\c") == 0) - { - if (!libpq_connstring_is_recognized(prev_wd)) - COMPLETE_WITH_QUERY(Query_for_list_of_roles); - } + 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); diff --git a/src/common/Makefile b/src/common/Makefile index 4bb88c1947..c2e456d89a 100644 --- a/src/common/Makefile +++ b/src/common/Makefile @@ -26,7 +26,7 @@ LIBS += $(PTHREAD_LIBS) OBJS_COMMON = exec.o pg_crc.o pg_lzcompress.o pgfnames.o psprintf.o relpath.o \ rmtree.o string.o username.o wait_error.o -OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o restricted_token.o connstrings.o +OBJS_FRONTEND = $(OBJS_COMMON) fe_memutils.o restricted_token.o OBJS_SRV = $(OBJS_COMMON:%.o=%_srv.o) diff --git a/src/common/connstrings.c b/src/common/connstrings.c deleted file mode 100644 index 91170a1884..0000000000 --- a/src/common/connstrings.c +++ /dev/null @@ -1,53 +0,0 @@ -/* - * connstrings.c - * connecting string processing functions - * - * Copyright (c) 2012-2015, PostgreSQL Global Development Group - * - * src/include/common/connstrings.c - */ -#include "postgres_fe.h" - -#include - -#include "common/connstrings.h" - - -/* The connection URI must start with either of the following designators: */ -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. - */ -int -libpq_connstring_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. - * - * Must be consistent with parse_connection_string: anything for which this - * returns true should at least look like it's parseable by that routine. - */ -bool -libpq_connstring_is_recognized(const char *connstr) -{ - return libpq_connstring_uri_prefix_length(connstr) != 0 || - strchr(connstr, '=') != NULL; -} diff --git a/src/include/common/connstrings.h b/src/include/common/connstrings.h deleted file mode 100644 index 4108ba481f..0000000000 --- a/src/include/common/connstrings.h +++ /dev/null @@ -1,16 +0,0 @@ -/* - * connstrings.h - * connecting string processing prototypes - * - * Copyright (c) 2012-2015, PostgreSQL Global Development Group - * - * src/include/common/connstrings.h - */ -#ifndef CONNSTRINGS_H -#define CONNSTRINGS_H - - -extern int libpq_connstring_uri_prefix_length(const char *connstr); -extern bool libpq_connstring_is_recognized(const char *connstr); - -#endif /* CONNSTRINGS_H */ diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 638d103f8c..e2a06b3d92 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -22,7 +22,6 @@ #include #include -#include "common/connstrings.h" #include "libpq-fe.h" #include "libpq-int.h" #include "fe-auth.h" @@ -340,6 +339,8 @@ static void closePGconn(PGconn *conn); static PQconninfoOption *conninfo_init(PQExpBuffer errorMessage); static PQconninfoOption *parse_connection_string(const char *conninfo, PQExpBuffer errorMessage, bool use_defaults); +static int uri_prefix_length(const char *connstr); +static bool recognized_connection_string(const char *connstr); static PQconninfoOption *conninfo_parse(const char *conninfo, PQExpBuffer errorMessage, bool use_defaults); static PQconninfoOption *conninfo_array_parse(const char *const * keywords, @@ -970,7 +971,7 @@ PQsetdbLogin(const char *pghost, const char *pgport, const char *pgoptions, * If the dbName parameter contains what looks like a connection string, * parse it into conn struct using connectOptions1. */ - if (dbName && libpq_connstring_is_recognized(dbName)) + if (dbName && recognized_connection_string(dbName)) { if (!connectOptions1(conn, dbName)) return conn; @@ -4184,13 +4185,46 @@ parse_connection_string(const char *connstr, PQExpBuffer errorMessage, bool use_defaults) { /* Parse as URI if connection string matches URI prefix */ - if (libpq_connstring_uri_prefix_length(connstr) != 0) + if (uri_prefix_length(connstr) != 0) return conninfo_uri_parse(connstr, errorMessage, use_defaults); /* Parse as default otherwise */ return conninfo_parse(connstr, errorMessage, use_defaults); } +/* + * 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. + */ +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. + * + * Must be consistent with parse_connection_string: anything for which this + * returns true should at least look like it's parseable by that routine. + */ +static bool +recognized_connection_string(const char *connstr) +{ + return uri_prefix_length(connstr) != 0 || strchr(connstr, '=') != NULL; +} + /* * Subroutine for parse_connection_string * @@ -4366,7 +4400,7 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage, * * If expand_dbname is non-zero, and the value passed for the first occurrence * of "dbname" keyword is a connection string (as indicated by - * libpq_connstring_is_recognized) then parse and process it, overriding any + * recognized_connection_string) then parse and process it, overriding any * previously processed conflicting keywords. Subsequent keywords will take * precedence, however. */ @@ -4397,7 +4431,7 @@ conninfo_array_parse(const char *const * keywords, const char *const * values, * defaults here -- those get picked up later. We only want to * override for those parameters actually passed. */ - if (libpq_connstring_is_recognized(pvalue)) + if (recognized_connection_string(pvalue)) { dbname_options = parse_connection_string(pvalue, errorMessage, false); if (dbname_options == NULL) @@ -4688,7 +4722,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri, start = buf; /* Skip the URI prefix */ - prefix_len = libpq_connstring_uri_prefix_length(uri); + prefix_len = uri_prefix_length(uri); if (prefix_len == 0) { /* Should never happen */