Skip to content

Commit b416c0b

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 7157fe8 commit b416c0b

File tree

2 files changed

+132
-41
lines changed

2 files changed

+132
-41
lines changed

‎src/bin/pg_dump/pg_dump.c‎

Lines changed: 130 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,9 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
256256
const char *objlabel);
257257
static const char *getAttrName(int attrnum, TableInfo *tblInfo);
258258
static const char *fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer);
259+
static bool nonemptyReloptions(const char *reloptions);
260+
static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer,
261+
const char *reloptions, const char *prefix);
259262
static char *get_synchronized_snapshot(Archive *fout);
260263
static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query);
261264
static void setupDumpWorker(Archive *AHX, DumpOptions *dopt, RestoreOptions *ropt);
@@ -4604,10 +4607,10 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
46044607
"d.refobjid AS owning_tab, "
46054608
"d.refobjsubid AS owning_col, "
46064609
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4607-
"array_to_string(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
4610+
"array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
46084611
"CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
46094612
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
4610-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4613+
"tc.reloptions AS toast_reloptions "
46114614
"FROM pg_class c "
46124615
"LEFT JOIN pg_depend d ON "
46134616
"(c.relkind = '%c' AND "
@@ -4646,10 +4649,10 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
46464649
"d.refobjid AS owning_tab, "
46474650
"d.refobjsubid AS owning_col, "
46484651
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4649-
"array_to_string(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
4652+
"array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
46504653
"CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
46514654
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
4652-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4655+
"tc.reloptions AS toast_reloptions "
46534656
"FROM pg_class c "
46544657
"LEFT JOIN pg_depend d ON "
46554658
"(c.relkind = '%c' AND "
@@ -4688,10 +4691,10 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
46884691
"d.refobjid AS owning_tab, "
46894692
"d.refobjsubid AS owning_col, "
46904693
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4691-
"array_to_string(array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded'), ', ') AS reloptions, "
4694+
"array_remove(array_remove(c.reloptions,'check_option=local'),'check_option=cascaded') AS reloptions, "
46924695
"CASE WHEN 'check_option=local' = ANY (c.reloptions) THEN 'LOCAL'::text "
46934696
"WHEN 'check_option=cascaded' = ANY (c.reloptions) THEN 'CASCADED'::text ELSE NULL END AS checkoption, "
4694-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4697+
"tc.reloptions AS toast_reloptions "
46954698
"FROM pg_class c "
46964699
"LEFT JOIN pg_depend d ON "
46974700
"(c.relkind = '%c' AND "
@@ -4730,8 +4733,8 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
47304733
"d.refobjid AS owning_tab, "
47314734
"d.refobjsubid AS owning_col, "
47324735
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4733-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4734-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4736+
"c.reloptions AS reloptions, "
4737+
"tc.reloptions AS toast_reloptions "
47354738
"FROM pg_class c "
47364739
"LEFT JOIN pg_depend d ON "
47374740
"(c.relkind = '%c' AND "
@@ -4770,8 +4773,8 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
47704773
"d.refobjid AS owning_tab, "
47714774
"d.refobjsubid AS owning_col, "
47724775
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4773-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4774-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4776+
"c.reloptions AS reloptions, "
4777+
"tc.reloptions AS toast_reloptions "
47754778
"FROM pg_class c "
47764779
"LEFT JOIN pg_depend d ON "
47774780
"(c.relkind = '%c' AND "
@@ -4809,8 +4812,8 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
48094812
"d.refobjid AS owning_tab, "
48104813
"d.refobjsubid AS owning_col, "
48114814
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4812-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4813-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4815+
"c.reloptions AS reloptions, "
4816+
"tc.reloptions AS toast_reloptions "
48144817
"FROM pg_class c "
48154818
"LEFT JOIN pg_depend d ON "
48164819
"(c.relkind = '%c' AND "
@@ -4848,7 +4851,7 @@ getTables(Archive *fout, DumpOptions *dopt, int *numTables)
48484851
"d.refobjid AS owning_tab, "
48494852
"d.refobjsubid AS owning_col, "
48504853
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4851-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4854+
"c.reloptions AS reloptions, "
48524855
"NULL AS toast_reloptions "
48534856
"FROM pg_class c "
48544857
"LEFT JOIN pg_depend d ON "
@@ -5321,7 +5324,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
53215324
i_conoid,
53225325
i_condef,
53235326
i_tablespace,
5324-
i_options,
5327+
i_indreloptions,
53255328
i_relpages;
53265329
int ntups;
53275330

