Re: tighten generic_option_name, or store more carefully in catalog? - Mailing list pgsql-hackers

Here's a proposed patch for the "=" issue.  Whether or not we should
rethink FDW validation behavior, doing so surely couldn't be
back-patched.  But I think this much should be.

            regards, tom lane

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 46c1dce222d..50747c16396 100644
--- 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 c14e038d54f..8d2d7431544 100644
--- 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,