Thread: recovery.conf parsing problems
While testing a PITR recovery, I discovered that recovery.conf doesn't seem to allow specifying ' in the command string, making it hard to protect the restore_command against problematic filenames (whitespace etc.). This doesn't seem to be a problem for archive_command, which allows \' (e.g. archive_command = '/path/to/script \'%f\' \'%p\''). Should this be fixed? -- Andrew, Supernews http://www.supernews.com - individual and corporate NNTP services
On Wed, 2006-12-13 at 04:23 +0000, Andrew - Supernews wrote: > While testing a PITR recovery, I discovered that recovery.conf doesn't > seem to allow specifying ' in the command string, making it hard to > protect the restore_command against problematic filenames (whitespace > etc.). This doesn't seem to be a problem for archive_command, which > allows \' (e.g. archive_command = '/path/to/script \'%f\' \'%p\''). > Should this be fixed? Yes, I'll look into that. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
On Wed, 2006-12-13 at 19:28 +0000, Simon Riggs wrote: > On Wed, 2006-12-13 at 04:23 +0000, Andrew - Supernews wrote: > > While testing a PITR recovery, I discovered that recovery.conf doesn't > > seem to allow specifying ' in the command string, making it hard to > > protect the restore_command against problematic filenames (whitespace > > etc.). This doesn't seem to be a problem for archive_command, which > > allows \' (e.g. archive_command = '/path/to/script \'%f\' \'%p\''). Immediate workaround is to use a script to encapsulate the actual desired request. > > Should this be fixed? > > Yes, I'll look into that. OK, I would propose to extend the guc-file.l to include sufficient code to allow the parsing of the conf files to be identical between the postgresql.conf and the recovery.conf (it isn't the same yet). There'll be care taken to ensure that the various options are not settable in the wrong file. Any requests for specific implementation details? I'll be looking to remove code from xlog.c, if possible. Implementation won't be immediate because of other current work. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> writes:
> OK, I would propose to extend the guc-file.l to include sufficient code
> to allow the parsing of the conf files to be identical between the
> postgresql.conf and the recovery.conf (it isn't the same yet).
It would probably be far easier for long-term maintenance if you just
built an independent lexer, instead of trying to make guc-file.l
serve multiple masters.
regards, tom lane
On 2006-12-13, "Simon Riggs" <simon@2ndquadrant.com> wrote: > On Wed, 2006-12-13 at 19:28 +0000, Simon Riggs wrote: >> On Wed, 2006-12-13 at 04:23 +0000, Andrew - Supernews wrote: >> > While testing a PITR recovery, I discovered that recovery.conf doesn't >> > seem to allow specifying ' in the command string, making it hard to >> > protect the restore_command against problematic filenames (whitespace >> > etc.). This doesn't seem to be a problem for archive_command, which >> > allows \' (e.g. archive_command = '/path/to/script \'%f\' \'%p\''). > > Immediate workaround is to use a script to encapsulate the actual > desired request. That only helps if you can trust %p not to contain whitespace or $. If it is always relative to somewhere in the data dir then this is probably ok, but if it's an absolute path then you can't assume that. -- Andrew, Supernews http://www.supernews.com - individual and corporate NNTP services
Andrew - Supernews <andrew+nonews@supernews.com> writes:
> That only helps if you can trust %p not to contain whitespace or $. If it
> is always relative to somewhere in the data dir then this is probably ok,
> but if it's an absolute path then you can't assume that.
It is relative, so I think this is actually not a problem. Still, we
ought to make the lexical structure the same as it is in postgresql.conf.
regards, tom lane
On Wed, 2006-12-13 at 17:02 -0500, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> writes: > > OK, I would propose to extend the guc-file.l to include sufficient code > > to allow the parsing of the conf files to be identical between the > > postgresql.conf and the recovery.conf (it isn't the same yet). > > It would probably be far easier for long-term maintenance if you just > built an independent lexer, instead of trying to make guc-file.l > serve multiple masters. Will do. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Simon Riggs wrote: > > It would probably be far easier for long-term maintenance if you > > just built an independent lexer, instead of trying to make > > guc-file.l serve multiple masters. > > Will do. I'm actually not so sure that this is a good idea. The lexical structure should be exactly the same, and some things like include files might become useful as well, so why should all this be replicated? -- Peter Eisentraut http://developer.postgresql.org/~petere/
On Thu, 2006-12-14 at 13:52 +0100, Peter Eisentraut wrote: > Simon Riggs wrote: > > > It would probably be far easier for long-term maintenance if you > > > just built an independent lexer, instead of trying to make > > > guc-file.l serve multiple masters. > > > > Will do. > > I'm actually not so sure that this is a good idea. The lexical > structure should be exactly the same, and some things like include > files might become useful as well, so why should all this be > replicated? I assumed the actual lexer would be the same, just the code that invokes it would be different. I'm happy to do things either way. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> writes:
> On Thu, 2006-12-14 at 13:52 +0100, Peter Eisentraut wrote:
>> I'm actually not so sure that this is a good idea. The lexical
>> structure should be exactly the same, and some things like include
>> files might become useful as well, so why should all this be
>> replicated?
> I assumed the actual lexer would be the same, just the code that invokes
> it would be different. I'm happy to do things either way.
Yeah, but the actual lexer is [ looks... ] less than 50 lines of flex
code. I think refactoring stuff to the point where that could be shared
would be more pain than it's worth ...
regards, tom lane
Added to TODO:
o Allow recovery.conf to allow the same syntax as postgresql.conf, including quoting
http://archives.postgresql.org/pgsql-hackers/2006-12/msg00497.php
---------------------------------------------------------------------------
Simon Riggs wrote:
> On Wed, 2006-12-13 at 19:28 +0000, Simon Riggs wrote:
> > On Wed, 2006-12-13 at 04:23 +0000, Andrew - Supernews wrote:
> > > While testing a PITR recovery, I discovered that recovery.conf doesn't
> > > seem to allow specifying ' in the command string, making it hard to
> > > protect the restore_command against problematic filenames (whitespace
> > > etc.). This doesn't seem to be a problem for archive_command, which
> > > allows \' (e.g. archive_command = '/path/to/script \'%f\' \'%p\'').
>
> Immediate workaround is to use a script to encapsulate the actual
> desired request.
>
> > > Should this be fixed?
> >
> > Yes, I'll look into that.
>
> OK, I would propose to extend the guc-file.l to include sufficient code
> to allow the parsing of the conf files to be identical between the
> postgresql.conf and the recovery.conf (it isn't the same yet).
>
> There'll be care taken to ensure that the various options are not
> settable in the wrong file.
>
> Any requests for specific implementation details? I'll be looking to
> remove code from xlog.c, if possible.
>
> Implementation won't be immediate because of other current work.
>
> --
> Simon Riggs
> EnterpriseDB http://www.enterprisedb.com
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
-- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +