From e4959fb5cb15f486dcc3489c3e6ddfa37b00c551 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 2 Jan 2016 19:04:45 -0500 Subject: [PATCH] Teach pg_dump to quote reloption values safely. Commit c7e27becd2e6eb93 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. --- src/bin/pg_dump/pg_dump.c | 139 ++++++++++++++++++++++++++++++-------- src/bin/pg_dump/pg_dump.h | 2 +- 2 files changed, 113 insertions(+), 28 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 02d1221b45..57a3eb450d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -245,6 +245,9 @@ static void binary_upgrade_extension_member(PQExpBuffer upgrade_buffer, const char *objlabel); static const char *getAttrName(int attrnum, TableInfo *tblInfo); static const char *fmtCopyColumnList(const TableInfo *ti); +static bool nonemptyReloptions(const char *reloptions); +static void fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, + const char *reloptions, const char *prefix); static void do_sql_command(PGconn *conn, const char *query); static void check_sql_result(PGresult *res, PGconn *conn, const char *query, ExecStatusType expected); @@ -3956,8 +3959,8 @@ getTables(int *numTables) "d.refobjid AS owning_tab, " "d.refobjsubid AS owning_col, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, " - "array_to_string(c.reloptions, ', ') AS reloptions, " - "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions " + "c.reloptions AS reloptions, " + "tc.reloptions AS toast_reloptions " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -3992,8 +3995,8 @@ getTables(int *numTables) "d.refobjid AS owning_tab, " "d.refobjsubid AS owning_col, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, " - "array_to_string(c.reloptions, ', ') AS reloptions, " - "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions " + "c.reloptions AS reloptions, " + "tc.reloptions AS toast_reloptions " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -4027,8 +4030,8 @@ getTables(int *numTables) "d.refobjid AS owning_tab, " "d.refobjsubid AS owning_col, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, " - "array_to_string(c.reloptions, ', ') AS reloptions, " - "array_to_string(array(SELECT 'toast.' || x FROM unnest(tc.reloptions) x), ', ') AS toast_reloptions " + "c.reloptions AS reloptions, " + "tc.reloptions AS toast_reloptions " "FROM pg_class c " "LEFT JOIN pg_depend d ON " "(c.relkind = '%c' AND " @@ -4062,7 +4065,7 @@ getTables(int *numTables) "d.refobjid AS owning_tab, " "d.refobjsubid AS owning_col, " "(SELECT spcname FROM pg_tablespace t WHERE t.oid = c.reltablespace) AS reltablespace, " - "array_to_string(c.reloptions, ', ') AS reloptions, " + "c.reloptions AS reloptions, " "NULL AS toast_reloptions " "FROM pg_class c " "LEFT JOIN pg_depend d ON " @@ -4497,7 +4500,7 @@ getIndexes(TableInfo tblinfo[], int numTables) i_conoid, i_condef, i_tablespace, - i_options; + i_indreloptions; int ntups; for (i = 0; i < numTables; i++) @@ -4545,7 +4548,7 @@ getIndexes(TableInfo tblinfo[], int numTables) "c.oid AS conoid, " "pg_catalog.pg_get_constraintdef(c.oid, false) AS condef, " "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " - "array_to_string(t.reloptions, ', ') AS options " + "t.reloptions AS indreloptions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_constraint c " @@ -4571,7 +4574,7 @@ getIndexes(TableInfo tblinfo[], int numTables) "c.oid AS conoid, " "null AS condef, " "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " - "array_to_string(t.reloptions, ', ') AS options " + "t.reloptions AS indreloptions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_depend d " @@ -4600,7 +4603,7 @@ getIndexes(TableInfo tblinfo[], int numTables) "c.oid AS conoid, " "null AS condef, " "(SELECT spcname FROM pg_catalog.pg_tablespace s WHERE s.oid = t.reltablespace) AS tablespace, " - "null AS options " + "null AS indreloptions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_depend d " @@ -4628,7 +4631,7 @@ getIndexes(TableInfo tblinfo[], int numTables) "c.oid AS conoid, " "null AS condef, " "NULL AS tablespace, " - "null AS options " + "null AS indreloptions " "FROM pg_catalog.pg_index i " "JOIN pg_catalog.pg_class t ON (t.oid = i.indexrelid) " "LEFT JOIN pg_catalog.pg_depend d " @@ -4659,7 +4662,7 @@ getIndexes(TableInfo tblinfo[], int numTables) "t.oid AS conoid, " "null AS condef, " "NULL AS tablespace, " - "null AS options " + "null AS indreloptions " "FROM pg_index i, pg_class t " "WHERE t.oid = i.indexrelid " "AND i.indrelid = '%u'::oid " @@ -4685,7 +4688,7 @@ getIndexes(TableInfo tblinfo[], int numTables) "t.oid AS conoid, " "null AS condef, " "NULL AS tablespace, " - "null AS options " + "null AS indreloptions " "FROM pg_index i, pg_class t " "WHERE t.oid = i.indexrelid " "AND i.indrelid = '%u'::oid " @@ -4713,7 +4716,7 @@ getIndexes(TableInfo tblinfo[], int numTables) i_conoid = PQfnumber(res, "conoid"); i_condef = PQfnumber(res, "condef"); i_tablespace = PQfnumber(res, "tablespace"); - i_options = PQfnumber(res, "options"); + i_indreloptions = PQfnumber(res, "indreloptions"); indxinfo = (IndxInfo *) malloc(ntups * sizeof(IndxInfo)); constrinfo = (ConstraintInfo *) malloc(ntups * sizeof(ConstraintInfo)); @@ -4732,7 +4735,7 @@ getIndexes(TableInfo tblinfo[], int numTables) indxinfo[j].indexdef = strdup(PQgetvalue(res, j, i_indexdef)); indxinfo[j].indnkeys = atoi(PQgetvalue(res, j, i_indnkeys)); indxinfo[j].tablespace = strdup(PQgetvalue(res, j, i_tablespace)); - indxinfo[j].options = strdup(PQgetvalue(res, j, i_options)); + indxinfo[j].indreloptions = strdup(PQgetvalue(res, j, i_indreloptions)); /* * In pre-7.4 releases, indkeys may contain more entries than @@ -12448,23 +12451,24 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) if (tbinfo->relkind == RELKIND_FOREIGN_TABLE) appendPQExpBuffer(q, "\nSERVER %s", fmtId(srvname)); - if ((tbinfo->reloptions && strlen(tbinfo->reloptions) > 0) || - (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0)) + if (nonemptyReloptions(tbinfo->reloptions) || + nonemptyReloptions(tbinfo->toast_reloptions)) { bool addcomma = false; - appendPQExpBuffer(q, "\nWITH ("); - if (tbinfo->reloptions && strlen(tbinfo->reloptions) > 0) + appendPQExpBufferStr(q, "\nWITH ("); + if (nonemptyReloptions(tbinfo->reloptions)) { addcomma = true; - appendPQExpBuffer(q, "%s", tbinfo->reloptions); + fmtReloptionsArray(fout, q, tbinfo->reloptions, ""); } - if (tbinfo->toast_reloptions && strlen(tbinfo->toast_reloptions) > 0) + if (nonemptyReloptions(tbinfo->toast_reloptions)) { - appendPQExpBuffer(q, "%s%s", addcomma ? ", " : "", - tbinfo->toast_reloptions); + if (addcomma) + appendPQExpBufferStr(q, ", "); + fmtReloptionsArray(fout, q, tbinfo->toast_reloptions, "toast."); } - appendPQExpBuffer(q, ")"); + appendPQExpBufferChar(q, ')'); } /* Dump generic options if any */ @@ -12959,8 +12963,12 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo) appendPQExpBuffer(q, ")"); - if (indxinfo->options && strlen(indxinfo->options) > 0) - appendPQExpBuffer(q, " WITH (%s)", indxinfo->options); + if (nonemptyReloptions(indxinfo->indreloptions)) + { + appendPQExpBufferStr(q, " WITH ("); + fmtReloptionsArray(fout, q, indxinfo->indreloptions, ""); + appendPQExpBufferChar(q, ')'); + } if (coninfo->condeferrable) { @@ -14572,6 +14580,83 @@ fmtCopyColumnList(const TableInfo *ti) return q->data; } +/* + * Check if a reloptions array is nonempty. + */ +static bool +nonemptyReloptions(const char *reloptions) +{ + /* Don't want to print it if it's just "{}" */ + return (reloptions != NULL && strlen(reloptions) > 2); +} + +/* + * Format a reloptions array and append it to the given buffer. + * + * "prefix" is prepended to the option names; typically it's "" or "toast.". + * + * Note: this logic should generally match the backend's flatten_reloptions() + * (in adt/ruleutils.c). + */ +static void +fmtReloptionsArray(Archive *fout, PQExpBuffer buffer, const char *reloptions, + const char *prefix) +{ + char **options; + int noptions; + int i; + + if (!parsePGArray(reloptions, &options, &noptions)) + { + write_msg(NULL, "WARNING: could not parse reloptions array\n"); + if (options) + free(options); + return; + } + + for (i = 0; i < noptions; i++) + { + char *option = options[i]; + char *name; + char *separator; + char *value; + + /* + * Each array element should have the form name=value. If the "=" is + * missing for some reason, treat it like an empty value. + */ + name = option; + separator = strchr(option, '='); + if (separator) + { + *separator = '\0'; + value = separator + 1; + } + else + value = ""; + + if (i > 0) + appendPQExpBufferStr(buffer, ", "); + appendPQExpBuffer(buffer, "%s%s=", prefix, fmtId(name)); + + /* + * In general we need to quote the value; but to avoid unnecessary + * clutter, do not quote if it is an identifier that would not need + * quoting. (We could also allow numbers, but that is a bit trickier + * than it looks --- for example, are leading zeroes significant? We + * don't want to assume very much here about what custom reloptions + * might mean.) + */ + if (strcmp(fmtId(value), value) == 0) + appendPQExpBufferStr(buffer, value); + else + appendStringLiteralAH(buffer, value, fout); + } + + if (options) + free(options); +} + /* * Convenience subroutine to execute a SQL command and check for * COMMAND_OK status. diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 90296ca738..be139c8aea 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -313,7 +313,7 @@ typedef struct _indxInfo TableInfo *indextable; /* link to table the index is for */ char *indexdef; char *tablespace; /* tablespace in which index is stored */ - char *options; /* options specified by WITH (...) */ + char *indreloptions; /* options specified by WITH (...) */ int indnkeys; Oid *indkeys; bool indisclustered; -- 2.40.0