Re: Re: pg_basebackup with -R option and start standby have problems with escaped password - Mailing list pgsql-hackers
From | Heikki Linnakangas |
---|---|
Subject | Re: Re: pg_basebackup with -R option and start standby have problems with escaped password |
Date | |
Msg-id | 519A548D.6040806@vmware.com Whole thread Raw |
In response to | Re: Re: pg_basebackup with -R option and start standby have problems with escaped password (Boszormenyi Zoltan <zb@cybertec.at>) |
List | 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 -W >>>>>>> >>>>>>> The 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. >> >> 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
pgsql-hackers by date: