Skip to content

Commit 6a0d63d

Browse files
tglsfdctglsfdc
committedJan 3, 2016
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 2917155 commit 6a0d63d

File tree

2 files changed

+126
-36
lines changed

2 files changed

+126
-36
lines changed
 

‎src/bin/pg_dump/pg_dump.c

Lines changed: 125 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,9 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
265265
const char *objlabel);
266266
static const char *getAttrName(int attrnum, TableInfo *tblInfo);
267267
static const char *fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer);
268+
static bool nonemptyReloptions(const char *reloptions);
269+
static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer,
270+
const char *reloptions, const char *prefix);
268271
static char *get_synchronized_snapshot(Archive *fout);
269272
static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query);
270273
static void setupDumpWorker(Archive *AHX, RestoreOptions *ropt);
@@ -4295,8 +4298,8 @@ getTables(Archive *fout, int *numTables)
42954298
"d.refobjid AS owning_tab, "
42964299
"d.refobjsubid AS owning_col, "
42974300
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4298-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4299-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4301+
"c.reloptions AS reloptions, "
4302+
"tc.reloptions AS toast_reloptions "
43004303
"FROM pg_class c "
43014304
"LEFT JOIN pg_depend d ON "
43024305
"(c.relkind = '%c' AND "
@@ -4333,8 +4336,8 @@ getTables(Archive *fout, int *numTables)
43334336
"d.refobjid AS owning_tab, "
43344337
"d.refobjsubid AS owning_col, "
43354338
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4336-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4337-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4339+
"c.reloptions AS reloptions, "
4340+
"tc.reloptions AS toast_reloptions "
43384341
"FROM pg_class c "
43394342
"LEFT JOIN pg_depend d ON "
43404343
"(c.relkind = '%c' AND "
@@ -4371,8 +4374,8 @@ getTables(Archive *fout, int *numTables)
43714374
"d.refobjid AS owning_tab, "
43724375
"d.refobjsubid AS owning_col, "
43734376
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4374-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4375-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4377+
"c.reloptions AS reloptions, "
4378+
"tc.reloptions AS toast_reloptions "
43764379
"FROM pg_class c "
43774380
"LEFT JOIN pg_depend d ON "
43784381
"(c.relkind = '%c' AND "
@@ -4408,8 +4411,8 @@ getTables(Archive *fout, int *numTables)
44084411
"d.refobjid AS owning_tab, "
44094412
"d.refobjsubid AS owning_col, "
44104413
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4411-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4412-
"array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions "
4414+
"c.reloptions AS reloptions, "
4415+
"tc.reloptions AS toast_reloptions "
44134416
"FROM pg_class c "
44144417
"LEFT JOIN pg_depend d ON "
44154418
"(c.relkind = '%c' AND "
@@ -4445,7 +4448,7 @@ getTables(Archive *fout, int *numTables)
44454448
"d.refobjid AS owning_tab, "
44464449
"d.refobjsubid AS owning_col, "
44474450
"(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, "
4448-
"array_to_string(c.reloptions, ', ') AS reloptions, "
4451+
"c.reloptions AS reloptions, "
44494452
"NULL AS toast_reloptions "
44504453
"FROM pg_class c "
44514454
"LEFT JOIN pg_depend d ON "
@@ -4896,7 +4899,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
48964899
i_conoid,
48974900
i_condef,
48984901
i_tablespace,
4899-
i_options,
4902+
i_indreloptions,
49004903
i_relpages;
49014904
int ntups;
49024905

