Re: pg_basebackup vs. Windows and tablespaces - Mailing list pgsql-hackers
| From | Amit Kapila |
|---|---|
| Subject | Re: pg_basebackup vs. Windows and tablespaces |
| Date | |
| Msg-id | CAA4eK1JF072W=ODguH8hJZZUw+-7JN31vmxa9nyho9uNVXZf+Q@mail.gmail.com Whole thread Raw |
| In response to | Re: pg_basebackup vs. Windows and tablespaces (Tom Lane <tgl@sss.pgh.pa.us>) |
| Responses |
Re: pg_basebackup vs. Windows and tablespaces
(Tom Lane <tgl@sss.pgh.pa.us>)
|
| List | pgsql-hackers |
On Mon, Dec 15, 2014 at 5:39 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
> >> On Sat, Dec 13, 2014 at 10:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> 1. The patch is completely naive about what might be in the symlink
> >>> path string; eg embedded spaces in the path would break it. On at
> >>> least some platforms, newlines could be in the path as well. I'm not
> >>> sure about how to guard against this while maintaining human readability
> >>> of the file.
>
> > One way to deal with this could be to append a delimiter(which is not
> > allowed
> > in tablespace path like quote (\')) at the end of tablespace path while
> > writing the same to symlink label file and then use that as end marker while
> > reading it from file.
>
> What makes you think quote isn't allowed in tablespace paths?
Oid
CreateTableSpace(CreateTableSpaceStmt *stmt)
{
..
/* disallow quotes, else CREATE DATABASE would be at risk */
if (strchr(location, '\''))
ereport(ERROR,
(errcode(ERRCODE_INVALID_NAME),
errmsg("tablespace location cannot contain single quotes")));
}
create_tablespace_directories()
{
..
location_with_version_dir = psprintf("%s/%s", location,
TABLESPACE_VERSION_DIRECTORY);
..
}
> But I doubt you can make that argument for spaces, quotes, or backslashes.
>
>
> Amit Kapila <amit.kapila16@gmail.com> writes:
> >> On Sat, Dec 13, 2014 at 10:48 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> 1. The patch is completely naive about what might be in the symlink
> >>> path string; eg embedded spaces in the path would break it. On at
> >>> least some platforms, newlines could be in the path as well. I'm not
> >>> sure about how to guard against this while maintaining human readability
> >>> of the file.
>
> > One way to deal with this could be to append a delimiter(which is not
> > allowed
> > in tablespace path like quote (\')) at the end of tablespace path while
> > writing the same to symlink label file and then use that as end marker while
> > reading it from file.
>
> What makes you think quote isn't allowed in tablespace paths?
Below part of code makes me think that quote is not allowed.
CreateTableSpace(CreateTableSpaceStmt *stmt)
{
..
/* disallow quotes, else CREATE DATABASE would be at risk */
if (strchr(location, '\''))
ereport(ERROR,
(errcode(ERRCODE_INVALID_NAME),
errmsg("tablespace location cannot contain single quotes")));
}
> Even if we
> were to disallow it at the SQL level, there'd be nothing stopping a DBA
> from changing the path after the fact by redefining the symlink outside
> SQL --- something I believe we specifically meant to allow, considering
> we went to the trouble of getting rid of the pg_tablespace.spclocation
> column.
>
> Pretty much the only character we can be entirely certain is not in a
> symlink's value is \0. As Alvaro mentioned, using that in the file
> is a possible alternative, although it could easily confuse some users
> and/or text editors. The only other alternatives I can see are:
>
> * Go over to a byte-count-then-value format. Also possible, also rather
> unfriendly from a user's standpoint.
>
> * Establish an escaping convention, eg backslash before any funny
> characters. Unfortunately backslash wouldn't be too nice from the
> viewpoint of Windows users.
>
> * Make pg_basebackup check for and fail on symlinks containing characters
> it can't handle. Pretty icky, though I suppose there's some argument
> that things like newlines wouldn't be in any rational tablespace path.
> were to disallow it at the SQL level, there'd be nothing stopping a DBA
> from changing the path after the fact by redefining the symlink outside
> SQL --- something I believe we specifically meant to allow, considering
> we went to the trouble of getting rid of the pg_tablespace.spclocation
> column.
>
> Pretty much the only character we can be entirely certain is not in a
> symlink's value is \0. As Alvaro mentioned, using that in the file
> is a possible alternative, although it could easily confuse some users
> and/or text editors. The only other alternatives I can see are:
>
> * Go over to a byte-count-then-value format. Also possible, also rather
> unfriendly from a user's standpoint.
>
> * Establish an escaping convention, eg backslash before any funny
> characters. Unfortunately backslash wouldn't be too nice from the
> viewpoint of Windows users.
>
> * Make pg_basebackup check for and fail on symlinks containing characters
> it can't handle. Pretty icky, though I suppose there's some argument
> that things like newlines wouldn't be in any rational tablespace path.
Yeah, another thing is that during tablespace creation, we use below
code to form tablespace path which prompted me to ask question that
do we allow newline in create tablespace path.
{
..
location_with_version_dir = psprintf("%s/%s", location,
TABLESPACE_VERSION_DIRECTORY);
..
}
Now if above code understand newline in path, then can't we make
some arrangement during file read?
> But I doubt you can make that argument for spaces, quotes, or backslashes.
>
If we disallow newline in symlink path via pg_basebackup path, then we
might be able to use 'Negated scanset' format specifier of fscanf ("%[^\n]s")
to handle other characters.
pgsql-hackers by date: