1
0
mirror of https://github.com/postgres/postgres.git synced 2025-06-08 22:02:03 +03:00

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
This commit is contained in:
Tom Lane 2025-06-02 15:22:44 -04:00
parent 31a7e175fd
commit aa87f69c00
4 changed files with 32 additions and 6 deletions

View File

@ -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_file_fdw_superuser SERVER file_server;
CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server; CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
-- validator tests -- 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 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'xml'); -- ERROR
ERROR: COPY format "xml" not recognized ERROR: COPY format "xml" not recognized
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':'); -- ERROR CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':'); -- ERROR

View File

@ -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; CREATE USER MAPPING FOR regress_no_priv_user SERVER file_server;
-- validator tests -- 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 'xml'); -- ERROR
CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', quote ':'); -- 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 CREATE FOREIGN TABLE tbl () SERVER file_server OPTIONS (format 'text', escape ':'); -- ERROR

View File

@ -1243,8 +1243,9 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
} }
else else
{ {
text *t; const char *name;
const char *value; const char *value;
text *t;
Size len; Size len;
/* /*
@ -1291,11 +1292,19 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
* have just "name", assume "name=true" is meant. Note: the * have just "name", assume "name=true" is meant. Note: the
* namespace is not output. * namespace is not output.
*/ */
name = def->defname;
if (def->arg != NULL) if (def->arg != NULL)
value = defGetString(def); value = defGetString(def);
else else
value = "true"; 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 * This is not a great place for this test, but there's no other
* convenient place to filter the option out. As WITH (oids = * 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. * amount of ugly.
*/ */
if (acceptOidsOff && def->defnamespace == NULL && if (acceptOidsOff && def->defnamespace == NULL &&
strcmp(def->defname, "oids") == 0) strcmp(name, "oids") == 0)
{ {
if (defGetBoolean(def)) if (defGetBoolean(def))
ereport(ERROR, ereport(ERROR,
@ -1313,11 +1322,11 @@ transformRelOptions(Datum oldOptions, List *defList, const char *namspace,
continue; continue;
} }
len = VARHDRSZ + strlen(def->defname) + 1 + strlen(value); len = VARHDRSZ + strlen(name) + 1 + strlen(value);
/* +1 leaves room for sprintf's trailing null */ /* +1 leaves room for sprintf's trailing null */
t = (text *) palloc(len + 1); t = (text *) palloc(len + 1);
SET_VARSIZE(t, len); SET_VARSIZE(t, len);
sprintf(VARDATA(t), "%s=%s", def->defname, value); sprintf(VARDATA(t), "%s=%s", name, value);
astate = accumArrayResult(astate, PointerGetDatum(t), astate = accumArrayResult(astate, PointerGetDatum(t),
false, TEXTOID, false, TEXTOID,

View File

@ -71,15 +71,26 @@ optionListToArray(List *options)
foreach(cell, options) foreach(cell, options)
{ {
DefElem *def = lfirst(cell); DefElem *def = lfirst(cell);
const char *name;
const char *value; const char *value;
Size len; Size len;
text *t; text *t;
name = def->defname;
value = defGetString(def); 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); t = palloc(len + 1);
SET_VARSIZE(t, len); SET_VARSIZE(t, len);
sprintf(VARDATA(t), "%s=%s", def->defname, value); sprintf(VARDATA(t), "%s=%s", name, value);
astate = accumArrayResult(astate, PointerGetDatum(t), astate = accumArrayResult(astate, PointerGetDatum(t),
false, TEXTOID, false, TEXTOID,