>
>
>
>> It looks like this might be fairly easy to fix by having
>> get_connect_string() use is_valid_dblink_option() to check each
>> option name, and silently ignore options that are inappropriate.
>>
>
> From what I can tell, it is very straightforward, the context oids are set
> up just a few lines above where the new is_valid_dblink_option() calls
> would be.
>
> I'm happy to write the patch, for both v10 and any back-patches we feel
> are necessary. However, I suspect with a patch this trivial that reviewing
> another person's patch might be more work for a committer than just doing
> it themselves. If that's not the case, let me know and I'll get started.
>

Joe indicated that he wouldn't be able to get to the patch until this
weekend at the earliest, so I went ahead and made the patches on my own.

Nothing unusual to report for master, 9.6, 9.5, or 9.3. The patch is
basically the same for all of them and I was able to re-run the test script
at the beginning of the thread to ensure that the fix worked.

In 9.4, I encountered a complaint about flex 2.6.0. After a little research
it seems that a fix for that made it into versions 9.3+, but not 9.4. That
mini-patch is attached as well (0001.configure.94.diff). The dblink patch
for 9.4 was basically the same as the others.

The issue (no validation of connection string elements pulled from an FDW)
exists in 9.2, however, the only possible source of such options I know of
(postgres_fdw) does not. So I doubt we need to patch 9.2, but it's trivial
to do so if we want to.
diff --git a/configure b/configure
index 5204869..7da2dac 100755
--- a/configure
+++ b/configure
@@ -7166,7 +7166,7 @@ else
         echo '%%'  > conftest.l
         if $pgac_candidate -t conftest.l 2>/dev/null | grep FLEX_SCANNER 
>/dev/null 2>&1; then
           pgac_flex_version=`$pgac_candidate --version 2>/dev/null`
-          if echo "$pgac_flex_version" | sed 's/[.a-z]/ /g' | $AWK '{ if ($1 = 
2 && $2 = 5 && $3 >= 31) exit 0; else exit 1;}'
+          if echo "$pgac_flex_version" | sed 's/[.a-z]/ /g' | $AWK '{ if ($1 
== 2 && ($2 > 5 || ($2 == 5 && $3 >= 31))) exit 0; else exit 1;}'
           then
             pgac_cv_path_flex=$pgac_candidate
             break 2
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index af062fa..491dec8 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -40,6 +40,7 @@
 #include "access/reloptions.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
@@ -2731,6 +2732,24 @@ get_connect_string(const char *servername)
        ForeignDataWrapper *fdw;
        AclResult       aclresult;
        char       *srvname;
+       static const PQconninfoOption *options = NULL;
+
+       /*
+        * Get list of valid libpq options.
+        *
+        * To avoid unnecessary work, we get the list once and use it throughout
+        * the lifetime of this backend process.  We don't need to care about
+        * memory context issues, because PQconndefaults allocates with malloc.
+        */
+       if (!options)
+       {
+               options = PQconndefaults();
+               if (!options)                   /* assume reason for failure is 
OOM */
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
+                                        errmsg("out of memory"),
+                        errdetail("could not get libpq's default connection 
options")));
+       }
 
        /* first gather the server connstr options */
        srvname = pstrdup(servername);
@@ -2755,16 +2774,18 @@ get_connect_string(const char *servername)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignDataWrapperRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, foreign_server->options)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignServerRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, user_mapping->options)
@@ -2772,8 +2793,9 @@ get_connect_string(const char *servername)
 
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
UserMappingRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                return buf->data;
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 009b877..9f01ef7 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -40,6 +40,7 @@
 #include "access/reloptions.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
@@ -2728,6 +2729,24 @@ get_connect_string(const char *servername)
        ForeignDataWrapper *fdw;
        AclResult       aclresult;
        char       *srvname;
+       static const PQconninfoOption *options = NULL;
+
+       /*
+        * Get list of valid libpq options.
+        *
+        * To avoid unnecessary work, we get the list once and use it throughout
+        * the lifetime of this backend process.  We don't need to care about
+        * memory context issues, because PQconndefaults allocates with malloc.
+        */
+       if (!options)
+       {
+               options = PQconndefaults();
+               if (!options)                   /* assume reason for failure is 
OOM */
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
+                                        errmsg("out of memory"),
+                        errdetail("could not get libpq's default connection 
options")));
+       }
 
        /* first gather the server connstr options */
        srvname = pstrdup(servername);
@@ -2752,16 +2771,18 @@ get_connect_string(const char *servername)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignDataWrapperRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, foreign_server->options)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignServerRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, user_mapping->options)
@@ -2769,8 +2790,9 @@ get_connect_string(const char *servername)
 
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
UserMappingRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                return buf->data;
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index c5892d3..e041381 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -40,6 +40,7 @@
 #include "access/reloptions.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