@@ -5379,7 +5382,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
53795382
"c.oid AS conoid, "
53805383
"pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, "
53815384
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5382-
"array_to_string(t.reloptions, ', ') AS options "
5385+
"t.reloptions AS indreloptions "
53835386
"FROM pg_catalog.pg_index i "
53845387
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
53855388
"LEFT JOIN pg_catalog.pg_constraint c "
@@ -5410,7 +5413,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
54105413
"c.oid AS conoid, "
54115414
"pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, "
54125415
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5413-
"array_to_string(t.reloptions, ', ') AS options "
5416+
"t.reloptions AS indreloptions "
54145417
"FROM pg_catalog.pg_index i "
54155418
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
54165419
"LEFT JOIN pg_catalog.pg_constraint c "
@@ -5437,7 +5440,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
54375440
"c.oid AS conoid, "
54385441
"null AS condef, "
54395442
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5440-
"array_to_string(t.reloptions, ', ') AS options "
5443+
"t.reloptions AS indreloptions "
54415444
"FROM pg_catalog.pg_index i "
54425445
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
54435446
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5467,7 +5470,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
54675470
"c.oid AS conoid, "
54685471
"null AS condef, "
54695472
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5470-
"null AS options "
5473+
"null AS indreloptions "
54715474
"FROM pg_catalog.pg_index i "
54725475
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
54735476
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5496,7 +5499,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
54965499
"c.oid AS conoid, "
54975500
"null AS condef, "
54985501
"NULL AS tablespace, "
5499-
"null AS options "
5502+
"null AS indreloptions "
55005503
"FROM pg_catalog.pg_index i "
55015504
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
55025505
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5528,7 +5531,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
55285531
"t.oid AS conoid, "
55295532
"null AS condef, "
55305533
"NULL AS tablespace, "
5531-
"null AS options "
5534+
"null AS indreloptions "
55325535
"FROM pg_index i, pg_class t "
55335536
"WHERE t.oid = i.indexrelid "
55345537
"AND i.indrelid = '%u'::oid "
@@ -5555,7 +5558,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
55555558
"t.oid AS conoid, "
55565559
"null AS condef, "
55575560
"NULL AS tablespace, "
5558-
"null AS options "
5561+
"null AS indreloptions "
55595562
"FROM pg_index i, pg_class t "
55605563
"WHERE t.oid = i.indexrelid "
55615564
"AND i.indrelid = '%u'::oid "
@@ -5584,7 +5587,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
55845587
i_conoid = PQfnumber(res, "conoid");
55855588
i_condef = PQfnumber(res, "condef");
55865589
i_tablespace = PQfnumber(res, "tablespace");
5587-
i_options = PQfnumber(res, "options");
5590+
i_indreloptions = PQfnumber(res, "indreloptions");
55885591

55895592
indxinfo = (IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo));
55905593
constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
@@ -5603,7 +5606,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
56035606
indxinfo[j].indexdef = pg_strdup(PQgetvalue(res, j, i_indexdef));
56045607
indxinfo[j].indnkeys = atoi(PQgetvalue(res, j, i_indnkeys));
56055608
indxinfo[j].tablespace = pg_strdup(PQgetvalue(res, j, i_tablespace));
5606-
indxinfo[j].options = pg_strdup(PQgetvalue(res, j, i_options));
5609+
indxinfo[j].indreloptions = pg_strdup(PQgetvalue(res, j, i_indreloptions));
56075610