@@ -4953,7 +4956,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
49534956
"c.oid AS conoid, "
49544957
"pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, "
49554958
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
4956-
"array_to_string(t.reloptions, ', ') AS options "
4959+
"t.reloptions AS indreloptions "
49574960
"FROM pg_catalog.pg_index i "
49584961
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
49594962
"LEFT JOIN pg_catalog.pg_constraint c "
@@ -4980,7 +4983,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
49804983
"c.oid AS conoid, "
49814984
"null AS condef, "
49824985
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
4983-
"array_to_string(t.reloptions, ', ') AS options "
4986+
"t.reloptions AS indreloptions "
49844987
"FROM pg_catalog.pg_index i "
49854988
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
49864989
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5010,7 +5013,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
50105013
"c.oid AS conoid, "
50115014
"null AS condef, "
50125015
"(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, "
5013-
"null AS options "
5016+
"null AS indreloptions "
50145017
"FROM pg_catalog.pg_index i "
50155018
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
50165019
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5039,7 +5042,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
50395042
"c.oid AS conoid, "
50405043
"null AS condef, "
50415044
"NULL AS tablespace, "
5042-
"null AS options "
5045+
"null AS indreloptions "
50435046
"FROM pg_catalog.pg_index i "
50445047
"JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) "
50455048
"LEFT JOIN pg_catalog.pg_depend d "
@@ -5071,7 +5074,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
50715074
"t.oid AS conoid, "
50725075
"null AS condef, "
50735076
"NULL AS tablespace, "
5074-
"null AS options "
5077+
"null AS indreloptions "
50755078
"FROM pg_index i, pg_class t "
50765079
"WHERE t.oid = i.indexrelid "
50775080
"AND i.indrelid = '%u'::oid "
@@ -5098,7 +5101,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
50985101
"t.oid AS conoid, "
50995102
"null AS condef, "
51005103
"NULL AS tablespace, "
5101-
"null AS options "
5104+
"null AS indreloptions "
51025105
"FROM pg_index i, pg_class t "
51035106
"WHERE t.oid = i.indexrelid "
51045107
"AND i.indrelid = '%u'::oid "
@@ -5126,7 +5129,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
51265129
i_conoid = PQfnumber(res, "conoid");
51275130
i_condef = PQfnumber(res, "condef");
51285131
i_tablespace = PQfnumber(res, "tablespace");
5129-
i_options = PQfnumber(res, "options");
5132+
i_indreloptions = PQfnumber(res, "indreloptions");
51305133

51315134
indxinfo = (IndxInfo *) pg_malloc(ntups * sizeof(IndxInfo));
51325135
constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo));
@@ -5145,7 +5148,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
51455148
indxinfo[j].indexdef = pg_strdup(PQgetvalue(res, j, i_indexdef));
51465149
indxinfo[j].indnkeys = atoi(PQgetvalue(res, j, i_indnkeys));
51475150
indxinfo[j].tablespace = pg_strdup(PQgetvalue(res, j, i_tablespace));
5148-
indxinfo[j].options = pg_strdup(PQgetvalue(res, j, i_options));
5151+
indxinfo[j].indreloptions = pg_strdup(PQgetvalue(res, j, i_indreloptions));
51495152

51505153
/*
51515154
* In pre-7.4 releases, indkeys may contain more entries than
@@ -12896,8 +12899,12 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1289612899
tbinfo->dobj.catId.oid, false);
1289712900

1289812901
appendPQExpBuffer(q, "CREATE VIEW %s", fmtId(tbinfo->dobj.name));
12899-
if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
12900-
appendPQExpBuffer(q, " WITH (%s)", tbinfo->reloptions);
12902+
if (nonemptyReloptions(tbinfo->reloptions))
12903+
{
12904+
appendPQExpBufferStr(q, " WITH (");
12905+
fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
12906+
appendPQExpBufferChar(q, ')');
12907+
}
1290112908
result = createViewAsClause(fout, tbinfo);
1290212909
appendPQExpBuffer(q, " AS\n%s;\n", result->data);
1290312910
destroyPQExpBuffer(result);
@@ -13140,23 +13147,24 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo)
1314013147
appendPQExpBuffer(q, "\nSERVER %s", fmtId(srvname));
1314113148
}
1314213149

13143-
if ((tbinfo->reloptions && strlen(tbinfo->reloptions) > 0) ||
13144-
(tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0))
13150+
if (nonemptyReloptions(tbinfo->reloptions) ||
13151+
nonemptyReloptions(tbinfo->toast_reloptions))
1314513152
{
1314613153
bool addcomma = false;
1314713154

13148-
appendPQExpBuffer(q, "\nWITH (");
13149-
if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0)
13155+
appendPQExpBufferStr(q, "\nWITH (");
13156+
if (nonemptyReloptions(tbinfo->reloptions))
1315013157
{
1315113158
addcomma = true;
13152-
appendPQExpBuffer(q, "%s", tbinfo->reloptions);
13159+
fmtReloptionsArray(fout, q, tbinfo->reloptions, "");
1315313160
}
13154-
if (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0)
13161+
if (nonemptyReloptions(tbinfo->toast_reloptions))
1315513162
{
13156-
appendPQExpBuffer(q, "%s%s", addcomma ? ", " : "",
13157-
tbinfo->toast_reloptions);
13163+
if (addcomma)
13164+
appendPQExpBufferStr(q, ", ");
13165+
fmtReloptionsArray(fout, q, tbinfo->toast_reloptions, "toast.");
1315813166
}
13159-
appendPQExpBuffer(q, ")");
13167+
appendPQExpBufferChar(q, ')');
1316013168
}
1316113169

1316213170
/* Dump generic options if any */
@@ -13697,8 +13705,12 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
1369713705

1369813706
appendPQExpBuffer(q, ")");
1369913707

13700-
if (indxinfo->options && strlen(indxinfo->options) > 0)
13701-
appendPQExpBuffer(q, " WITH (%s)", indxinfo->options);
13708+
if (nonemptyReloptions(indxinfo->indreloptions))
13709+
{
13710+
appendPQExpBufferStr(q, " WITH (");
13711+
fmtReloptionsArray(fout, q, indxinfo->indreloptions, "");
13712+
appendPQExpBufferChar(q, ')');
13713+
}
1370213714

1370313715
if (coninfo->condeferrable)
1370413716
{
@@ -14558,11 +14570,12 @@ dumpRule(Archive *fout, RuleInfo *rinfo)
1455814570
/*
1455914571
* Apply view's reloptions when its ON SELECT rule is separate.
1456014572
*/
14561-
if (rinfo->reloptions && strlen(rinfo->reloptions) > 0)
14573+
if (nonemptyReloptions(rinfo->reloptions))
1456214574
{
14563-
appendPQExpBuffer(cmd, "ALTER VIEW %s SET (%s);\n",
14564-
fmtId(tbinfo->dobj.name),
14565-
rinfo->reloptions);
14575+
appendPQExpBuffer(cmd, "ALTER VIEW %s SET (",
14576+
fmtId(tbinfo->dobj.name));
14577+
fmtReloptionsArray(fout, cmd, rinfo->reloptions, "");
14578+
appendPQExpBufferStr(cmd, ");\n");
1456614579
}
1456714580

1456814581
/*
@@ -15432,6 +15445,83 @@ fmtCopyColumnList(const TableInfo *ti, PQExpBuffer buffer)
1543215445
return buffer->data;
1543315446
}
1543415447

15448+
/*
15449+
* Check if a reloptions array is nonempty.
15450+
*/
15451+
static bool
15452+
nonemptyReloptions(const char *reloptions)
15453+
{
15454+
/* Don't want to print it if it's just "{}" */
15455+
return (reloptions != NULL && strlen(reloptions) > 2);
15456+
}
15457+
15458+
/*
15459+
* Format a reloptions array and append it to the given buffer.
15460+
*
15461+
* "prefix" is prepended to the option names; typically it's "" or "toast.".
15462+
*
15463+
* Note: this logic should generally match the backend's flatten_reloptions()
15464+
* (in adt/ruleutils.c).
15465+
*/
15466+
static void
15467+
fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, const char *reloptions,
15468+
const char *prefix)
15469+
{
15470+
char **options;
15471+
int noptions;
15472+
int i;
15473+
15474+
if (!parsePGArray(reloptions, &options, &noptions))
15475+
{
15476+
write_msg(NULL, "WARNING: could not parse reloptions array\n");
15477+
if (options)
15478+
free(options);
15479+
return;
15480+
}
15481+
15482+
for (i = 0; i < noptions; i++)
15483+
{
15484+
char *option = options[i];
15485+
char *name;
15486+
char *separator;
15487+
char *value;
15488+
15489+
/*
15490+
* Each array element should have the form name=value. If the "=" is
15491+
* missing for some reason, treat it like an empty value.
15492+
*/
15493+
name = option;
15494+
separator = strchr(option, '=');
15495+
if (separator)
15496+
{
15497+
*separator = '\0';
15498+
value = separator + 1;
15499+
}
15500+
else
15501+
value = "";
15502+
15503+
if (i > 0)
15504+
appendPQExpBufferStr(buffer, ", ");
15505+
appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name));
15506+
15507+
/*
15508+
* In general we need to quote the value; but to avoid unnecessary
15509+
* clutter, do not quote if it is an identifier that would not need
15510+
* quoting. (We could also allow numbers, but that is a bit trickier
15511+
* than it looks --- for example, are leading zeroes significant? We
15512+
* don't want to assume very much here about what custom reloptions
15513+
* might mean.)
15514+
*/
15515+
if (strcmp(fmtId(value), value) == 0)
15516+
appendPQExpBufferStr(buffer, value);
15517+
else
15518+
appendStringLiteralAH(buffer, value, fout);
15519+
}
15520+
15521+
if (options)
15522+
free(options);
15523+
}
15524+
1543515525
/*
1543615526
* Execute an SQL query and verify that we got exactly one row back.
1543715527
*/

‎src/bin/pg_dump/pg_dump.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ typedef struct _indxInfo
312312
TableInfo *indextable; /* link to table the index is for */
313313
char *indexdef;
314314
char *tablespace; /* tablespace in which index is stored */
315-
char *options; /* options specified by WITH (...) */
315+
char *indreloptions; /* options specified by WITH (...) */
316316
int indnkeys;
317317
Oid *indkeys;
318318
bool indisclustered;

0 commit comments

Comments
 (0)