Skip to content

Commit aab4b73

Browse files
tglsfdctglsfdc
committed
Teach pg_dump to quote reloption values safely.
Commit c7e27be fixed this on the backend side, but we neglected the fact that several code paths in pg_dump were printing reloptions values that had not gotten massaged by ruleutils. Apply essentially the same quoting logic in those places, too.
1 parent 1cd3840 commit aab4b73

File tree

2 files changed

+128
-38
lines changed

2 files changed

+128
-38
lines changed

‎src/bin/pg_dump/pg_dump.c

Lines changed: 126 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
263263
const char *objlabel);
264264
static const char *getAttrName(int attrnum, TableInfo *tblInfo);
265265
static const char *fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer);
266+
static bool nonemptyReloptions(const char *reloptions);
267+
static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer,
268+
const char *reloptions, const char *prefix);
266269
static char *get_synchronized_snapshot(Archive *fout);
267270
static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query);
268271
static void setupDumpWorker(Archive *AHX, RestoreOptions *ropt);
@@ -4336,10 +4339,10 @@ getTables(Archive *fout, int *numTables)
43364339
"d.refobjid AS owning_tab, "
43374340
"d.refobjsubid AS owning_col, "
43384341
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4339-
"array_to_string(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
4342+
"array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
43404343
"CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
43414344
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
4342-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4345+
"tc.reloptions AS toast_reloptions "
43434346
"FROM pg_class c "
43444347
"LEFT JOIN pg_depend d ON "
43454348
"(c.relkind = '%c' AND "
@@ -4376,10 +4379,10 @@ getTables(Archive *fout, int *numTables)
43764379
"d.refobjid AS owning_tab, "
43774380
"d.refobjsubid AS owning_col, "
43784381
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4379-
"array_to_string(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
4382+
"array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
43804383
"CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
43814384
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
4382-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4385+
"tc.reloptions AS toast_reloptions "
43834386
"FROM pg_class c "
43844387
"LEFT JOIN pg_depend d ON "
43854388
"(c.relkind = '%c' AND "
@@ -4416,8 +4419,8 @@ getTables(Archive *fout, int *numTables)
44164419
"d.refobjid AS owning_tab, "
44174420
"d.refobjsubid AS owning_col, "
44184421
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4419-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4420-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4422+
"c.reloptions AS reloptions, "
4423+
"tc.reloptions AS toast_reloptions "
44214424
"FROM pg_class c "
44224425
"LEFT JOIN pg_depend d ON "
44234426
"(c.relkind = '%c' AND "
@@ -4454,8 +4457,8 @@ getTables(Archive *fout, int *numTables)
44544457
"d.refobjid AS owning_tab, "
44554458
"d.refobjsubid AS owning_col, "
44564459
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4457-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4458-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4460+
"c.reloptions AS reloptions, "
4461+
"tc.reloptions AS toast_reloptions "
44594462
"FROM pg_class c "
44604463
"LEFT JOIN pg_depend d ON "
44614464
"(c.relkind = '%c' AND "
@@ -4491,8 +4494,8 @@ getTables(Archive *fout, int *numTables)
44914494
"d.refobjid AS owning_tab, "
44924495
"d.refobjsubid AS owning_col, "
44934496
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4494-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4495-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4497+
"c.reloptions AS reloptions, "
4498+
"tc.reloptions AS toast_reloptions "
44964499
"FROM pg_class c "
44974500
"LEFT JOIN pg_depend d ON "
44984501
"(c.relkind = '%c' AND "
@@ -4528,7 +4531,7 @@ getTables(Archive *fout, int *numTables)
45284531
"d.refobjid AS owning_tab, "
45294532
"d.refobjsubid AS owning_col, "
45304533
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4531-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4534+
"c.reloptions AS reloptions, "
45324535
"NULL AS toast_reloptions "
45334536
"FROM pg_class c "
45344537
"LEFT JOIN pg_depend d ON "
@@ -4987,7 +4990,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
49874990
i_conoid,
49884991
i_condef,
49894992
i_tablespace,
4990-
i_options,
4993+
i_indreloptions,
49914994
i_relpages;
49924995
int ntups;
49934996

