Skip to content

Commit f3eb76b

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 23ca82d commit f3eb76b

File tree

8 files changed

+326
-30
lines changed

8 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
@@ -2640,14 +2640,39 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
26402640
/*
26412641
* Variables that are marked GUC_LIST_QUOTE were already fully
26422642
* quoted by flatten_set_variable_args() before they were put
2643-
* into the proconfig array; we mustn't re-quote them or we'll
2644-
* make a mess. Variables that are not so marked should just
2645-
* be emitted as simple string literals. If the variable is
2646-
* not known to guc.c, we'll do the latter; this makes it
2647-
* unsafe to use GUC_LIST_QUOTE for extension variables.
2643+
* into the proconfig array. However, because the quoting
2644+
* rules used there aren't exactly like SQL's, we have to
2645+
* break the list value apart and then quote the elements as
2646+
* string literals. (The elements may be double-quoted as-is,
2647+
* but we can't just feed them to the SQL parser; it would do
2648+
* the wrong thing with elements that are zero-length or
2649+
* longer than NAMEDATALEN.)
2650+
*
2651+
* Variables that are not so marked should just be emitted as
2652+
* simple string literals. If the variable is not known to
2653+
* guc.c, we'll do that; this makes it unsafe to use
2654+
* GUC_LIST_QUOTE for extension variables.
26482655
*/
26492656
if (GetConfigOptionFlags(configitem, true) & GUC_LIST_QUOTE)
2650-
appendStringInfoString(&buf, pos);
2657+
{
2658+
List *namelist;
2659+
ListCell *lc;
2660+
2661+
/* Parse string into list of identifiers */
2662+
if (!SplitGUCList(pos, ',', &namelist))
2663+
{
2664+
/* this shouldn't fail really */
2665+
elog(ERROR, "invalid list syntax in proconfig item");
2666+
}
2667+
foreach(lc, namelist)
2668+
{
2669+
char *curname = (char *) lfirst(lc);
2670+
2671+
simple_quote_literal(&buf, curname);
2672+
if (lnext(lc))
2673+
appendStringInfoString(&buf, ", ");
2674+
}
2675+
}
26512676
else
26522677
simple_quote_literal(&buf, pos);
26532678
appendStringInfoChar(&buf, '\n');

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

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

35053505

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

‎src/bin/pg_dump/dumputils.c

Lines changed: 138 additions & 6 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

@@ -873,6 +875,115 @@ variable_is_guc_list_quote(const char *name)
873875
return false;
874876
}
875877

878+
/*
879+
* SplitGUCList --- parse a string containing identifiers or file names
880+
*
881+
* This is used to split the value of a GUC_LIST_QUOTE GUC variable, without
882+
* presuming whether the elements will be taken as identifiers or file names.
883+
* See comparable code in src/backend/utils/adt/varlena.c.
884+
*
885+
* Inputs:
886+
* rawstring: the input string; must be overwritable! On return, it's
887+
* been modified to contain the separated identifiers.
888+
* separator: the separator punctuation expected between identifiers
889+
* (typically '.' or ','). Whitespace may also appear around
890+
* identifiers.
891+
* Outputs:
892+
* namelist: receives a malloc'd, null-terminated array of pointers to
893+
* identifiers within rawstring. Caller should free this
894+
* even on error return.
895+
*
896+
* Returns true if okay, false if there is a syntax error in the string.
897+
*/
898+
bool
899+
SplitGUCList(char *rawstring, char separator,
900+
char ***namelist)
901+
{
902+
char *nextp = rawstring;
903+
bool done = false;
904+
char **nextptr;
905+
906+
/*
907+
* Since we disallow empty identifiers, this is a conservative
908+
* overestimate of the number of pointers we could need. Allow one for
909+
* list terminator.
910+
*/
911+
*namelist = nextptr = (char **)
912+
pg_malloc((strlen(rawstring) / 2 + 2) * sizeof(char *));
913+
*nextptr = NULL;
914+
915+
while (isspace((unsigned char) *nextp))
916+
nextp++; /* skip leading whitespace */
917+
918+
if (*nextp == '\0')
919+
return true; /* allow empty string */
920+
921+
/* At the top of the loop, we are at start of a new identifier. */
922+
do
923+
{
924+
char *curname;
925+
char *endp;
926+
927+
if (*nextp == '"')
928+
{
929+
/* Quoted name --- collapse quote-quote pairs */
930+
curname = nextp + 1;
931+
for (;;)
932+
{
933+
endp = strchr(nextp + 1, '"');
934+
if (endp == NULL)
935+
return false; /* mismatched quotes */
936+
if (endp[1] != '"')
937+
break; /* found end of quoted name */
938+
/* Collapse adjacent quotes into one quote, and look again */
939+
memmove(endp, endp + 1, strlen(endp));
940+
nextp = endp;
941+
}
942+
/* endp now points at the terminating quote */
943+
nextp = endp + 1;
944+
}
945+
else
946+
{
947+
/* Unquoted name --- extends to separator or whitespace */
948+
curname = nextp;
949+
while (*nextp && *nextp != separator &&
950+
!isspace((unsigned char) *nextp))
951+
nextp++;
952+
endp = nextp;
953+
if (curname == nextp)
954+
return false; /* empty unquoted name not allowed */
955+
}
956+
957+
while (isspace((unsigned char) *nextp))
958+
nextp++; /* skip trailing whitespace */
959+
960+
if (*nextp == separator)
961+
{
962+
nextp++;
963+
while (isspace((unsigned char) *nextp))
964+
nextp++; /* skip leading whitespace for next */
965+
/* we expect another name, so done remains false */
966+
}
967+
else if (*nextp == '\0')
968+
done = true;
969+
else
970+
return false; /* invalid syntax */
971+
972+
/* Now safe to overwrite separator with a null */
973+
*endp = '\0';
974+
975+
/*
976+
* Finished isolating current name --- add it to output array
977+
*/
978+
*nextptr++ = curname;
979+
980+
/* Loop back if we didn't reach end of string */
981+
} while (!done);
982+
983+
*nextptr = NULL;
984+
return true;
985+
}
986+
876987
/*
877988
* Helper function for dumping "ALTER DATABASE/ROLE SET ..." commands.
878989
*
@@ -912,14 +1023,35 @@ makeAlterConfigCommand(PGconn *conn, const char *configitem,
9121023
/*
9131024
* Variables that are marked GUC_LIST_QUOTE were already fully quoted by
9141025
* flatten_set_variable_args() before they were put into the setconfig
915-
* array; we mustn't re-quote them or we'll make a mess. Variables that
916-
* are not so marked should just be emitted as simple string literals. If
917-
* the variable is not known to variable_is_guc_list_quote(), we'll do the
918-
* latter; this makes it unsafe to use GUC_LIST_QUOTE for extension
919-
* variables.
1026+
* array. However, because the quoting rules used there aren't exactly
1027+
* like SQL's, we have to break the list value apart and then quote the
1028+
* elements as string literals. (The elements may be double-quoted as-is,
1029+
* but we can't just feed them to the SQL parser; it would do the wrong
1030+
* thing with elements that are zero-length or longer than NAMEDATALEN.)
1031+
*
1032+
* Variables that are not so marked should just be emitted as simple
1033+
* string literals. If the variable is not known to
1034+
* variable_is_guc_list_quote(), we'll do that; this makes it unsafe to
1035+
* use GUC_LIST_QUOTE for extension variables.
9201036
*/
9211037
if (variable_is_guc_list_quote(mine))
922-
appendPQExpBufferStr(buf, pos);
1038+
{
1039+
char **namelist;
1040+
char **nameptr;
1041+
1042+
/* Parse string into list of identifiers */
1043+
/* this shouldn't fail really */
1044+
if (SplitGUCList(pos, ',', &namelist))
1045+
{
1046+
for (nameptr = namelist; *nameptr; nameptr++)
1047+
{
1048+
if (nameptr != namelist)
1049+
appendPQExpBufferStr(buf, ", ");
1050+
appendStringLiteralConn(buf, *nameptr, conn);
1051+
}
1052+
}
1053+
pg_free(namelist);
1054+
}
9231055
else
9241056
appendStringLiteralConn(buf, pos, conn);
9251057

‎src/bin/pg_dump/dumputils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,9 @@ 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
extern void makeAlterConfigCommand(PGconn *conn, const char *configitem,
6265
const char *type, const char *name,
6366
const char *type2, const char *name2,

‎src/bin/pg_dump/pg_dump.c

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11964,14 +11964,36 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1196411964
/*
1196511965
* Variables that are marked GUC_LIST_QUOTE were already fully quoted
1196611966
* by flatten_set_variable_args() before they were put into the
11967-
* proconfig array; we mustn't re-quote them or we'll make a mess.
11967+
* proconfig array. However, because the quoting rules used there
11968+
* aren't exactly like SQL's, we have to break the list value apart
11969+
* and then quote the elements as string literals. (The elements may
11970+
* be double-quoted as-is, but we can't just feed them to the SQL
11971+
* parser; it would do the wrong thing with elements that are
11972+
* zero-length or longer than NAMEDATALEN.)
11973+
*
1196811974
* Variables that are not so marked should just be emitted as simple
1196911975
* string literals. If the variable is not known to
11970-
* variable_is_guc_list_quote(), we'll do the latter; this makes it
11971-
* unsafe to use GUC_LIST_QUOTE for extension variables.
11976+
* variable_is_guc_list_quote(), we'll do that; this makes it unsafe
11977+
* to use GUC_LIST_QUOTE for extension variables.
1197211978
*/
1197311979
if (variable_is_guc_list_quote(configitem))
11974-
appendPQExpBufferStr(q, pos);
11980+
{
11981+
char **namelist;
11982+
char **nameptr;
11983+
11984+
/* Parse string into list of identifiers */
11985+
/* this shouldn't fail really */
11986+
if (SplitGUCList(pos, ',', &namelist))
11987+
{
11988+
for (nameptr = namelist; *nameptr; nameptr++)
11989+
{
11990+
if (nameptr != namelist)
11991+
appendPQExpBufferStr(q, ", ");
11992+
appendStringLiteralAH(q, *nameptr, fout);
11993+
}
11994+
}
11995+
pg_free(namelist);
11996+
}
1197511997
else
1197611998
appendStringLiteralAH(q, pos, fout);
1197711999
}

‎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)