56085611
/*
56095612
* In pre-7.4 releases, indkeys may contain more entries than
@@ -10255,7 +10258,8 @@ dumpFunc(Archive *fout, DumpOptions *dopt, FuncInfo *finfo)
1025510258
"pg_catalog.pg_get_function_result(oid) AS funcresult, "
1025610259
"array_to_string(protrftypes, ' ') AS protrftypes, "
1025710260
"proiswindow, provolatile, proisstrict, prosecdef, "
10258-
"proleakproof, proconfig, procost, prorows, proparallel, "
10261+
"proleakproof, proconfig, procost, prorows, "
10262+
"proparallel, "
1025910263
"(SELECT lanname FROM pg_catalog.pg_language WHERE oid = prolang) AS lanname "
1026010264
"FROM pg_catalog.pg_proc "
1026110265
"WHERE oid = '%u'::pg_catalog.oid",
@@ -13869,8 +13873,12 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
1386913873
tbinfo->dobj.catId.oid, false);
1387013874

1387113875
appendPQExpBuffer(q, "CREATE VIEW %s", fmtId(tbinfo->dobj.name));
13872-
if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
13873-
appendPQExpBuffer(q, " WITH (%s)", tbinfo->reloptions);
13876+
if (nonemptyReloptions(tbinfo->reloptions))
13877+
{
13878+
appendPQExpBufferStr(q, " WITH (");
13879+
fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
13880+
appendPQExpBufferChar(q, ')');
13881+
}
1387413882
result = createViewAsClause(fout, tbinfo);
1387513883
appendPQExpBuffer(q, " AS\n%s", result->data);
1387613884
destroyPQExpBuffer(result);
@@ -14114,21 +14122,22 @@ dumpTableSchema(Archive *fout, DumpOptions *dopt, TableInfo *tbinfo)
1411414122
appendPQExpBuffer(q, "\nSERVER %s", fmtId(srvname));
1411514123
}
1411614124

14117-
if ((tbinfo->reloptions && strlen(tbinfo->reloptions) > 0) ||
14118-
(tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0))
14125+
if (nonemptyReloptions(tbinfo->reloptions) ||
14126+
nonemptyReloptions(tbinfo->toast_reloptions))
1411914127
{
1412014128
bool addcomma = false;
1412114129

1412214130
appendPQExpBufferStr(q, "\nWITH (");
14123-
if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
14131+
if (nonemptyReloptions(tbinfo->reloptions))
1412414132
{
1412514133
addcomma = true;
14126-
appendPQExpBufferStr(q, tbinfo->reloptions);
14134+
fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
1412714135
}
14128-
if (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0)
14136+
if (nonemptyReloptions(tbinfo->toast_reloptions))
1412914137
{
14130-
appendPQExpBuffer(q, "%s%s", addcomma ? ", " : "",
14131-
tbinfo->toast_reloptions);
14138+
if (addcomma)
14139+
appendPQExpBufferStr(q, ", ");
14140+
fmtReloptionsArray(fout, q, tbinfo->toast_reloptions, "toast.");
1413214141
}
1413314142
appendPQExpBufferChar(q, ')');
1413414143
}
@@ -14710,8 +14719,12 @@ dumpConstraint(Archive *fout, DumpOptions *dopt, ConstraintInfo *coninfo)
1471014719

1471114720
appendPQExpBufferChar(q, ')');
1471214721

14713-
if (indxinfo->options && strlen(indxinfo->options) > 0)
14714-
appendPQExpBuffer(q, " WITH (%s)", indxinfo->options);
14722+
if (nonemptyReloptions(indxinfo->indreloptions))
14723+
{
14724+
appendPQExpBufferStr(q, " WITH (");
14725+
fmtReloptionsArray(fout, q, indxinfo->indreloptions, "");
14726+
appendPQExpBufferChar(q, ')');
14727+
}
1471514728

1471614729
if (coninfo->condeferrable)
1471714730
{
@@ -15571,11 +15584,12 @@ dumpRule(Archive *fout, DumpOptions *dopt, RuleInfo *rinfo)
1557115584
/*
1557215585
* Apply view's reloptions when its ON SELECT rule is separate.
1557315586
*/
15574-
if (rinfo->reloptions && strlen(rinfo->reloptions) > 0)
15587+
if (nonemptyReloptions(rinfo->reloptions))
1557515588
{
15576-
appendPQExpBuffer(cmd, "ALTER VIEW %s SET (%s);\n",
15577-
fmtId(tbinfo->dobj.name),
15578-
rinfo->reloptions);
15589+
appendPQExpBuffer(cmd, "ALTER VIEW %s SET (",
15590+
fmtId(tbinfo->dobj.name));
15591+
fmtReloptionsArray(fout, cmd, rinfo->reloptions, "");
15592+
appendPQExpBufferStr(cmd, ");\n");
1557915593
}
1558015594

