From: Arseny Sher Date: Thu, 27 Jul 2017 12:41:17 +0300 Subject: [PATCH] Allow 'dbname' option contain full-fledged connstring in postgres_fdw And if this is the case, validate that it doesn't contain any invalid options such as client_encoding or user. --- contrib/postgres_fdw/connection.c | 2 +- contrib/postgres_fdw/option.c | 80 ++++++++++++++++++++++++++++++++------- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index be4ec07cf9..bfcd9ed2e3 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -263,7 +263,7 @@ connect_pg_server(ForeignServer *server, UserMapping *user) /* verify connection parameters and make connection */ check_conn_params(keywords, values); - conn = PQconnectdbParams(keywords, values, false); + conn = PQconnectdbParams(keywords, values, true); if (!conn || PQstatus(conn) != CONNECTION_OK) ereport(ERROR, (errcode(ERRCODE_SQLCLIENT_UNABLE_TO_ESTABLISH_SQLCONNECTION), diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index 67e1c59951..26c11e5179 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -50,8 +50,10 @@ static PQconninfoOption *libpq_options; * Helper functions */ static void InitPgFdwOptions(void); +static void validate_dbname(const char *dbname); +static void get_opts_hint(StringInfo buf, Oid context, bool libpq_options); static bool is_valid_option(const char *keyword, Oid context); -static bool is_libpq_option(const char *keyword); +static bool is_libpq_option(const char *keyword, Oid context); /* @@ -86,16 +88,8 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) * Unknown option specified, complain about it. Provide a hint * with list of valid options for the object. */ - PgFdwOption *opt; StringInfoData buf; - - initStringInfo(&buf); - for (opt = postgres_fdw_options; opt->keyword; opt++) - { - if (catalog == opt->optcontext) - appendStringInfo(&buf, "%s%s", (buf.len > 0) ? ", " : "", - opt->keyword); - } + get_opts_hint(&buf, catalog, false); ereport(ERROR, (errcode(ERRCODE_FDW_INVALID_OPTION_NAME), @@ -143,6 +137,10 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) errmsg("%s requires a non-negative integer value", def->defname))); } + else if (strcmp(def->defname, "dbname") == 0) + { + validate_dbname(defGetString(def)); + } } PG_RETURN_VOID(); @@ -250,6 +248,59 @@ InitPgFdwOptions(void) } /* + * If dbname param specified, make sure it doesn't carry invalid + * options if it is a connstring. + */ +static void +validate_dbname(const char *dbname) +{ + PQconninfoOption *lopts; + PQconninfoOption *lopt; + + /* If it is not a connstring, just skip the check */ + if ((lopts = PQconninfoParse(dbname, NULL)) != NULL) + { + for (lopt = lopts; lopt->keyword; lopt++) + { + if (lopt->val != NULL && !is_libpq_option(lopt->keyword, + ForeignServerRelationId)) + { + StringInfoData buf; + get_opts_hint(&buf, ForeignServerRelationId, true); + + ereport(ERROR, + (errcode(ERRCODE_FDW_INVALID_OPTION_NAME), + errmsg("invalid option in dbname connstring \"%s\"", + lopt->keyword), + errhint("Valid options in this context are: %s", + buf.data))); + } + } + PQconninfoFree(lopts); + } +} + +/* + * Put to 'buf' a hint with list of valid options for the object 'context'. If + * libpq_options is true, only libpq options are provided. 'buf' must point to + * existing StringInfoData struct when called. + */ +static void +get_opts_hint(StringInfo buf, Oid context, bool libpq_options) +{ + PgFdwOption *opt; + + initStringInfo(buf); + for (opt = postgres_fdw_options; opt->keyword; opt++) + { + if (context == opt->optcontext && + (!libpq_options || opt->is_libpq_opt)) + appendStringInfo(buf, "%s%s", (buf->len > 0) ? ", " : "", + opt->keyword); + } +} + +/* * Check whether the given option is one of the valid postgres_fdw options. * context is the Oid of the catalog holding the object the option is for. */ @@ -271,9 +322,11 @@ is_valid_option(const char *keyword, Oid context) /* * Check whether the given option is one of the valid libpq options. + * context is the Oid of the catalog holding the object the option is for; + * if it is InvalidOid, don't check the catalog. */ static bool -is_libpq_option(const char *keyword) +is_libpq_option(const char *keyword, Oid context) { PgFdwOption *opt; @@ -281,7 +334,8 @@ is_libpq_option(const char *keyword) for (opt = postgres_fdw_options; opt->keyword; opt++) { - if (opt->is_libpq_opt && strcmp(opt->keyword, keyword) == 0) + if (opt->is_libpq_opt && strcmp(opt->keyword, keyword) == 0 && + (context == InvalidOid || context == opt->optcontext)) return true; } @@ -308,7 +362,7 @@ ExtractConnectionOptions(List *defelems, const char **keywords, { DefElem *d = (DefElem *) lfirst(lc); - if (is_libpq_option(d->defname)) + if (is_libpq_option(d->defname, InvalidOid)) { keywords[i] = d->defname; values[i] = defGetString(d); -- 2.11.0 --=-=-= Content-Type: text/plain -- Arseny Sher Postgres Professional: http://www.postgrespro.com The Russian Postgres Company --=-=-= Content-Type: text/plain Content-Disposition: inline Content-Transfer-Encoding: 8bit MIME-Version: 1.0 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers --=-=-=--