Fix escaping in generated recovery.conf file.

In the primary_conninfo line that "pg_basebackup -R" generates, single
quotes in parameter values need to be escaped into \\'; the libpq parser
requires the quotes to be escaped into \', and recovery.conf parser requires
the \ to be escaped into \\.

Also, don't quote parameter values unnecessarily, to make the connection
string prettier. Most options in a libpq connection string don't need
quoting.

Reported by Hari Babu, closer analysis by Zoltan Boszormenyi, although I
didn't use his patch.

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 45585069a5ed998ee547587864a1bafe293c8109..84c34979228598abef6e4b1c2b2bc4f2b6f5e61f 100644 (file)
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1107,7 +1107,71 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 }
 
 /*
- * Escape single quotes used in connection parameters
+ * Escape a parameter value so that it can be used as part of a libpq
+ * connection string, e.g. in:
+ *
+ * application_name=<value>
+ *
+ * The returned string is malloc'd. Return NULL on out-of-memory.
+ */
+static char *
+escapeConnectionParameter(const char *src)
+{
+       bool            need_quotes = false;
+       bool            need_escaping = false;
+       const char *p;
+       char       *dstbuf;
+       char       *dst;
+
+       /*
+        * First check if quoting is needed. Any quote (') or backslash (\)
+        * characters need to be escaped. Parameters are separated by whitespace,
+        * so any string containing whitespace characters need to be quoted. An
+        * empty string is represented by ''.
+        */
+       if (strchr(src, '\'') != NULL || strchr(src, '\\') != NULL)
+               need_escaping = true;
+
+       for (p = src; *p; p++)
+       {
+               if (isspace(*p))
+               {
+                       need_quotes = true;
+                       break;
+               }
+       }
+
+       if (*src == '\0')
+               return pg_strdup("''");
+
+       if (!need_quotes && !need_escaping)
+               return pg_strdup(src); /* no quoting or escaping needed */
+
+       /*
+        * Allocate a buffer large enough for the worst case that all the source
+        * characters need to be escaped, plus quotes.
+        */
+       dstbuf = pg_malloc(strlen(src) * 2 + 2 + 1);
+
+       dst = dstbuf;
+       if (need_quotes)
+               *(dst++) = '\'';
+       for (; *src; src++)
+       {
+               if (*src == '\'' || *src == '\\')
+                       *(dst++) = '\\';
+               *(dst++) = *src;
+       }
+       if (need_quotes)
+               *(dst++) = '\'';
+       *dst = '\0';
+
+       return dstbuf;
+}
+
+/*
+ * Escape a string so that it can be used as a value in a key-value pair
+ * a configuration file.
  */
 static char *
 escape_quotes(const char *src)
@@ -1130,6 +1194,8 @@ GenerateRecoveryConf(PGconn *conn)
 {
        PQconninfoOption *connOptions;
        PQconninfoOption *option;
+       PQExpBufferData conninfo_buf;
+       char       *escaped;
 
        recoveryconfcontents = createPQExpBuffer();
        if (!recoveryconfcontents)
@@ -1146,12 +1212,10 @@ GenerateRecoveryConf(PGconn *conn)
        }
 
        appendPQExpBufferStr(recoveryconfcontents, "standby_mode = 'on'\n");
-       appendPQExpBufferStr(recoveryconfcontents, "primary_conninfo = '");
 
+       initPQExpBuffer(&conninfo_buf);
        for (option = connOptions; option && option->keyword; option++)
        {
-               char       *escaped;
-
                /*
                 * Do not emit this setting if: - the setting is "replication",
                 * "dbname" or "fallback_application_name", since these would be
@@ -1165,24 +1229,37 @@ GenerateRecoveryConf(PGconn *conn)
                        (option->val != NULL && option->val[0] == '\0'))
                        continue;
 
+               /* Separate key-value pairs with spaces */
+               if (conninfo_buf.len != 0)
+                       appendPQExpBufferStr(&conninfo_buf, " ");
+
                /*
-                * Write "keyword='value'" pieces, the value string is escaped if
-                * necessary and doubled single quotes around the value string.
+                * Write "keyword=value" pieces, the value string is escaped and/or
+                * quoted if necessary.
                 */
-               escaped = escape_quotes(option->val);
-
-               appendPQExpBuffer(recoveryconfcontents, "%s=''%s'' ", option->keyword, escaped);
-
+               escaped = escapeConnectionParameter(option->val);
+               appendPQExpBuffer(&conninfo_buf, "%s=%s", option->keyword, escaped);
                free(escaped);
        }
 
-       appendPQExpBufferStr(recoveryconfcontents, "'\n");
-       if (PQExpBufferBroken(recoveryconfcontents))
+       /*
+        * Escape the connection string, so that it can be put in the config file.
+        * Note that this is different from the escaping of individual connection
+        * options above!
+        */
+       escaped = escape_quotes(conninfo_buf.data);
+       appendPQExpBuffer(recoveryconfcontents, "primary_conninfo = '%s'\n", escaped);
+       free(escaped);
+
+       if (PQExpBufferBroken(recoveryconfcontents) ||
+               PQExpBufferDataBroken(conninfo_buf))
        {
                fprintf(stderr, _("%s: out of memory\n"), progname);
                disconnect_and_exit(1);
        }
 
+       termPQExpBuffer(&conninfo_buf);
+
        PQconninfoFree(connOptions);
 }