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);
 }