Re: pg_basebackup with -R option and start standby have problems with escaped password
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu <haribabu.kommi@huawei.com> wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the backup
database starts.FATAL: could not connect to the primary server: missing "=" after "1'" in
connection info string
What does the resulting recovery.conf file look like?
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: 50ffaaf2.c8c10e0a.6db5.ffff9d60SMTPIN_ADDED_BROKEN@mx.google.comReference msg id not found: 50ffaaf2.c8c10e0a.6db5.ffff9d60SMTPIN_ADDED_BROKEN@mx.google.com
On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the backup
database starts.FATAL: could not connect to the primary server: missing "=" after "1'"
in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' '
I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also makes
the
continuos double quote('') as single quote(').
By using the same connection string while connecting to primary server the
function "conninfo_parse" the escape quotes are not able to parse properly
and it is leading
to problem.
please correct me if any thing wrong in my observation.
Regards,
Hari babu.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jan 24, 2013 at 7:04 AM, Hari Babu <haribabu.kommi@huawei.com> wrote:
On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the backup
database starts.FATAL: could not connect to the primary server: missing "=" after "1'"
in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' 'I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also makes
the
continuos double quote('') as single quote(').By using the same connection string while connecting to primary server the
function "conninfo_parse" the escape quotes are not able to parse properly
and it is leading
to problem.please correct me if any thing wrong in my observation.
Well, it's clearly broken at least :O
Zoltan, do you have time to look at it? I won't have time until at
least after FOSDEM, unfortunately.
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Import Notes
Reply to msg id not found: 5100cefe.a952420a.4c1f.ffffd111SMTPIN_ADDED_BROKEN@mx.google.comReference msg id not found: 50ffaaf2.c8c10e0a.6db5.ffff9d60SMTPIN_ADDED_BROKEN@mx.google.com
2013-01-29 11:15 keltezéssel, Magnus Hagander írta:
On Thu, Jan 24, 2013 at 7:04 AM, Hari Babu <haribabu.kommi@huawei.com> wrote:
On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the backup
database starts.FATAL: could not connect to the primary server: missing "=" after "1'"
in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' 'I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also makes
the
continuos double quote('') as single quote(').By using the same connection string while connecting to primary server the
function "conninfo_parse" the escape quotes are not able to parse properly
and it is leading
to problem.please correct me if any thing wrong in my observation.
Well, it's clearly broken at least :O
Zoltan, do you have time to look at it? I won't have time until at
least after FOSDEM, unfortunately.
I looked at it shortly. What I tried first is adding another pair of single
quotes manually like this:
primary_conninfo = 'user=''user1'' password=''use''''1'' host=''192.168.1.2''
port=''5432'' sslmode=''disable'' sslcompression=''1'' '
But it doesn't solve the problem either, I got:
FATAL: could not connect to the primary server: missing "=" after "'1'" in connection
info string
This worked though:
primary_conninfo = 'user=user1 password=use\'1 host=192.168.1.2 port=5432 sslmode=disable
sslcompression=1 '
When I added an elog() to print the conninfo string in libpqrcv_connect(),
I saw that the double quotes were properly eliminated by ParseConfigFp()
in the first case.
So, there is a bug in generating recovery.conf by not double-escaping
the values and another bug in parsing the connection string in libpq
when the parameter value starts with a single-quote character.
Attached are two patches to fix these two bugs, the libpq part can
be back-patched.
Best regards,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
Attachments:
01-conninfo-fix-libpq.patchtext/x-patch; name=01-conninfo-fix-libpq.patchDownload
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index eea9c6b..6528f77 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4216,9 +4216,12 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
}
if (*cp == '\'')
{
- *cp2 = '\0';
cp++;
- break;
+ if (*cp != '\'')
+ {
+ *cp2 = '\0';
+ break;
+ }
}
*cp2++ = *cp++;
}
02-conninfo-fix-pg_basebackup.patchtext/x-patch; name=02-conninfo-fix-pg_basebackup.patchDownload
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index fb5a1bd..1c2ef9a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1127,6 +1127,7 @@ escape_quotes(const char *src)
static void
GenerateRecoveryConf(PGconn *conn)
{
+ PQExpBufferData arg_buf;
PQconninfoOption *connOptions;
PQconninfoOption *option;
@@ -1137,6 +1138,13 @@ GenerateRecoveryConf(PGconn *conn)
disconnect_and_exit(1);
}
+ initPQExpBuffer(&arg_buf);
+ if (PQExpBufferDataBroken(arg_buf))
+ {
+ fprintf(stderr, _("%s: out of memory"), progname);
+ disconnect_and_exit(1);
+ }
+
connOptions = PQconninfo(conn);
if (connOptions == NULL)
{
@@ -1150,6 +1158,7 @@ GenerateRecoveryConf(PGconn *conn)
for (option = connOptions; option && option->keyword; option++)
{
char *escaped;
+ char *escaped2;
/*
* Do not emit this setting if: - the setting is "replication",
@@ -1169,10 +1178,13 @@ GenerateRecoveryConf(PGconn *conn)
* necessary and doubled single quotes around the value string.
*/
escaped = escape_quotes(option->val);
+ printfPQExpBuffer(&arg_buf, "%s='%s'", option->keyword, escaped);
- appendPQExpBuffer(recoveryconfcontents, "%s=''%s'' ", option->keyword, escaped);
+ escaped2 = escape_quotes(arg_buf.data);
+ appendPQExpBuffer(recoveryconfcontents, "%s ", escaped2);
free(escaped);
+ free(escaped2);
}
appendPQExpBufferStr(recoveryconfcontents, "'\n");
@@ -1183,6 +1195,7 @@ GenerateRecoveryConf(PGconn *conn)
}
PQconninfoFree(connOptions);
+ termPQExpBuffer(&arg_buf);
}
On Monday, February 18, 2013 8:06 PM Boszormenyi Zoltan wrote:
On 2013-01-29 11:15 keltezéssel, Magnus Hagander írta:
On Thu, Jan 24, 2013 at 7:04 AM, Hari Babu wrote:
On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser
login password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the
backup database starts.FATAL: could not connect to the primary server: missing "=" after
"1'"
in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' 'I observed the problem is while reading primary_conninfo from the
recovery.conf file the function "GUC_scanstr" removes the quotes of
the string and also makes the continuos double quote('') as single
quote(').By using the same connection string while connecting to primary
server the function "conninfo_parse" the escape quotes are not able
to parse properly and it is leading to problem.please correct me if any thing wrong in my observation.
Well, it's clearly broken at least :O
So, there is a bug in generating recovery.conf by not double-escaping the
values and another bug >in parsing the connection string in libpq when the
parameter value starts with a single-quote >character.
Attached are two patches to fix these two bugs, the libpq part can be
back-patched.
With the attached patches I tested the defect and it is fixed.
Regards,
Hari babu.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 18.02.2013 16:35, Boszormenyi Zoltan wrote:
2013-01-29 11:15 keltezéssel, Magnus Hagander írta:
On Thu, Jan 24, 2013 at 7:04 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the backup
database starts.FATAL: could not connect to the primary server: missing "=" after "1'"
in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' 'I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also
makes
the
continuos double quote('') as single quote(').By using the same connection string while connecting to primary
server the
function "conninfo_parse" the escape quotes are not able to parse
properly
and it is leading
to problem.please correct me if any thing wrong in my observation.
Well, it's clearly broken at least :O
Zoltan, do you have time to look at it? I won't have time until at
least after FOSDEM, unfortunately.I looked at it shortly. What I tried first is adding another pair of single
quotes manually like this:primary_conninfo = 'user=''user1'' password=''use''''1''
host=''192.168.1.2'' port=''5432'' sslmode=''disable''
sslcompression=''1'' 'But it doesn't solve the problem either, I got:
FATAL: could not connect to the primary server: missing "=" after "'1'"
in connection info stringThis worked though:
primary_conninfo = 'user=user1 password=use\'1 host=192.168.1.2
port=5432 sslmode=disable sslcompression=1 'When I added an elog() to print the conninfo string in libpqrcv_connect(),
I saw that the double quotes were properly eliminated by ParseConfigFp()
in the first case.So, there is a bug in generating recovery.conf by not double-escaping
the values and another bug in parsing the connection string in libpq
when the parameter value starts with a single-quote character.
No, the libpq connection string parser is working as intended. Per the
docs on PQconnectdb:
The passed string can be empty to use all default parameters, or it
can contain one or more parameter settings separated by whitespace.
Each parameter setting is in the form keyword = value. Spaces around
the equal sign are optional. To write an empty value, or a value
containing spaces, surround it with single quotes, e.g., keyword = 'a
value'. Single quotes and backslashes within the value must be
escaped with a backslash, i.e., \' and \\.
So, the proper way to escape a quote in a libpq connection string is \',
not ''. There are two escaping systems layered on top of each other; the
recovery.conf parser's, where you use '', and the libpq system, where
you use \'. So we need two different escaping functions in pg_basebackup
to get this right.
Apart from that, does it bother anyone else that the the
primary_conninfo line that pg_basebackup creates is butt-ugly?
primary_conninfo = 'user=''heikki'' host=''localhost'' port=''5432''
sslmode=''prefer'' sslcompression=''1'' '
We can't avoid quoting option values that need it, but that's probably
very rare in practice. I think we should work a bit harder and leave out
the quotes where not necessary. Also, do we really need to include the
ssl options when they are the defaults?
I think the attached patch fixes the original test scenario correctly,
without changing libpq's quoting rules, and only quotes when necessary.
I didn't do anything about the ssl options. Please take a look.
- Heikki
Attachments:
conninfo-fix-pg_basebackup-2.patchtext/x-diff; name=conninfo-fix-pg_basebackup-2.patchDownload
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 4558506..84c3497 100644
--- 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);
}
2013-05-17 16:05 keltezéssel, Heikki Linnakangas írta:
On 18.02.2013 16:35, Boszormenyi Zoltan wrote:
2013-01-29 11:15 keltezéssel, Magnus Hagander írta:
On Thu, Jan 24, 2013 at 7:04 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the backup
database starts.FATAL: could not connect to the primary server: missing "=" after "1'"
in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' 'I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also
makes
the
continuos double quote('') as single quote(').By using the same connection string while connecting to primary
server the
function "conninfo_parse" the escape quotes are not able to parse
properly
and it is leading
to problem.please correct me if any thing wrong in my observation.
Well, it's clearly broken at least :O
Zoltan, do you have time to look at it? I won't have time until at
least after FOSDEM, unfortunately.I looked at it shortly. What I tried first is adding another pair of single
quotes manually like this:primary_conninfo = 'user=''user1'' password=''use''''1''
host=''192.168.1.2'' port=''5432'' sslmode=''disable''
sslcompression=''1'' 'But it doesn't solve the problem either, I got:
FATAL: could not connect to the primary server: missing "=" after "'1'"
in connection info stringThis worked though:
primary_conninfo = 'user=user1 password=use\'1 host=192.168.1.2
port=5432 sslmode=disable sslcompression=1 'When I added an elog() to print the conninfo string in libpqrcv_connect(),
I saw that the double quotes were properly eliminated by ParseConfigFp()
in the first case.So, there is a bug in generating recovery.conf by not double-escaping
the values and another bug in parsing the connection string in libpq
when the parameter value starts with a single-quote character.No, the libpq connection string parser is working as intended. Per the docs on PQconnectdb:
The passed string can be empty to use all default parameters, or it
can contain one or more parameter settings separated by whitespace.
Each parameter setting is in the form keyword = value. Spaces around
the equal sign are optional. To write an empty value, or a value
containing spaces, surround it with single quotes, e.g., keyword = 'a
value'. Single quotes and backslashes within the value must be
escaped with a backslash, i.e., \' and \\.So, the proper way to escape a quote in a libpq connection string is \', not ''. There
are two escaping systems layered on top of each other; the recovery.conf parser's, where
you use '', and the libpq system, where you use \'. So we need two different escaping
functions in pg_basebackup to get this right.
Why is extending the libpq parser to allow doubling the single
quotes not a good solution? It would be consistent in this regard
with the recovery.conf parser. From this POV the first patch only
needs a little change in the libpq docs.
Also, my patch for libpq only changed the libpq parser when the
parameter argument starts with \'. So in that state the doubled
single quotes should be an accepted escaping in the middle of
the string, too. If you look at the code, there are two branches
that process the argument differently depending on whether
it starts with a \' or not.
Apart from that, does it bother anyone else that the the primary_conninfo line that
pg_basebackup creates is butt-ugly?primary_conninfo = 'user=''heikki'' host=''localhost'' port=''5432'' sslmode=''prefer''
sslcompression=''1'' '
Not a single bit. It's machine generated and the code is generic.
The parser can deal with it.
We can't avoid quoting option values that need it, but that's probably very rare in
practice. I think we should work a bit harder and leave out the quotes where not necessary.
Maybe. :-)
Also, do we really need to include the ssl options when they are the defaults?
I think we were through about this bullet point. IIRC the reasoning and
the consensus was that the backup and the generated recovery.conf
should also work on another machine with a possibly different set of
compilation options for libpq and envvars present on the system.
I think the attached patch fixes the original test scenario correctly, without changing
libpq's quoting rules, and only quotes when necessary. I didn't do anything about the
ssl options. Please take a look.
Your patch should work, too.
But as a separate issue, please consider my reasoning for the libpq patch.
Best regards,
Zoltán Böszörményi
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 17.05.2013 19:03, Boszormenyi Zoltan wrote:
2013-05-17 16:05 keltezéssel, Heikki Linnakangas írta:
On 18.02.2013 16:35, Boszormenyi Zoltan wrote:
2013-01-29 11:15 keltezéssel, Magnus Hagander írta:
On Thu, Jan 24, 2013 at 7:04 AM, Hari Babu <haribabu.kommi@huawei.com>
wrote:On Wed, Jan 23, 2013 11:48 PM, Magnus Hagander wrote:
On Wed, Jan 23, 2013 at 10:18 AM, Hari Babu
<haribabu.kommi@huawei.com>wrote:
Test scenario to reproduce:
1. Start the server
2. create the user as follows
./psql postgres -c "create user user1 superuser login
password 'use''1'"3. Take the backup with -R option as follows.
./pg_basebackup -D ../../data1 -R -U user1 -WThe following errors are occurring when the new standby on the
backup
database starts.FATAL: could not connect to the primary server: missing "=" after
"1'"in
connection info string
What does the resulting recovery.conf file look like?
The recovery.conf which is generated is as follows
standby_mode = 'on'
primary_conninfo = 'user=''user1'' password=''use''1'' port=''5432'' 'I observed the problem is while reading primary_conninfo from the
recovery.conf file
the function "GUC_scanstr" removes the quotes of the string and also
makes
the
continuos double quote('') as single quote(').By using the same connection string while connecting to primary
server the
function "conninfo_parse" the escape quotes are not able to parse
properly
and it is leading
to problem.please correct me if any thing wrong in my observation.
Well, it's clearly broken at least :O
Zoltan, do you have time to look at it? I won't have time until at
least after FOSDEM, unfortunately.I looked at it shortly. What I tried first is adding another pair of
single
quotes manually like this:primary_conninfo = 'user=''user1'' password=''use''''1''
host=''192.168.1.2'' port=''5432'' sslmode=''disable''
sslcompression=''1'' 'But it doesn't solve the problem either, I got:
FATAL: could not connect to the primary server: missing "=" after "'1'"
in connection info stringThis worked though:
primary_conninfo = 'user=user1 password=use\'1 host=192.168.1.2
port=5432 sslmode=disable sslcompression=1 'When I added an elog() to print the conninfo string in
libpqrcv_connect(),
I saw that the double quotes were properly eliminated by ParseConfigFp()
in the first case.So, there is a bug in generating recovery.conf by not double-escaping
the values and another bug in parsing the connection string in libpq
when the parameter value starts with a single-quote character.No, the libpq connection string parser is working as intended. Per the
docs on PQconnectdb:The passed string can be empty to use all default parameters, or it
can contain one or more parameter settings separated by whitespace.
Each parameter setting is in the form keyword = value. Spaces around
the equal sign are optional. To write an empty value, or a value
containing spaces, surround it with single quotes, e.g., keyword = 'a
value'. Single quotes and backslashes within the value must be
escaped with a backslash, i.e., \' and \\.So, the proper way to escape a quote in a libpq connection string is
\', not ''. There are two escaping systems layered on top of each
other; the recovery.conf parser's, where you use '', and the libpq
system, where you use \'. So we need two different escaping functions
in pg_basebackup to get this right.Why is extending the libpq parser to allow doubling the single
quotes not a good solution? It would be consistent in this regard
with the recovery.conf parser. From this POV the first patch only
needs a little change in the libpq docs.
I guess that would work too, but I don't see any meaningful advantage in
doing that.
Apart from that, does it bother anyone else that the the
primary_conninfo line that pg_basebackup creates is butt-ugly?primary_conninfo = 'user=''heikki'' host=''localhost'' port=''5432''
sslmode=''prefer'' sslcompression=''1'' 'Not a single bit. It's machine generated and the code is generic.
The parser can deal with it.
Oh, ok :-). Well, IMO that's really ugly; the file ought to be
human-readable too.
Also, do we really need to include the ssl options when they are the
defaults?I think we were through about this bullet point. IIRC the reasoning and
the consensus was that the backup and the generated recovery.conf
should also work on another machine with a possibly different set of
compilation options for libpq and envvars present on the system.
Ok.
I think the attached patch fixes the original test scenario correctly,
without changing libpq's quoting rules, and only quotes when
necessary. I didn't do anything about the ssl options. Please take a
look.Your patch should work, too.
Thanks, I've applied that.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers