Skip to content

Commit 31b29b1

Browse files
tglsfdctglsfdc
committed
Further fixes for quoted-list GUC values in pg_dump and ruleutils.c.
Commits 7428699 et al turn out to be a couple bricks shy of a load. We were dumping the stored values of GUC_LIST_QUOTE variables as they appear in proconfig or setconfig catalog columns. However, although that quoting rule looks a lot like SQL-identifier double quotes, there are two critical differences: empty strings ("") are legal, and depending on which variable you're considering, values longer than NAMEDATALEN might be valid too. So the current technique fails altogether on empty-string list entries (as reported by Steven Winfield in bug #15248) and it also risks truncating file pathnames during dump/reload of GUC values that are lists of pathnames. To fix, split the stored value without any downcasing or truncation, and then emit each element as a SQL string literal. This is a tad annoying, because we now have three copies of the comma-separated-string splitting logic in varlena.c as well as a fourth one in dumputils.c. (Not to mention the randomly-different-from-those splitting logic in libpq...) I looked at unifying these, but it would be rather a mess unless we're willing to tweak the API definitions of SplitIdentifierString, SplitDirectoriesString, or both. That might be worth doing in future; but it seems pretty unsafe for a back-patched bug fix, so for now accept the duplication. Back-patch to all supported branches, as the previous fix was. Discussion: https://postgr.es/m/7585.1529435872@sss.pgh.pa.us
1 parent 2c4d0f3 commit 31b29b1

File tree

9 files changed

+326
-30
lines changed

9 files changed

+326
-30
lines changed

‎src/backend/utils/adt/ruleutils.c

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2571,14 +2571,39 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
25712571
/*
25722572
* Variables that are marked GUC_LIST_QUOTE were already fully
25732573
* quoted by flatten_set_variable_args() before they were put
2574-
* into the proconfig array; we mustn't re-quote them or we'll
2575-
* make a mess. Variables that are not so marked should just
2576-
* be emitted as simple string literals. If the variable is
2577-
* not known to guc.c, we'll do the latter; this makes it
2578-
* unsafe to use GUC_LIST_QUOTE for extension variables.
2574+
* into the proconfig array. However, because the quoting
2575+
* rules used there aren't exactly like SQL's, we have to
2576+
* break the list value apart and then quote the elements as
2577+
* string literals. (The elements may be double-quoted as-is,
2578+
* but we can't just feed them to the SQL parser; it would do
2579+
* the wrong thing with elements that are zero-length or
2580+
* longer than NAMEDATALEN.)
2581+
*
2582+
* Variables that are not so marked should just be emitted as
2583+
* simple string literals. If the variable is not known to
2584+
* guc.c, we'll do that; this makes it unsafe to use
2585+
* GUC_LIST_QUOTE for extension variables.
25792586
*/
25802587
if (GetConfigOptionFlags(configitem, true) & GUC_LIST_QUOTE)
2581-
appendStringInfoString(&buf, pos);
2588+
{
2589+
List *namelist;
2590+
ListCell *lc;
2591+
2592+
/* Parse string into list of identifiers */
2593+
if (!SplitGUCList(pos, ',', &namelist))
2594+
{
2595+
/* this shouldn't fail really */
2596+
elog(ERROR, "invalid list syntax in proconfig item");
2597+
}
2598+
foreach(lc, namelist)
2599+
{
2600+
char *curname = (char *) lfirst(lc);
2601+
2602+
simple_quote_literal(&buf, curname);
2603+
if (lnext(lc))
2604+
appendStringInfoString(&buf, ", ");
2605+
}
2606+
}
25822607
else
25832608
simple_quote_literal(&buf, pos);
25842609
appendStringInfoChar(&buf, '\n');

‎src/backend/utils/adt/varlena.c

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3477,6 +3477,118 @@ SplitDirectoriesString(char *rawstring, char separator,
34773477
}
34783478

34793479

3480+
/*
3481+
* SplitGUCList --- parse a string containing identifiers or file names
3482+
*
3483+
* This is used to split the value of a GUC_LIST_QUOTE GUC variable, without
3484+
* presuming whether the elements will be taken as identifiers or file names.
3485+
* We assume the input has already been through flatten_set_variable_args(),
3486+
* so that we need never downcase (if appropriate, that was done already).
3487+
* Nor do we ever truncate, since we don't know the correct max length.
3488+
* We disallow embedded whitespace for simplicity (it shouldn't matter,
3489+
* because any embedded whitespace should have led to double-quoting).
3490+
* Otherwise the API is identical to SplitIdentifierString.
3491+
*
3492+
* XXX it's annoying to have so many copies of this string-splitting logic.
3493+
* However, it's not clear that having one function with a bunch of option
3494+
* flags would be much better.
3495+
*
3496+
* XXX there is a version of this function in src/bin/pg_dump/dumputils.c.
3497+
* Be sure to update that if you have to change this.
3498+
*
3499+
* Inputs:
3500+
* rawstring: the input string; must be overwritable! On return, it's
3501+
* been modified to contain the separated identifiers.
3502+
* separator: the separator punctuation expected between identifiers
3503+
* (typically '.' or ','). Whitespace may also appear around
3504+
* identifiers.
3505+
* Outputs:
3506+
* namelist: filled with a palloc'd list of pointers to identifiers within
3507+
* rawstring. Caller should list_free() this even on error return.
3508+
*
3509+
* Returns true if okay, false if there is a syntax error in the string.
3510+
*/
3511+
bool
3512+
SplitGUCList(char *rawstring, char separator,
3513+
List **namelist)
3514+
{
3515+
char *nextp = rawstring;
3516+
bool done = false;
3517+
3518+
*namelist = NIL;
3519+
3520+
while (scanner_isspace(*nextp))
3521+
nextp++; /* skip leading whitespace */
3522+
3523+
if (*nextp == '\0')
3524+
return true; /* allow empty string */
3525+
3526+
/* At the top of the loop, we are at start of a new identifier. */
3527+
do
3528+
{
3529+
char *curname;
3530+
char *endp;
3531+
3532+
if (*nextp == '"')
3533+
{
3534+
/* Quoted name --- collapse quote-quote pairs */
3535+
curname = nextp + 1;
3536+
for (;;)
3537+
{
3538+
endp = strchr(nextp + 1, '"');
3539+
if (endp == NULL)
3540+
return false; /* mismatched quotes */
3541+
if (endp[1] != '"')
3542+
break; /* found end of quoted name */
3543+
/* Collapse adjacent quotes into one quote, and look again */
3544+
memmove(endp, endp + 1, strlen(endp));
3545+
nextp = endp;
3546+
}
3547+
/* endp now points at the terminating quote */
3548+
nextp = endp + 1;
3549+
}
3550+
else
3551+
{
3552+
/* Unquoted name --- extends to separator or whitespace */
3553+
curname = nextp;
3554+
while (*nextp && *nextp != separator &&
3555+
!scanner_isspace(*nextp))
3556+
nextp++;
3557+
endp = nextp;
3558+
if (curname == nextp)
3559+
return false; /* empty unquoted name not allowed */
3560+
}
3561+
3562+
while (scanner_isspace(*nextp))
3563+
nextp++; /* skip trailing whitespace */
3564+
3565+
if (*nextp == separator)
3566+
{
3567+
nextp++;
3568+
while (scanner_isspace(*nextp))
3569+
nextp++; /* skip leading whitespace for next */
3570+
/* we expect another name, so done remains false */
3571+
}
3572+
else if (*nextp == '\0')
3573+
done = true;
3574+
else
3575+
return false; /* invalid syntax */
3576+
3577+
/* Now safe to overwrite separator with a null */
3578+
*endp = '\0';
3579+
3580+
/*
3581+
* Finished isolating current name --- add it to list
3582+
*/
3583+
*namelist = lappend(*namelist, curname);
3584+
3585+
/* Loop back if we didn't reach end of string */
3586+
} while (!done);
3587+
3588+
return true;
3589+
}
3590+
3591+
34803592
/*****************************************************************************
34813593
* Comparison Functions used for bytea
34823594
*

‎src/bin/pg_dump/dumputils.c

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
*/
1515
#include "postgres_fe.h"
1616

17+
#include <ctype.h>
18+
1719
#include "dumputils.h"
1820
#include "fe_utils/string_utils.h"
1921

@@ -869,3 +871,112 @@ variable_is_guc_list_quote(const char *name)
869871
else
870872
return false;
871873
}
874+
875+
/*
876+
* SplitGUCList --- parse a string containing identifiers or file names
877+
*
878+
* This is used to split the value of a GUC_LIST_QUOTE GUC variable, without
879+
* presuming whether the elements will be taken as identifiers or file names.
880+
* See comparable code in src/backend/utils/adt/varlena.c.
881+
*
882+
* Inputs:
883+
* rawstring: the input string; must be overwritable! On return, it's
884+
* been modified to contain the separated identifiers.
885+
* separator: the separator punctuation expected between identifiers
886+
* (typically '.' or ','). Whitespace may also appear around
887+
* identifiers.
888+
* Outputs:
889+
* namelist: receives a malloc'd, null-terminated array of pointers to
890+
* identifiers within rawstring. Caller should free this
891+
* even on error return.
892+
*
893+
* Returns true if okay, false if there is a syntax error in the string.
894+
*/
895+
bool
896+
SplitGUCList(char *rawstring, char separator,
897+
char ***namelist)
898+
{
899+
char *nextp = rawstring;
900+
bool done = false;
901+
char **nextptr;
902+
903+
/*
904+
* Since we disallow empty identifiers, this is a conservative
905+
* overestimate of the number of pointers we could need. Allow one for
906+
* list terminator.
907+
*/
908+
*namelist = nextptr = (char **)
909+
pg_malloc((strlen(rawstring) / 2 + 2) * sizeof(char *));
910+
*nextptr = NULL;
911+
912+
while (isspace((unsigned char) *nextp))
913+
nextp++; /* skip leading whitespace */
914+
915+
if (*nextp == '\0')
916+
return true; /* allow empty string */
917+
918+
/* At the top of the loop, we are at start of a new identifier. */
919+
do
920+
{
921+
char *curname;
922+
char *endp;
923+
924+
if (*nextp == '"')
925+
{
926+
/* Quoted name --- collapse quote-quote pairs */
927+
curname = nextp + 1;
928+
for (;;)
929+
{
930+
endp = strchr(nextp + 1, '"');
931+
if (endp == NULL)
932+
return false; /* mismatched quotes */
933+
if (endp[1] != '"')
934+
break; /* found end of quoted name */
935+
/* Collapse adjacent quotes into one quote, and look again */
936+
memmove(endp, endp + 1, strlen(endp));
937+
nextp = endp;
938+
}
939+
/* endp now points at the terminating quote */
940+
nextp = endp + 1;
941+
}
942+
else
943+
{
944+
/* Unquoted name --- extends to separator or whitespace */
945+
curname = nextp;
946+
while (*nextp && *nextp != separator &&
947+
!isspace((unsigned char) *nextp))
948+
nextp++;
949+
endp = nextp;
950+
if (curname == nextp)
951+
return false; /* empty unquoted name not allowed */
952+
}
953+
954+
while (isspace((unsigned char) *nextp))
955+
nextp++; /* skip trailing whitespace */
956+
957+
if (*nextp == separator)
958+
{
959+
nextp++;
960+
while (isspace((unsigned char) *nextp))
961+
nextp++; /* skip leading whitespace for next */
962+
/* we expect another name, so done remains false */
963+
}
964+
else if (*nextp == '\0')
965+
done = true;
966+
else
967+
return false; /* invalid syntax */
968+
969+
/* Now safe to overwrite separator with a null */
970+
*endp = '\0';
971+
972+
/*
973+
* Finished isolating current name --- add it to output array
974+
*/
975+
*nextptr++ = curname;
976+
977+
/* Loop back if we didn't reach end of string */
978+
} while (!done);
979+
980+
*nextptr = NULL;
981+
return true;
982+
}

‎src/bin/pg_dump/dumputils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,4 +58,7 @@ extern void buildACLQueries(PQExpBuffer acl_subquery, PQExpBuffer racl_subquery,
5858

5959
extern bool variable_is_guc_list_quote(const char *name);
6060

61+
extern bool SplitGUCList(char *rawstring, char separator,
62+
char ***namelist);
63+
6164
#endif /* DUMPUTILS_H */

‎src/bin/pg_dump/pg_dump.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11482,14 +11482,36 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1148211482
/*
1148311483
* Variables that are marked GUC_LIST_QUOTE were already fully quoted
1148411484
* by flatten_set_variable_args() before they were put into the
11485-
* proconfig array; we mustn't re-quote them or we'll make a mess.
11485+
* proconfig array. However, because the quoting rules used there
11486+
* aren't exactly like SQL's, we have to break the list value apart
11487+
* and then quote the elements as string literals. (The elements may
11488+
* be double-quoted as-is, but we can't just feed them to the SQL
11489+
* parser; it would do the wrong thing with elements that are
11490+
* zero-length or longer than NAMEDATALEN.)
11491+
*
1148611492
* Variables that are not so marked should just be emitted as simple
1148711493
* string literals. If the variable is not known to
11488-
* variable_is_guc_list_quote(), we'll do the latter; this makes it
11489-
* unsafe to use GUC_LIST_QUOTE for extension variables.
11494+
* variable_is_guc_list_quote(), we'll do that; this makes it unsafe
11495+
* to use GUC_LIST_QUOTE for extension variables.
1149011496
*/
1149111497
if (variable_is_guc_list_quote(configitem))
11492-
appendPQExpBufferStr(q, pos);
11498+
{
11499+
char **namelist;
11500+
char **nameptr;
11501+
11502+
/* Parse string into list of identifiers */
11503+
/* this shouldn't fail really */
11504+
if (SplitGUCList(pos, ',', &namelist))
11505+
{
11506+
for (nameptr = namelist; *nameptr; nameptr++)
11507+
{
11508+
if (nameptr != namelist)
11509+
appendPQExpBufferStr(q, ", ");
11510+
appendStringLiteralAH(q, *nameptr, fout);
11511+
}
11512+
}
11513+
pg_free(namelist);
11514+
}
1149311515
else
1149411516
appendStringLiteralAH(q, pos, fout);
1149511517
}

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,14 +1708,35 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
17081708
/*
17091709
* Variables that are marked GUC_LIST_QUOTE were already fully quoted by
17101710
* flatten_set_variable_args() before they were put into the setconfig
1711-
* array; we mustn't re-quote them or we'll make a mess. Variables that
1712-
* are not so marked should just be emitted as simple string literals. If
1713-
* the variable is not known to variable_is_guc_list_quote(), we'll do the
1714-
* latter; this makes it unsafe to use GUC_LIST_QUOTE for extension
1715-
* variables.
1711+
* array. However, because the quoting rules used there aren't exactly
1712+
* like SQL's, we have to break the list value apart and then quote the
1713+
* elements as string literals. (The elements may be double-quoted as-is,
1714+
* but we can't just feed them to the SQL parser; it would do the wrong
1715+
* thing with elements that are zero-length or longer than NAMEDATALEN.)
1716+
*
1717+
* Variables that are not so marked should just be emitted as simple
1718+
* string literals. If the variable is not known to
1719+
* variable_is_guc_list_quote(), we'll do that; this makes it unsafe to
1720+
* use GUC_LIST_QUOTE for extension variables.
17161721
*/
17171722
if (variable_is_guc_list_quote(mine))
1718-
appendPQExpBufferStr(buf, pos + 1);
1723+
{
1724+
char **namelist;
1725+
char **nameptr;
1726+
1727+
/* Parse string into list of identifiers */
1728+
/* this shouldn't fail really */
1729+
if (SplitGUCList(pos + 1, ',', &namelist))
1730+
{
1731+
for (nameptr = namelist; *nameptr; nameptr++)
1732+
{
1733+
if (nameptr != namelist)
1734+
appendPQExpBufferStr(buf, ", ");
1735+
appendStringLiteralConn(buf, *nameptr, conn);
1736+
}
1737+
}
1738+
pg_free(namelist);
1739+
}
17191740
else
17201741
appendStringLiteralConn(buf, pos + 1, conn);
17211742
appendPQExpBufferStr(buf, ";\n");

‎src/include/utils/varlena.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ extern bool SplitIdentifierString(char *rawstring, char separator,
3131
List **namelist);
3232
extern bool SplitDirectoriesString(char *rawstring, char separator,
3333
List **namelist);
34+
extern bool SplitGUCList(char *rawstring, char separator,
35+
List **namelist);
3436
extern text *replace_text_regexp(text *src_text, void *regexp,
3537
text *replace_text, bool glob);
3638

0 commit comments

Comments
 (0)