• Tom Lane's avatar
    Code review for server's handling of "tablespace map" files. · 8620a7f6
    Tom Lane authored
    While looking at Robert Foggia's report, I noticed a passel of
    other issues in the same area:
    
    * The scheme for backslash-quoting newlines in pathnames is just
    wrong; it will misbehave if the last ordinary character in a pathname
    is a backslash.  I'm not sure why we're bothering to allow newlines
    in tablespace paths, but if we're going to do it we should do it
    without introducing other problems.  Hence, backslashes themselves
    have to be backslashed too.
    
    * The author hadn't read the sscanf man page very carefully, because
    this code would drop any leading whitespace from the path.  (I doubt
    that a tablespace path with leading whitespace could happen in
    practice; but if we're bothering to allow newlines in the path, it
    sure seems like leading whitespace is little less implausible.)  Using
    sscanf for the task of finding the first space is overkill anyway.
    
    * While I'm not 100% sure what the rationale for escaping both \r and
    \n is, if the idea is to allow Windows newlines in the file then this
    code failed, because it'd throw an error if it saw \r followed by \n.
    
    * There's no cross-check for an incomplete final line in the map file,
    which would be a likely apparent symptom of the improper-escaping
    bug.
    
    On the generation end, aside from the escaping issue we have:
    
    * If needtblspcmapfile is true then do_pg_start_backup will pass back
    escaped strings in tablespaceinfo->path values, which no caller wants
    or is prepared to deal with.  I'm not sure if there's a live bug from
    that, but it looks like there might be (given the dubious assumption
    that anyone actually has newlines in their tablespace paths).
    
    * It's not being very paranoid about the possibility of random stuff
    in the pg_tblspc directory.  IMO we should ignore anything without an
    OID-like name.
    
    The escaping rule change doesn't seem back-patchable: it'll require
    doubling of backslashes in the tablespace_map file, which is basically
    a basebackup format change.  The odds of that causing trouble are
    considerably more than the odds of the existing bug causing trouble.
    The rest of this seems somewhat unlikely to cause problems too,
    so no back-patch.
    8620a7f6
basebackup.c 55.2 KB