Disallow "=" in names of reloptions and foreign-data options.

We store values for these options as array elements with the syntax
"name=value", hence a name containing "=" confuses matters when
it's time to read the array back in.  Since validation of the
options is often done (long) after this conversion to array format,
that leads to confusing and off-point error messages.  We can
improve matters by rejecting names containing "=" up-front.

(Probably a better design would have involved pairs of array
elements, but it's too late now --- and anyway, there's no
evident use-case for option names like this.  We already
reject such names in some other contexts such as GUCs.)

Reported-by: Chapman Flack <jcflack@acm.org>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Chapman Flack <jcflack@acm.org>
Discussion: https://postgr.es/m/6830EB30.8090904@acm.org
Backpatch-through: 13

diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out
index 86c148a86ba3afa8d916c6323c0ba99f10f6c17c..4a31613fa2910d585bc0d666cf18dea4431d3496 100644 (file)
--- a/contrib/file_fdw/expected/file_fdw.out
+++ b/contrib/file_fdw/expected/file_fdw.out
@@ -48,6 +48,10 @@ SET ROLE regress_file_fdw_superuser;
 CREATE USER MAPPING FOR regress_file_fdw_superuser SERVER file_server;
 CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
 -- validator tests
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (foo 'bar');  -- ERROR
+ERROR:  invalid option "foo"
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS ("a=b" 'true');  -- ERROR
+ERROR:  invalid option name "a=b": must not contain "="
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml');  -- ERROR
 ERROR:  COPY format "xml" not recognized
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':');          -- ERROR

diff --git a/contrib/file_fdw/sql/file_fdw.sql b/contrib/file_fdw/sql/file_fdw.sql
index f0548e14e18452789cd83b4fa8f6d88a6b9284a9..e91b9799c4293e46e4268d8ccddeaa3fff8917c6 100644 (file)
--- a/contrib/file_fdw/sql/file_fdw.sql
+++ b/contrib/file_fdw/sql/file_fdw.sql
@@ -55,6 +55,8 @@ CREATE USER MAPPING FOR regress_file_fdw_superuser SERVER file_server;
 CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
 
 -- validator tests
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (foo 'bar');  -- ERROR
+CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS ("a=b" 'true');  -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml');  -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':');          -- ERROR
 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':');         -- ERROR

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index d6eb5d8559939fef263e9cab96c9a98d13cbc45f..c6a2d13be8d3fcb0da60619fe7d23c432c3683f0 100644 (file)
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1232,8 +1232,9 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
                }
                else
                {
-                       text       *t;
+                       const char *name;
                        const char *value;
+                       text       *t;
                        Size            len;
 
                        /*
@@ -1280,11 +1281,19 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
                         * have just "name", assume "name=true" is meant.  Note: the
                         * namespace is not output.
                         */
+                       name = def->defname;
                        if (def->arg != NULL)
                                value = defGetString(def);
                        else
                                value = "true";
 
+                       /* Insist that name not contain "=", else "a=b=c" is ambiguous */
+                       if (strchr(name, '=') != NULL)
+                               ereport(ERROR,
+                                               (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                                errmsg("invalid option name \"%s\": must not contain \"=\"",
+                                                               name)));
+
                        /*
                         * This is not a great place for this test, but there's no other
                         * convenient place to filter the option out. As WITH (oids =
@@ -1292,7 +1301,7 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
                         * amount of ugly.
                         */
                        if (acceptOidsOff && def->defnamespace == NULL &&
-                               strcmp(def->defname, "oids") == 0)
+                               strcmp(name, "oids") == 0)
                        {
                                if (defGetBoolean(def))
                                        ereport(ERROR,
@@ -1302,11 +1311,11 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
                                continue;
                        }
 
-                       len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
+                       len = VARHDRSZ + strlen(name) + 1 + strlen(value);
                        /* +1 leaves room for sprintf's trailing null */
                        t = (text *) palloc(len + 1);
                        SET_VARSIZE(t, len);
-                       sprintf(VARDATA(t), "%s=%s", def->defname, value);
+                       sprintf(VARDATA(t), "%s=%s", name, value);
 
                        astate = accumArrayResult(astate, PointerGetDatum(t),
                                                                          false, TEXTOID,

diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c
index cf61bbac1fa104f197361d1c20420ed5374aa603..17f40599d269fb657213abbcd30a94f0b939c3ad 100644 (file)
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -71,15 +71,26 @@ optionListToArray(List *options)
        foreach(cell, options)
        {
                DefElem    *def = lfirst(cell);
+               const char *name;
                const char *value;
                Size            len;
                text       *t;
 
+               name = def->defname;
                value = defGetString(def);
-               len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
+
+               /* Insist that name not contain "=", else "a=b=c" is ambiguous */
+               if (strchr(name, '=') != NULL)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+                                        errmsg("invalid option name \"%s\": must not contain \"=\"",
+                                                       name)));
+
+               len = VARHDRSZ + strlen(name) + 1 + strlen(value);
+               /* +1 leaves room for sprintf's trailing null */
                t = palloc(len + 1);
                SET_VARSIZE(t, len);
-               sprintf(VARDATA(t), "%s=%s", def->defname, value);
+               sprintf(VARDATA(t), "%s=%s", name, value);
 
                astate = accumArrayResult(astate, PointerGetDatum(t),
                                                                  false, TEXTOID,