From 09e3924501d219331b8b8fca12a9ea35a689ba10 Mon Sep 17 00:00:00 2001
From: Arseny Sher <sher-ars@yandex.ru>
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