@@ -5044,7 +5047,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
50445047
"c.oid AS conoid, "
50455048
"pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, "
50465049
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5047-
"array_to_string(t.reloptions, ', ') AS options "
5050+
"t.reloptions AS indreloptions "
50485051
"FROM pg_catalog.pg_index i "
50495052
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
50505053
"LEFT JOIN pg_catalog.pg_constraint c "
@@ -5075,7 +5078,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
50755078
"c.oid AS conoid, "
50765079
"pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, "
50775080
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5078-
"array_to_string(t.reloptions, ', ') AS options "
5081+
"t.reloptions AS indreloptions "
50795082
"FROM pg_catalog.pg_index i "
50805083
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
50815084
"LEFT JOIN pg_catalog.pg_constraint c "
@@ -5102,7 +5105,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
51025105
"c.oid AS conoid, "
51035106
"null AS condef, "
51045107
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5105-
"array_to_string(t.reloptions, ', ') AS options "
5108+
"t.reloptions AS indreloptions "
51065109
"FROM pg_catalog.pg_index i "
51075110
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
51085111
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5132,7 +5135,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
51325135
"c.oid AS conoid, "
51335136
"null AS condef, "
51345137
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5135-
"null AS options "
5138+
"null AS indreloptions "
51365139
"FROM pg_catalog.pg_index i "
51375140
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
51385141
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5161,7 +5164,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
51615164
"c.oid AS conoid, "
51625165
"null AS condef, "
51635166
"NULL AS tablespace, "
5164-
"null AS options "
5167+
"null AS indreloptions "
51655168
"FROM pg_catalog.pg_index i "
51665169
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
51675170
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5193,7 +5196,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
51935196
"t.oid AS conoid, "
51945197
"null AS condef, "
51955198
"NULL AS tablespace, "
5196-
"null AS options "
5199+
"null AS indreloptions "
51975200
"FROM pg_index i, pg_class t "
51985201
"WHERE t.oid = i.indexrelid "
51995202
"AND i.indrelid = '%u'::oid "
@@ -5220,7 +5223,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
52205223
"t.oid AS conoid, "
52215224
"null AS condef, "
52225225
"NULL AS tablespace, "
5223-
"null AS options "
5226+
"null AS indreloptions "
52245227
"FROM pg_index i, pg_class t "
52255228
"WHERE t.oid = i.indexrelid "
52265229
"AND i.indrelid = '%u'::oid "
@@ -5249,7 +5252,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
52495252
i_conoid = PQfnumber(res, "conoid");
52505253
i_condef = PQfnumber(res, "condef");
52515254
i_tablespace = PQfnumber(res, "tablespace");
5252-
i_options = PQfnumber(res, "options");
5255+
i_indreloptions = PQfnumber(res, "indreloptions");
52535256

52545257
indxinfo = (IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo));
52555258
constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
@@ -5268,7 +5271,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
52685271
indxinfo[j].indexdef = pg_strdup(PQgetvalue(res, j, i_indexdef));
52695272
indxinfo[j].indnkeys = atoi(PQgetvalue(res, j, i_indnkeys));
52705273
indxinfo[j].tablespace = pg_strdup(PQgetvalue(res, j, i_tablespace));
5271-
indxinfo[j].options = pg_strdup(PQgetvalue(res, j, i_options));
5274+
indxinfo[j].indreloptions = pg_strdup(PQgetvalue(res, j, i_indreloptions));
52725275

52735276
/*
52745277
* In pre-7.4 releases, indkeys may contain more entries than
@@ -13201,8 +13204,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1320113204
tbinfo->dobj.catId.oid, false);
1320213205

1320313206
appendPQExpBuffer(q, "CREATE VIEW %s", fmtId(tbinfo->dobj.name));
13204-
if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
13205-
appendPQExpBuffer(q, " WITH (%s)", tbinfo->reloptions);
13207+
if (nonemptyReloptions(tbinfo->reloptions))
13208+
{
13209+
appendPQExpBufferStr(q, " WITH (");
13210+
fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
13211+
appendPQExpBufferChar(q, ')');
13212+
}
1320613213
result = createViewAsClause(fout, tbinfo);
1320713214
appendPQExpBuffer(q, " AS\n%s", result->data);
1320813215
destroyPQExpBuffer(result);
@@ -13446,21 +13453,22 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1344613453
appendPQExpBuffer(q, "\nSERVER %s", fmtId(srvname));
1344713454
}
1344813455

13449-
if ((tbinfo->reloptions && strlen(tbinfo->reloptions) > 0) ||
13450-
(tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0))
13456+
if (nonemptyReloptions(tbinfo->reloptions) ||
13457+
nonemptyReloptions(tbinfo->toast_reloptions))
1345113458
{
1345213459
bool addcomma = false;
1345313460

1345413461
appendPQExpBufferStr(q, "\nWITH (");
13455-
if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
13462+
if (nonemptyReloptions(tbinfo->reloptions))
1345613463
{
1345713464
addcomma = true;
13458-
appendPQExpBufferStr(q, tbinfo->reloptions);
13465+
fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
1345913466
}
13460-
if (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0)
13467+
if (nonemptyReloptions(tbinfo->toast_reloptions))
1346113468
{
13462-
appendPQExpBuffer(q, "%s%s", addcomma ? ", " : "",
13463-
tbinfo->toast_reloptions);
13469+
if (addcomma)
13470+
appendPQExpBufferStr(q, ", ");
13471+
fmtReloptionsArray(fout, q, tbinfo->toast_reloptions, "toast.");
1346413472
}
1346513473
appendPQExpBufferChar(q, ')');
1346613474
}
@@ -14034,8 +14042,12 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
1403414042

1403514043
appendPQExpBufferChar(q, ')');
1403614044

14037-
if (indxinfo->options && strlen(indxinfo->options) > 0)
14038-
appendPQExpBuffer(q, " WITH (%s)", indxinfo->options);
14045+
if (nonemptyReloptions(indxinfo->indreloptions))
14046+
{
14047+
appendPQExpBufferStr(q, " WITH (");
14048+
fmtReloptionsArray(fout, q, indxinfo->indreloptions, "");
14049+
appendPQExpBufferChar(q, ')');
14050+
}
1403914051

1404014052
if (coninfo->condeferrable)
1404114053
{
@@ -14895,11 +14907,12 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
1489514907
/*
1489614908
* Apply view's reloptions when its ON SELECT rule is separate.
1489714909
*/
14898-
if (rinfo->reloptions && strlen(rinfo->reloptions) > 0)
14910+
if (nonemptyReloptions(rinfo->reloptions))
1489914911
{
14900-
appendPQExpBuffer(cmd, "ALTER VIEW %s SET (%s);\n",
14901-
fmtId(tbinfo->dobj.name),
14902-
rinfo->reloptions);
14912+
appendPQExpBuffer(cmd, "ALTER VIEW %s SET (",
14913+
fmtId(tbinfo->dobj.name));
14914+
fmtReloptionsArray(fout, cmd, rinfo->reloptions, "");
14915+
appendPQExpBufferStr(cmd, ");\n");
1490314916
}
1490414917

1490514918
/*
@@ -15769,6 +15782,83 @@ fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer)
1576915782
return buffer->data;
1577015783
}
1577115784

15785+
/*
15786+
* Check if a reloptions array is nonempty.
15787+
*/
15788+
static bool
15789+
nonemptyReloptions(const char *reloptions)
15790+
{
15791+
/* Don't want to print it if it's just "{}" */
15792+
return (reloptions != NULL && strlen(reloptions) > 2);
15793+
}
15794+
15795+
/*
15796+
* Format a reloptions array and append it to the given buffer.
15797+
*
15798+
* "prefix" is prepended to the option names; typically it's "" or "toast.".
15799+
*
15800+
* Note: this logic should generally match the backend's flatten_reloptions()
15801+
* (in adt/ruleutils.c).
15802+
*/
15803+
static void
15804+
fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, const char *reloptions,
15805+
const char *prefix)
15806+
{
15807+
char **options;
15808+
int noptions;
15809+
int i;
15810+
15811+
if (!parsePGArray(reloptions, &options, &noptions))
15812+
{
15813+
write_msg(NULL, "WARNING: could not parse reloptions array\n");
15814+
if (options)
15815+
free(options);
15816+
return;
15817+
}
15818+
15819+
for (i = 0; i < noptions; i++)
15820+
{
15821+
char *option = options[i];
15822+
char *name;
15823+
char *separator;
15824+
char *value;
15825+
15826+
/*
15827+
* Each array element should have the form name=value. If the "=" is
15828+
* missing for some reason, treat it like an empty value.
15829+
*/
15830+
name = option;
15831+
separator = strchr(option, '=');
15832+
if (separator)
15833+
{
15834+
*separator = '\0';
15835+
value = separator + 1;
15836+
}
15837+
else
15838+
value = "";
15839+
15840+
if (i > 0)
15841+
appendPQExpBufferStr(buffer, ", ");
15842+
appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name));
15843+
15844+
/*
15845+
* In general we need to quote the value; but to avoid unnecessary
15846+
* clutter, do not quote if it is an identifier that would not need
15847+
* quoting. (We could also allow numbers, but that is a bit trickier
15848+
* than it looks --- for example, are leading zeroes significant? We
15849+
* don't want to assume very much here about what custom reloptions
15850+
* might mean.)
15851+
*/
15852+
if (strcmp(fmtId(value), value) == 0)
15853+
appendPQExpBufferStr(buffer, value);
15854+
else
15855+
appendStringLiteralAH(buffer, value, fout);
15856+
}
15857+
15858+
if (options)
15859+
free(options);
15860+
}
15861+
1577215862
/*
1577315863
* Execute an SQL query and verify that we got exactly one row back.
1577415864
*/

‎src/bin/pg_dump/pg_dump.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ typedef struct _tableInfo
239239
char relreplident; /* replica identifier */
240240
char *reltablespace; /* relation tablespace */
241241
char *reloptions; /* options specified by WITH (...) */
242-
char *checkoption; /* WITH CHECK OPTION */
242+
char *checkoption; /* WITH CHECK OPTION, if any */
243243
char *toast_reloptions; /* WITH options for the TOAST table */
244244
bool hasindex; /* does it have any indexes? */
245245
bool hasrules; /* does it have any rules? */
@@ -314,7 +314,7 @@ typedef struct _indxInfo
314314
TableInfo *indextable; /* link to table the index is for */
315315
char *indexdef;
316316
char *tablespace; /* tablespace in which index is stored */
317-
char *options; /* options specified by WITH (...) */
317+
char *indreloptions; /* options specified by WITH (...) */
318318
int indnkeys;
319319
Oid *indkeys;
320320
bool indisclustered;

0 commit comments

Comments
 (0)