From: Tom Lane <tgl@sss.pgh.pa.us>
To: Chapman Flack <jcflack@acm.org>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: tighten generic_option_name, or store more carefully in catalog?
Date: Mon, 02 Jun 2025 14:13:43 -0400
Message-ID: <557970.1748888023@sss.pgh.pa.us> (raw)
In-Reply-To: <683BBE52.8050900@acm.org>
References: <6830EB30.8090904@acm.org>
<3020890.1748735715@sss.pgh.pa.us>
<683BA390.7000903@acm.org>
<3029560.1748740373@sss.pgh.pa.us>
<683BBE52.8050900@acm.org>
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
Attachments:v1-disallow-equal-in-option-names.patch (text/x-diff, 3.1K) downloaddiff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.cindex 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.cindex 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,
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, jcflack@acm.org
Subject: Re: tighten generic_option_name, or store more carefully in catalog?
In-Reply-To: <557970.1748888023@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox