projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 31a7e17)
Disallow "=" in names of reloptions and foreign-data options. master github/master
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 2 Jun 2025 19:22:44 +0000 (15:22 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 2 Jun 2025 19:22:44 +0000 (15:22 -0400)
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

contrib/file_fdw/expected/file_fdw.out patch | blob | blame | history
contrib/file_fdw/sql/file_fdw.sql patch | blob | blame | history
src/backend/access/common/reloptions.c patch | blob | blame | history
src/backend/commands/foreigncmds.c patch | blob | blame | history

diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out
index df8d43b37498982416430c514072a18cbceb34fa..246e3d3e566fc89445b96e1f8e5e7a0941e859d1 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 2cdbe7a8a4c526400b8a7c1b8ad35c963ddf1b3e..1a397ad4bd15047327fb28da79b57c4b841bc078 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 46c1dce222d1003bdfcddcdb655f739bbcf4473b..50747c163961221c53cb3614e5183e95fe1440e9 100644 (file)
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1243,8 +1243,9 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
        }
        else
        {
-           text       *t;
+           const char *name;
            const char *value;
+           text       *t;
            Size        len;
 
            /*
@@ -1291,11 +1292,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 =
@@ -1303,7 +1312,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,
@@ -1313,11 +1322,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 c14e038d54f1491a9397a2918e535bc200340d60..8d2d7431544627ee537a6ba3179b0d6574a7b29a 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,
This is the main PostgreSQL git repository.
close