1558115595
/*
@@ -16448,6 +16462,83 @@ fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer)
1644816462
return buffer->data;
1644916463
}
1645016464

16465+
/*
16466+
* Check if a reloptions array is nonempty.
16467+
*/
16468+
static bool
16469+
nonemptyReloptions(const char *reloptions)
16470+
{
16471+
/* Don't want to print it if it's just "{}" */
16472+
return (reloptions != NULL && strlen(reloptions) > 2);
16473+
}
16474+
16475+
/*
16476+
* Format a reloptions array and append it to the given buffer.
16477+
*
16478+
* "prefix" is prepended to the option names; typically it's "" or "toast.".
16479+
*
16480+
* Note: this logic should generally match the backend's flatten_reloptions()
16481+
* (in adt/ruleutils.c).
16482+
*/
16483+
static void
16484+
fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, const char *reloptions,
16485+
const char *prefix)
16486+
{
16487+
char **options;
16488+
int noptions;
16489+
int i;
16490+
16491+
if (!parsePGArray(reloptions, &options, &noptions))
16492+
{
16493+
write_msg(NULL, "WARNING: could not parse reloptions array\n");
16494+
if (options)
16495+
free(options);
16496+
return;
16497+
}
16498+
16499+
for (i = 0; i < noptions; i++)
16500+
{
16501+
char *option = options[i];
16502+
char *name;
16503+
char *separator;
16504+
char *value;
16505+
16506+
/*
16507+
* Each array element should have the form name=value. If the "=" is
16508+
* missing for some reason, treat it like an empty value.
16509+
*/
16510+
name = option;
16511+
separator = strchr(option, '=');
16512+
if (separator)
16513+
{
16514+
*separator = '\0';
16515+
value = separator + 1;
16516+
}
16517+
else
16518+
value = "";
16519+
16520+
if (i > 0)
16521+
appendPQExpBufferStr(buffer, ", ");
16522+
appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name));
16523+
16524+
/*
16525+
* In general we need to quote the value; but to avoid unnecessary
16526+
* clutter, do not quote if it is an identifier that would not need
16527+
* quoting. (We could also allow numbers, but that is a bit trickier
16528+
* than it looks --- for example, are leading zeroes significant? We
16529+
* don't want to assume very much here about what custom reloptions
16530+
* might mean.)
16531+
*/
16532+
if (strcmp(fmtId(value), value) == 0)
16533+
appendPQExpBufferStr(buffer, value);
16534+
else
16535+
appendStringLiteralAH(buffer, value, fout);
16536+
}
16537+
16538+
if (options)
16539+
free(options);
16540+
}
16541+
1645116542
/*
1645216543
* Execute an SQL query and verify that we got exactly one row back.
1645316544
*/

‎src/bin/pg_dump/pg_dump.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ typedef struct _tableInfo
205205
char relreplident; /* replica identifier */
206206
char *reltablespace; /* relation tablespace */
207207
char *reloptions; /* options specified by WITH (...) */
208-
char *checkoption; /* WITH CHECK OPTION */
208+
char *checkoption; /* WITH CHECK OPTION, if any */
209209
char *toast_reloptions; /* WITH options for the TOAST table */
210210
bool hasindex; /* does it have any indexes? */
211211
bool hasrules; /* does it have any rules? */
@@ -282,7 +282,7 @@ typedef struct _indxInfo
282282
TableInfo *indextable; /* link to table the index is for */
283283
char *indexdef;
284284
char *tablespace; /* tablespace in which index is stored */
285-
char *options; /* options specified by WITH (...) */
285+
char *indreloptions; /* options specified by WITH (...) */
286286
int indnkeys;
287287
Oid *indkeys;
288288
bool indisclustered;

0 commit comments

Comments
 (0)