Skip to content

Commit eb42346

Browse files
tglsfdctglsfdc
committed
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
1 parent d4556f5 commit eb42346

File tree

4 files changed

+33
-6
lines changed

4 files changed

+33
-6
lines changed

‎contrib/file_fdw/input/file_fdw.source

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ CREATE USER MAPPING FOR regress_file_fdw_superuser SERVER file_server;
3636
CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
3737

3838
-- validator tests
39+
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (foo 'bar'); -- ERROR
40+
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS ("a=b" 'true'); -- ERROR
3941
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml'); -- ERROR
4042
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', header 'true'); -- ERROR
4143
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':'); -- ERROR

‎contrib/file_fdw/output/file_fdw.source

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ SET ROLE regress_file_fdw_superuser;
3131
CREATE USER MAPPING FOR regress_file_fdw_superuser SERVER file_server;
3232
CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
3333
-- validator tests
34+
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (foo 'bar'); -- ERROR
35+
ERROR: invalid option "foo"
36+
HINT: Valid options in this context are: filename, program, format, header, delimiter, quote, escape, null, encoding
37+
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS ("a=b" 'true'); -- ERROR
38+
ERROR: invalid option name "a=b": must not contain "="
3439
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml'); -- ERROR
3540
ERROR: COPY format "xml" not recognized
3641
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', header 'true'); -- ERROR

‎src/backend/access/common/reloptions.c

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1226,8 +1226,9 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
12261226
}
12271227
else
12281228
{
1229-
text *t;
1229+
const char *name;
12301230
const char *value;
1231+
text *t;
12311232
Size len;
12321233

12331234
/*
@@ -1274,19 +1275,27 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
12741275
* have just "name", assume "name=true" is meant. Note: the
12751276
* namespace is not output.
12761277
*/
1278+
name = def->defname;
12771279
if (def->arg != NULL)
12781280
value = defGetString(def);
12791281
else
12801282
value = "true";
12811283

1284+
/* Insist that name not contain "=", else "a=b=c" is ambiguous */
1285+
if (strchr(name, '=') != NULL)
1286+
ereport(ERROR,
1287+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
1288+
errmsg("invalid option name \"%s\": must not contain \"=\"",
1289+
name)));
1290+
12821291
/*
12831292
* This is not a great place for this test, but there's no other
12841293
* convenient place to filter the option out. As WITH (oids =
12851294
* false) will be removed someday, this seems like an acceptable
12861295
* amount of ugly.
12871296
*/
12881297
if (acceptOidsOff && def->defnamespace == NULL &&
1289-
strcmp(def->defname, "oids") == 0)
1298+
strcmp(name, "oids") == 0)
12901299
{
12911300
if (defGetBoolean(def))
12921301
ereport(ERROR,
@@ -1296,11 +1305,11 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
12961305
continue;
12971306
}
12981307

1299-
len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
1308+
len = VARHDRSZ + strlen(name) + 1 + strlen(value);
13001309
/* +1 leaves room for sprintf's trailing null */
13011310
t = (text *) palloc(len + 1);
13021311
SET_VARSIZE(t, len);
1303-
sprintf(VARDATA(t), "%s=%s", def->defname, value);
1312+
sprintf(VARDATA(t), "%s=%s", name, value);
13041313

13051314
astate = accumArrayResult(astate, PointerGetDatum(t),
13061315
false, TEXTOID,

‎src/backend/commands/foreigncmds.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,26 @@ optionListToArray(List *options)
7171
foreach(cell, options)
7272
{
7373
DefElem *def = lfirst(cell);
74+
const char *name;
7475
const char *value;
7576
Size len;
7677
text *t;
7778

79+
name = def->defname;
7880
value = defGetString(def);
79-
len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value);
81+
82+
/* Insist that name not contain "=", else "a=b=c" is ambiguous */
83+
if (strchr(name, '=') != NULL)
84+
ereport(ERROR,
85+
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
86+
errmsg("invalid option name \"%s\": must not contain \"=\"",
87+
name)));
88+
89+
len = VARHDRSZ + strlen(name) + 1 + strlen(value);
90+
/* +1 leaves room for sprintf's trailing null */
8091
t = palloc(len + 1);
8192
SET_VARSIZE(t, len);
82-
sprintf(VARDATA(t), "%s=%s", def->defname, value);
93+
sprintf(VARDATA(t), "%s=%s", name, value);
8394

8495
astate = accumArrayResult(astate, PointerGetDatum(t),
8596
false, TEXTOID,

0 commit comments

Comments
 (0)