pg_ctl -D canonicalization

Magnus Hagander wrote:

It seems pg_ctl calls canonicalize_path() only on the path as being used
to access for example the pid file, and not the path that is sent along
to the postmaster.
Specifically, this causes failure on win32 when a path is passed with a
trailing backslash, when it's inside quotes such as:
pg_ctl start -D "c:\program fiels\postgresql\data\"

The quotes are of course necessary since there are spaces in the
filename. In my specific case the trailing backslash is automatically
added by windows installer. but other cases when the backslash is added
can easily be seen...

Attached patch makes pg_ctl call canonicalize_path() on the path as
passed on the commandline as well.

I have only tested this on win32, where it appears to work fine. I think
it would be good on other platsforms as well which is why I didn't
#ifdef it. If not, then please add #ifdefs.

Uh, isn't the proper fix to fix the postmaster's handling of -D paths?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

It seems pg_ctl calls canonicalize_path() only on the path as being
used to access for example the pid file, and not the path

that is sent

along to the postmaster.
Specifically, this causes failure on win32 when a path is

passed with

a trailing backslash, when it's inside quotes such as:
pg_ctl start -D "c:\program fiels\postgresql\data\"

The quotes are of course necessary since there are spaces in the
filename. In my specific case the trailing backslash is

automatically

added by windows installer. but other cases when the backslash is
added can easily be seen...

Attached patch makes pg_ctl call canonicalize_path() on the path as
passed on the commandline as well.

I have only tested this on win32, where it appears to work fine. I
think it would be good on other platsforms as well which is why I
didn't #ifdef it. If not, then please add #ifdefs.

Uh, isn't the proper fix to fix the postmaster's handling of -D paths?

Nope.

The problem is that in this case, pg_ctl gets the path:
c:\program files\postgresql\data\"

(not unbalanced quotes). Yes, win32 commandline quoting is horrible.
Anyway. then it properly quotes this for the commandline:
"c:\program files\postgresql\data\""

and you can see where that breaks down...

Another option would be simple check to simply strip the " (and the
trailing backslash too, probably, to make things safer at the next
step). I just thought it'd be cleaner to use canonicalize_path(). But
it's just the trailing quote stuff that causes the problem. (See the
second step in canonicalize_path() where this has been noticed befoer,
followed by the trim-trailing part which would also be nice).

(Everything works if you register it as a service because then it uses
pg_data and not pgdata_opt. This is just when you start things manually,
which is why we missed it before...)

//Magnus

OK, attached patch applied. Your original patch didn't canonicalize the
PGDATA environment variable. '-D' takes precidence but it should be
accurate.

I also updated the comments in canonicalize_path because it does a lot
more now that just win to unix path conversion.

---------------------------------------------------------------------------

Magnus Hagander wrote:

It seems pg_ctl calls canonicalize_path() only on the path as being
used to access for example the pid file, and not the path

that is sent

along to the postmaster.
Specifically, this causes failure on win32 when a path is

passed with

a trailing backslash, when it's inside quotes such as:
pg_ctl start -D "c:\program fiels\postgresql\data\"

The quotes are of course necessary since there are spaces in the
filename. In my specific case the trailing backslash is

automatically

added by windows installer. but other cases when the backslash is
added can easily be seen...

Attached patch makes pg_ctl call canonicalize_path() on the path as
passed on the commandline as well.

I have only tested this on win32, where it appears to work fine. I
think it would be good on other platsforms as well which is why I
didn't #ifdef it. If not, then please add #ifdefs.

Uh, isn't the proper fix to fix the postmaster's handling of -D paths?

Nope.

The problem is that in this case, pg_ctl gets the path:
c:\program files\postgresql\data\"

(not unbalanced quotes). Yes, win32 commandline quoting is horrible.
Anyway. then it properly quotes this for the commandline:
"c:\program files\postgresql\data\""

and you can see where that breaks down...

Another option would be simple check to simply strip the " (and the
trailing backslash too, probably, to make things safer at the next
step). I just thought it'd be cleaner to use canonicalize_path(). But
it's just the trailing quote stuff that causes the problem. (See the
second step in canonicalize_path() where this has been noticed befoer,
followed by the trim-trailing part which would also be nice).

(Everything works if you register it as a service because then it uses
pg_data and not pgdata_opt. This is just when you start things manually,
which is why we missed it before...)

//Magnus

---------------------------(end of broadcast)---------------------------
TIP 6: Have you searched our list archives?

http://archives.postgresql.org

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Thanks. I can confirm that this fixes the issue.

//Magnus