@@ -2728,6 +2729,24 @@ get_connect_string(const char *servername)
        ForeignDataWrapper *fdw;
        AclResult       aclresult;
        char       *srvname;
+       static const PQconninfoOption *options = NULL;
+
+       /*
+        * Get list of valid libpq options.
+        *
+        * To avoid unnecessary work, we get the list once and use it throughout
+        * the lifetime of this backend process.  We don't need to care about
+        * memory context issues, because PQconndefaults allocates with malloc.
+        */
+       if (!options)
+       {
+               options = PQconndefaults();
+               if (!options)                   /* assume reason for failure is 
OOM */
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
+                                        errmsg("out of memory"),
+                        errdetail("could not get libpq's default connection 
options")));
+       }
 
        /* first gather the server connstr options */
        srvname = pstrdup(servername);
@@ -2752,16 +2771,18 @@ get_connect_string(const char *servername)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignDataWrapperRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, foreign_server->options)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignServerRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, user_mapping->options)
@@ -2769,8 +2790,9 @@ get_connect_string(const char *servername)
 
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
UserMappingRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                return buf->data;
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index d4f9090..0d0b848 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -40,6 +40,7 @@
 #include "access/reloptions.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
@@ -2726,6 +2727,24 @@ get_connect_string(const char *servername)
        ForeignDataWrapper *fdw;
        AclResult       aclresult;
        char       *srvname;
+       static const PQconninfoOption *options = NULL;
+
+       /*
+        * Get list of valid libpq options.
+        *
+        * To avoid unnecessary work, we get the list once and use it throughout
+        * the lifetime of this backend process.  We don't need to care about
+        * memory context issues, because PQconndefaults allocates with malloc.
+        */
+       if (!options)
+       {
+               options = PQconndefaults();
+               if (!options)                   /* assume reason for failure is 
OOM */
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
+                                        errmsg("out of memory"),
+                        errdetail("could not get libpq's default connection 
options")));
+       }
 
        /* first gather the server connstr options */
        srvname = pstrdup(servername);
@@ -2750,16 +2769,18 @@ get_connect_string(const char *servername)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignDataWrapperRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, foreign_server->options)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignServerRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, user_mapping->options)
@@ -2767,8 +2788,9 @@ get_connect_string(const char *servername)
 
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
UserMappingRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                return buf->data;
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index d4f9090..ac8cdce 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -40,6 +40,7 @@
 #include "access/reloptions.h"
 #include "catalog/indexing.h"
 #include "catalog/namespace.h"
+#include "catalog/pg_foreign_data_wrapper.h"
 #include "catalog/pg_foreign_server.h"
 #include "catalog/pg_type.h"
 #include "catalog/pg_user_mapping.h"
@@ -2727,6 +2728,25 @@ get_connect_string(const char *servername)
        AclResult       aclresult;
        char       *srvname;
 
+       static const PQconninfoOption *options = NULL;
+
+       /*
+        * Get list of valid libpq options.
+        *
+        * To avoid unnecessary work, we get the list once and use it throughout
+        * the lifetime of this backend process.  We don't need to care about
+        * memory context issues, because PQconndefaults allocates with malloc.
+        */
+       if (!options)
+       {
+               options = PQconndefaults();
+               if (!options)                   /* assume reason for failure is 
OOM */
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FDW_OUT_OF_MEMORY),
+                                        errmsg("out of memory"),
+                        errdetail("could not get libpq's default connection 
options")));
+       }
+
        /* first gather the server connstr options */
        srvname = pstrdup(servername);
        truncate_identifier(srvname, strlen(srvname), false);
@@ -2750,16 +2770,18 @@ get_connect_string(const char *servername)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignDataWrapperRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, foreign_server->options)
                {
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
ForeignServerRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                foreach(cell, user_mapping->options)
@@ -2767,8 +2789,9 @@ get_connect_string(const char *servername)
 
                        DefElem    *def = lfirst(cell);
 
-                       appendStringInfo(buf, "%s='%s' ", def->defname,
-                                                        
escape_param_str(strVal(def->arg)));
+                       if ( is_valid_dblink_option(options, def->defname, 
UserMappingRelationId ) )
+                               appendStringInfo(buf, "%s='%s' ", def->defname,
+                                                                
escape_param_str(strVal(def->arg)));
                }
 
                return buf->data;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to