Skip to content

Commit 88adf1a

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 addf9e1 commit 88adf1a

File tree

9 files changed

+323
-29
lines changed

9 files changed

+323
-29
lines changed

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

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2055,14 +2055,39 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
20552055
/*
20562056
* Variables that are marked GUC_LIST_QUOTE were already fully
20572057
* quoted by flatten_set_variable_args() before they were put
2058-
* into the proconfig array; we mustn't re-quote them or we'll
2059-
* make a mess. Variables that are not so marked should just
2060-
* be emitted as simple string literals. If the variable is
2061-
* not known to guc.c, we'll do the latter; this makes it
2062-
* unsafe to use GUC_LIST_QUOTE for extension variables.
2058+
* into the proconfig array. However, because the quoting
2059+
* rules used there aren't exactly like SQL's, we have to
2060+
* break the list value apart and then quote the elements as
2061+
* string literals. (The elements may be double-quoted as-is,
2062+
* but we can't just feed them to the SQL parser; it would do
2063+
* the wrong thing with elements that are zero-length or
2064+
* longer than NAMEDATALEN.)
2065+
*
2066+
* Variables that are not so marked should just be emitted as
2067+
* simple string literals. If the variable is not known to
2068+
* guc.c, we'll do that; this makes it unsafe to use
2069+
* GUC_LIST_QUOTE for extension variables.
20632070
*/
20642071
if (GetConfigOptionFlags(configitem, true) & GUC_LIST_QUOTE)
2065-
appendStringInfoString(&buf, pos);
2072+
{
2073+
List *namelist;
2074+
ListCell *lc;
2075+
2076+
/* Parse string into list of identifiers */
2077+
if (!SplitGUCList(pos, ',', &namelist))
2078+
{
2079+
/* this shouldn't fail really */
2080+
elog(ERROR, "invalid list syntax in proconfig item");
2081+
}
2082+
foreach(lc, namelist)
2083+
{
2084+
char *curname = (char *) lfirst(lc);
2085+
2086+
simple_quote_literal(&buf, curname);
2087+
if (lnext(lc))
2088+
appendStringInfoString(&buf, ", ");
2089+
}
2090+
}
20662091
else
20672092
simple_quote_literal(&buf, pos);
20682093
appendStringInfoChar(&buf, '\n');

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

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

25702570

2571+
/*
2572+
* SplitGUCList --- parse a string containing identifiers or file names
2573+
*
2574+
* This is used to split the value of a GUC_LIST_QUOTE GUC variable, without
2575+
* presuming whether the elements will be taken as identifiers or file names.
2576+
* We assume the input has already been through flatten_set_variable_args(),
2577+
* so that we need never downcase (if appropriate, that was done already).
2578+
* Nor do we ever truncate, since we don't know the correct max length.
2579+
* We disallow embedded whitespace for simplicity (it shouldn't matter,
2580+
* because any embedded whitespace should have led to double-quoting).
2581+
* Otherwise the API is identical to SplitIdentifierString.
2582+
*
2583+
* XXX it's annoying to have so many copies of this string-splitting logic.
2584+
* However, it's not clear that having one function with a bunch of option
2585+
* flags would be much better.
2586+
*
2587+
* XXX there is a version of this function in src/bin/pg_dump/dumputils.c.
2588+
* Be sure to update that if you have to change this.
2589+
*
2590+
* Inputs:
2591+
* rawstring: the input string; must be overwritable! On return, it's
2592+
* been modified to contain the separated identifiers.
2593+
* separator: the separator punctuation expected between identifiers
2594+
* (typically '.' or ','). Whitespace may also appear around
2595+
* identifiers.
2596+
* Outputs:
2597+
* namelist: filled with a palloc'd list of pointers to identifiers within
2598+
* rawstring. Caller should list_free() this even on error return.
2599+
*
2600+
* Returns true if okay, false if there is a syntax error in the string.
2601+
*/
2602+
bool
2603+
SplitGUCList(char *rawstring, char separator,
2604+
List **namelist)
2605+
{
2606+
char *nextp = rawstring;
2607+
bool done = false;
2608+
2609+
*namelist = NIL;
2610+
2611+
while (scanner_isspace(*nextp))
2612+
nextp++; /* skip leading whitespace */
2613+
2614+
if (*nextp == '\0')
2615+
return true; /* allow empty string */
2616+
2617+
/* At the top of the loop, we are at start of a new identifier. */
2618+
do
2619+
{
2620+
char *curname;
2621+
char *endp;
2622+
2623+
if (*nextp == '"')
2624+
{
2625+
/* Quoted name --- collapse quote-quote pairs */
2626+
curname = nextp + 1;
2627+
for (;;)
2628+
{
2629+
endp = strchr(nextp + 1, '"');
2630+
if (endp == NULL)
2631+
return false; /* mismatched quotes */
2632+
if (endp[1] != '"')
2633+
break; /* found end of quoted name */
2634+
/* Collapse adjacent quotes into one quote, and look again */
2635+
memmove(endp, endp + 1, strlen(endp));
2636+
nextp = endp;
2637+
}
2638+
/* endp now points at the terminating quote */
2639+
nextp = endp + 1;
2640+
}
2641+
else
2642+
{
2643+
/* Unquoted name --- extends to separator or whitespace */
2644+
curname = nextp;
2645+
while (*nextp && *nextp != separator &&
2646+
!scanner_isspace(*nextp))
2647+
nextp++;
2648+
endp = nextp;
2649+
if (curname == nextp)
2650+
return false; /* empty unquoted name not allowed */
2651+
}
2652+
2653+
while (scanner_isspace(*nextp))
2654+
nextp++; /* skip trailing whitespace */
2655+
2656+
if (*nextp == separator)
2657+
{
2658+
nextp++;
2659+
while (scanner_isspace(*nextp))
2660+
nextp++; /* skip leading whitespace for next */
2661+
/* we expect another name, so done remains false */
2662+
}
2663+
else if (*nextp == '\0')
2664+
done = true;
2665+
else
2666+
return false; /* invalid syntax */
2667+
2668+
/* Now safe to overwrite separator with a null */
2669+
*endp = '\0';
2670+
2671+
/*
2672+
* Finished isolating current name --- add it to list
2673+
*/
2674+
*namelist = lappend(*namelist, curname);
2675+
2676+
/* Loop back if we didn't reach end of string */
2677+
} while (!done);
2678+
2679+
return true;
2680+
}
2681+
2682+
25712683
/*****************************************************************************
25722684
* Comparison Functions used for bytea
25732685
*

‎src/bin/pg_dump/dumputils.c

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1545,3 +1545,112 @@ variable_is_guc_list_quote(const char *name)
15451545
else
15461546
return false;
15471547
}
1548+
1549+
/*
1550+
* SplitGUCList --- parse a string containing identifiers or file names
1551+
*
1552+
* This is used to split the value of a GUC_LIST_QUOTE GUC variable, without
1553+
* presuming whether the elements will be taken as identifiers or file names.
1554+
* See comparable code in src/backend/utils/adt/varlena.c.
1555+
*
1556+
* Inputs:
1557+
* rawstring: the input string; must be overwritable! On return, it's
1558+
* been modified to contain the separated identifiers.
1559+
* separator: the separator punctuation expected between identifiers
1560+
* (typically '.' or ','). Whitespace may also appear around
1561+
* identifiers.
1562+
* Outputs:
1563+
* namelist: receives a malloc'd, null-terminated array of pointers to
1564+
* identifiers within rawstring. Caller should free this
1565+
* even on error return.
1566+
*
1567+
* Returns true if okay, false if there is a syntax error in the string.
1568+
*/
1569+
bool
1570+
SplitGUCList(char *rawstring, char separator,
1571+
char ***namelist)
1572+
{
1573+
char *nextp = rawstring;
1574+
bool done = false;
1575+
char **nextptr;
1576+
1577+
/*
1578+
* Since we disallow empty identifiers, this is a conservative
1579+
* overestimate of the number of pointers we could need. Allow one for
1580+
* list terminator.
1581+
*/
1582+
*namelist = nextptr = (char **)
1583+
pg_malloc((strlen(rawstring) / 2 + 2) * sizeof(char *));
1584+
*nextptr = NULL;
1585+
1586+
while (isspace((unsigned char) *nextp))
1587+
nextp++; /* skip leading whitespace */
1588+
1589+
if (*nextp == '\0')
1590+
return true; /* allow empty string */
1591+
1592+
/* At the top of the loop, we are at start of a new identifier. */
1593+
do
1594+
{
1595+
char *curname;
1596+
char *endp;
1597+
1598+
if (*nextp == '"')
1599+
{
1600+
/* Quoted name --- collapse quote-quote pairs */
1601+
curname = nextp + 1;
1602+
for (;;)
1603+
{
1604+
endp = strchr(nextp + 1, '"');
1605+
if (endp == NULL)
1606+
return false; /* mismatched quotes */
1607+
if (endp[1] != '"')
1608+
break; /* found end of quoted name */
1609+
/* Collapse adjacent quotes into one quote, and look again */
1610+
memmove(endp, endp + 1, strlen(endp));
1611+
nextp = endp;
1612+
}
1613+
/* endp now points at the terminating quote */
1614+
nextp = endp + 1;
1615+
}
1616+
else
1617+
{
1618+
/* Unquoted name --- extends to separator or whitespace */
1619+
curname = nextp;
1620+
while (*nextp && *nextp != separator &&
1621+
!isspace((unsigned char) *nextp))
1622+
nextp++;
1623+
endp = nextp;
1624+
if (curname == nextp)
1625+
return false; /* empty unquoted name not allowed */
1626+
}
1627+
1628+
while (isspace((unsigned char) *nextp))
1629+
nextp++; /* skip trailing whitespace */
1630+
1631+
if (*nextp == separator)
1632+
{
1633+
nextp++;
1634+
while (isspace((unsigned char) *nextp))
1635+
nextp++; /* skip leading whitespace for next */
1636+
/* we expect another name, so done remains false */
1637+
}
1638+
else if (*nextp == '\0')
1639+
done = true;
1640+
else
1641+
return false; /* invalid syntax */
1642+
1643+
/* Now safe to overwrite separator with a null */
1644+
*endp = '\0';
1645+
1646+
/*
1647+
* Finished isolating current name --- add it to output array
1648+
*/
1649+
*nextptr++ = curname;
1650+
1651+
/* Loop back if we didn't reach end of string */
1652+
} while (!done);
1653+
1654+
*nextptr = NULL;
1655+
return true;
1656+
}

‎src/bin/pg_dump/dumputils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,7 @@ extern bool simple_string_list_member(SimpleStringList *list, const char *val);
7777

7878
extern bool variable_is_guc_list_quote(const char *name);
7979

80+
extern bool SplitGUCList(char *rawstring, char separator,
81+
char ***namelist);
82+
8083
#endif /* DUMPUTILS_H */

‎src/bin/pg_dump/pg_dump.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10151,14 +10151,36 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1015110151
/*
1015210152
* Variables that are marked GUC_LIST_QUOTE were already fully quoted
1015310153
* by flatten_set_variable_args() before they were put into the
10154-
* proconfig array; we mustn't re-quote them or we'll make a mess.
10154+
* proconfig array. However, because the quoting rules used there
10155+
* aren't exactly like SQL's, we have to break the list value apart
10156+
* and then quote the elements as string literals. (The elements may
10157+
* be double-quoted as-is, but we can't just feed them to the SQL
10158+
* parser; it would do the wrong thing with elements that are
10159+
* zero-length or longer than NAMEDATALEN.)
10160+
*
1015510161
* Variables that are not so marked should just be emitted as simple
1015610162
* string literals. If the variable is not known to
10157-
* variable_is_guc_list_quote(), we'll do the latter; this makes it
10158-
* unsafe to use GUC_LIST_QUOTE for extension variables.
10163+
* variable_is_guc_list_quote(), we'll do that; this makes it unsafe
10164+
* to use GUC_LIST_QUOTE for extension variables.
1015910165
*/
1016010166
if (variable_is_guc_list_quote(configitem))
10161-
appendPQExpBufferStr(q, pos);
10167+
{
10168+
char **namelist;
10169+
char **nameptr;
10170+
10171+
/* Parse string into list of identifiers */
10172+
/* this shouldn't fail really */
10173+
if (SplitGUCList(pos, ',', &namelist))
10174+
{
10175+
for (nameptr = namelist; *nameptr; nameptr++)
10176+
{
10177+
if (nameptr != namelist)
10178+
appendPQExpBufferStr(q, ", ");
10179+
appendStringLiteralAH(q, *nameptr, fout);
10180+
}
10181+
}
10182+
pg_free(namelist);
10183+
}
1016210184
else
1016310185
appendStringLiteralAH(q, pos, fout);
1016410186
}

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1612,14 +1612,35 @@ makeAlterConfigCommand(PGconn *conn, const char *arrayitem,
16121612
/*
16131613
* Variables that are marked GUC_LIST_QUOTE were already fully quoted by
16141614
* flatten_set_variable_args() before they were put into the setconfig
1615-
* array; we mustn't re-quote them or we'll make a mess. Variables that
1616-
* are not so marked should just be emitted as simple string literals. If
1617-
* the variable is not known to variable_is_guc_list_quote(), we'll do the
1618-
* latter; this makes it unsafe to use GUC_LIST_QUOTE for extension
1619-
* variables.
1615+
* array. However, because the quoting rules used there aren't exactly
1616+
* like SQL's, we have to break the list value apart and then quote the
1617+
* elements as string literals. (The elements may be double-quoted as-is,
1618+
* but we can't just feed them to the SQL parser; it would do the wrong
1619+
* thing with elements that are zero-length or longer than NAMEDATALEN.)
1620+
*
1621+
* Variables that are not so marked should just be emitted as simple
1622+
* string literals. If the variable is not known to
1623+
* variable_is_guc_list_quote(), we'll do that; this makes it unsafe to
1624+
* use GUC_LIST_QUOTE for extension variables.
16201625
*/
16211626
if (variable_is_guc_list_quote(mine))
1622-
appendPQExpBufferStr(buf, pos + 1);
1627+
{
1628+
char **namelist;
1629+
char **nameptr;
1630+
1631+
/* Parse string into list of identifiers */
1632+
/* this shouldn't fail really */
1633+
if (SplitGUCList(pos + 1, ',', &namelist))
1634+
{
1635+
for (nameptr = namelist; *nameptr; nameptr++)
1636+
{
1637+
if (nameptr != namelist)
1638+
appendPQExpBufferStr(buf, ", ");
1639+
appendStringLiteralConn(buf, *nameptr, conn);
1640+
}
1641+
}
1642+
pg_free(namelist);
1643+
}
16231644
else
16241645
appendStringLiteralConn(buf, pos + 1, conn);
16251646
appendPQExpBufferStr(buf, ";\n");

‎src/include/utils/builtins.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,8 @@ extern bool SplitIdentifierString(char *rawstring, char separator,
808808
List **namelist);
809809
extern bool SplitDirectoriesString(char *rawstring, char separator,
810810
List **namelist);
811+
extern bool SplitGUCList(char *rawstring, char separator,
812+
List **namelist);
811813
extern Datum replace_text(PG_FUNCTION_ARGS);
812814
extern text *replace_text_regexp(text *src_text, void *regexp,
813815
text *replace_text, bool glob);

0 commit comments

Comments
 (0)