Thread: Quoting in recovery.conf

Quoting in recovery.conf

From
Heikki Linnakangas
Date:
To follow up on the discussion here:

http://archives.postgresql.org/pgsql-docs/2010-02/msg00039.php

It seems like a big oversight that there's no way to insert quotes in
strings in recovery.conf. In the long run, the parsing should be done
the same way as postgresql.conf, or the two files be merged altogether,
but right now I think we should just add support for escaping quotes. I
propose two quotes '' to mean a quote mark in the string, like in
strings in SQL queries.

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com


Re: Quoting in recovery.conf

From
Fujii Masao
Date:
On Tue, Apr 6, 2010 at 3:47 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> To follow up on the discussion here:
>
> http://archives.postgresql.org/pgsql-docs/2010-02/msg00039.php
>
> It seems like a big oversight that there's no way to insert quotes in
> strings in recovery.conf. In the long run, the parsing should be done
> the same way as postgresql.conf, or the two files be merged altogether,
> but right now I think we should just add support for escaping quotes. I
> propose two quotes '' to mean a quote mark in the string, like in
> strings in SQL queries.

Agreed. This would be useful for users to specify the application_name
containing a space in the primary_conninfo, for example.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Re: Quoting in recovery.conf

From
Simon Riggs
Date:
On Tue, 2010-04-06 at 16:07 +0900, Fujii Masao wrote:
> On Tue, Apr 6, 2010 at 3:47 PM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com> wrote:
> > To follow up on the discussion here:
> >
> > http://archives.postgresql.org/pgsql-docs/2010-02/msg00039.php
> >
> > It seems like a big oversight that there's no way to insert quotes in
> > strings in recovery.conf. In the long run, the parsing should be done
> > the same way as postgresql.conf, or the two files be merged altogether,
> > but right now I think we should just add support for escaping quotes. I
> > propose two quotes '' to mean a quote mark in the string, like in
> > strings in SQL queries.
> 
> Agreed. This would be useful for users to specify the application_name
> containing a space in the primary_conninfo, for example.

+1

-- Simon Riggs           www.2ndQuadrant.com



Re: Quoting in recovery.conf

From
Heikki Linnakangas
Date:
Simon Riggs wrote:
> On Tue, 2010-04-06 at 16:07 +0900, Fujii Masao wrote:
>> On Tue, Apr 6, 2010 at 3:47 PM, Heikki Linnakangas
>> <heikki.linnakangas@enterprisedb.com> wrote:
>>> To follow up on the discussion here:
>>>
>>> http://archives.postgresql.org/pgsql-docs/2010-02/msg00039.php
>>>
>>> It seems like a big oversight that there's no way to insert quotes in
>>> strings in recovery.conf. In the long run, the parsing should be done
>>> the same way as postgresql.conf, or the two files be merged altogether,
>>> but right now I think we should just add support for escaping quotes. I
>>> propose two quotes '' to mean a quote mark in the string, like in
>>> strings in SQL queries.
>> Agreed. This would be useful for users to specify the application_name
>> containing a space in the primary_conninfo, for example.
>
> +1

Ok, here's what I came up with.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index abdf4d8..73ef0f9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4894,6 +4894,100 @@ str_time(pg_time_t tnow)
 }

 /*
+ * Parse one line from recovery.conf. 'cmdline' is the raw line from the
+ * file. If the line is parsed successfully, returns true, false indicates
+ * syntax error. On success, *key_p and *value_p are set to the parameter
+ * name and value on the line, respectively. If the line is an empty line,
+ * consisting entirely of whitespace and comments, function returns true
+ * and *keyp_p and *value_p are set to NULL.
+ *
+ * The pointers returned in *key_p and *value_p point to an internal buffer
+ * that is valid only until the next call of parseRecoveryCommandFile().
+ */
+static bool
+parseRecoveryCommandFileLine(char *cmdline, char **key_p, char **value_p)
+{
+    char       *ptr;
+    char       *bufp;
+    char       *key;
+    char       *value;
+    static char *buf = NULL;
+
+    *key_p = *value_p = NULL;
+
+    /*
+     * Allocate the buffer on first use. It's used to hold both the
+     * parameter name and value.
+     */
+    if (buf == NULL)
+        buf = malloc(MAXPGPATH + 1);
+    bufp = buf;
+
+    /* Skip any whitespace at the beginning of line */
+    for (ptr = cmdline; *ptr; ptr++)
+    {
+        if (!isspace((unsigned char) *ptr))
+            break;
+    }
+    /* Ignore empty lines */
+    if (*ptr == '\0' || *ptr == '#')
+        return true;
+
+    /* Read the parameter name */
+    key = bufp;
+    while (*ptr && !isspace((unsigned char) *ptr) &&
+           *ptr != '=' && *ptr != '\'')
+        *(bufp++) = *(ptr++);
+    *(bufp++) = '\0';
+
+    /* Skip to the beginning quote of the parameter value */
+    ptr = strchr(ptr, '\'');
+    if (!ptr)
+        return false;
+    ptr++;
+
+    /* Read the parameter value to *bufp. Collapse any '' escapes as we go. */
+    value = bufp;
+    for (;;)
+    {
+        if (*ptr == '\'')
+        {
+            ptr++;
+            if (*ptr == '\'')
+                *(bufp++) = '\'';
+            else
+            {
+                /* end of parameter */
+                *bufp = '\0';
+                break;
+            }
+        }
+        else if (*ptr == '\0')
+            return false;    /* unterminated quoted string */
+        else
+            *(bufp++) = *ptr;
+
+        ptr++;
+    }
+    *(bufp++) = '\0';
+
+    /* Check that there's no garbage after the value */
+    while (*ptr)
+    {
+        if (*ptr == '#')
+            break;
+        if (!isspace((unsigned char) *ptr))
+            return false;
+        ptr++;
+    }
+
+    /* Success! */
+    *key_p = key;
+    *value_p = value;
+    return true;
+}
+
+/*
  * See if there is a recovery command file (recovery.conf), and if so
  * read in parameters for archive recovery and XLOG streaming.
  *
@@ -4926,39 +5020,16 @@ readRecoveryCommandFile(void)
      */
     while (fgets(cmdline, sizeof(cmdline), fd) != NULL)
     {
-        /* skip leading whitespace and check for # comment */
-        char       *ptr;
         char       *tok1;
         char       *tok2;

-        for (ptr = cmdline; *ptr; ptr++)
-        {
-            if (!isspace((unsigned char) *ptr))
-                break;
-        }
-        if (*ptr == '\0' || *ptr == '#')
-            continue;
-
-        /* identify the quoted parameter value */
-        tok1 = strtok(ptr, "'");
-        if (!tok1)
-        {
-            syntaxError = true;
-            break;
-        }
-        tok2 = strtok(NULL, "'");
-        if (!tok2)
-        {
-            syntaxError = true;
-            break;
-        }
-        /* reparse to get just the parameter name */
-        tok1 = strtok(ptr, " \t=");
-        if (!tok1)
+        if (!parseRecoveryCommandFileLine(cmdline, &tok1, &tok2))
         {
             syntaxError = true;
             break;
         }
+        if (tok1 == NULL)
+            continue;

         if (strcmp(tok1, "restore_command") == 0)
         {

Re: Quoting in recovery.conf

From
Fujii Masao
Date:
On Wed, Apr 7, 2010 at 1:48 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Ok, here's what I came up with.

Looks OK to me.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


Re: Quoting in recovery.conf

From
Heikki Linnakangas
Date:
Fujii Masao wrote:
> On Wed, Apr 7, 2010 at 1:48 AM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com> wrote:
>> Ok, here's what I came up with.
> 
> Looks OK to me.

Committed.

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com