On 10/20/14 4:51 PM, Peter Eisentraut wrote:
> On 10/20/14 2:59 PM, Tom Lane wrote:
>> What do we want to do about this? I think a minimum expectation would be
>> for pg_basebackup to notice and complain when it's trying to create an
>> unworkably long symlink entry, but it would be far better if we found a
>> way to cope instead.
>
> Isn't it the backend that should error out before sending truncated
> files names?
>
> src/port/tar.c:
>
> /* Name 100 */
> sprintf(&h[0], "%.99s", filename);
Here are patches to address that. First, it reports errors when
attempting to create a tar header that would truncate file or symlink
names. Second, it works around the problem in the tests by creating a
symlink from the short-name tempdir that we had set up for the
Unix-socket directory case.
The first patch can be backpatched to 9.3. The tar code before that is
different and would need manual adjustments.
If someone has a too-long tablespace path, I think they can work around
that after this patch by creating a shorter symlink and updating the
pg_tblspc symlinks to point there.
From 16d5eb9514c89391e6c2361212363a7ac2f16e0b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 4 Nov 2014 15:19:18 -0500
Subject: [PATCH 1/2] Error when creating names too long for tar format
The tar format (at least the version we are using), does not support
file names or symlink targets longer than 99 bytes. Until now, the tar
creation code would silently truncate any names that are too long. (Its
original application was pg_dump, where this never happens.) This
creates problems when running base backups over the replication
protocol.
The most important problem is when a tablespace path is longer than 99
bytes, which will result in a truncated tablespace path being backed up.
Less importantly, the basebackup protocol also promises to back up any
other files it happens to find in the data directory, which would also
lead to file name truncation if someone put a file with a long name in
there.
Now both of these cases result in an error during the backup.
---
src/backend/replication/basebackup.c | 21 ++++++++++++++++++++-
src/include/pgtar.h | 10 +++++++++-
src/port/tar.c | 10 +++++++++-
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index fbcecbb..5835725 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -1234,11 +1234,30 @@ _tarWriteHeader(const char *filename, const char *linktarget,
struct stat * statbuf)
{
char h[512];
+ enum tarError rc;
- tarCreateHeader(h, filename, linktarget, statbuf->st_size,
+ rc = tarCreateHeader(h, filename, linktarget, statbuf->st_size,
statbuf->st_mode, statbuf->st_uid, statbuf->st_gid,
statbuf->st_mtime);
+ switch (rc)
+ {
+ case TAR_OK:
+ break;
+ case TAR_NAME_TOO_LONG:
+ ereport(ERROR,
+ (errmsg("file name too long for tar format: \"%s\"",
+ filename)));
+ break;
+ case TAR_SYMLINK_TOO_LONG:
+ ereport(ERROR,
+ (errmsg("symbolic link target too long for tar format: file name \"%s\", target \"%s\"",
+ filename, linktarget)));
+ break;
+ default:
+ elog(ERROR, "unrecognized tar error: %d", rc);
+ }
+
pq_putmessage('d', h, 512);
}
diff --git a/src/include/pgtar.h b/src/include/pgtar.h
index 2dd6f6b..6b6c1e1 100644
--- a/src/include/pgtar.h
+++ b/src/include/pgtar.h
@@ -11,5 +11,13 @@
*
*-------------------------------------------------------------------------
*/
-extern void tarCreateHeader(char *h, const char *filename, const char *linktarget, size_t size, mode_t mode, uid_t uid, gid_t gid, time_t mtime);
+
+enum tarError
+{
+ TAR_OK = 0,
+ TAR_NAME_TOO_LONG,
+ TAR_SYMLINK_TOO_LONG
+};
+
+extern enum tarError tarCreateHeader(char *h, const char *filename, const char *linktarget, size_t size, mode_t mode, uid_t uid, gid_t gid, time_t mtime);
extern int tarChecksum(char *header);
diff --git a/src/port/tar.c b/src/port/tar.c
index 09fd6c1..74168e8 100644
--- a/src/port/tar.c
+++ b/src/port/tar.c
@@ -49,10 +49,16 @@ tarChecksum(char *header)
* must always have space for 512 characters, which is a requirement by
* the tar format.
*/
-void
+enum tarError
tarCreateHeader(char *h, const char *filename, const char *linktarget,
size_t size, mode_t mode, uid_t uid, gid_t gid, time_t mtime)
{
+ if (strlen(filename) > 99)
+ return TAR_NAME_TOO_LONG;
+
+ if (linktarget && strlen(linktarget) > 99)
+ return TAR_SYMLINK_TOO_LONG;
+
/*
* Note: most of the fields in a tar header are not supposed to be
* null-terminated. We use sprintf, which will write a null after the
@@ -141,4 +147,6 @@ tarCreateHeader(char *h, const char *filename, const char *linktarget,
* 6 digits, a space, and a null, which is legal per POSIX.
*/
sprintf(&h[148], "%06o ", tarChecksum(h));
+
+ return TAR_OK;
}
--
2.1.3
From 25f6daded3a4be8182af2d185f4826708f7a1b76 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Tue, 4 Nov 2014 15:27:46 -0500
Subject: [PATCH 2/2] pg_basebackup: Adjust tests for long file name issues
Add tests that fail when a too-long file name or symlink is attempted to
be backed up.
Work around accidental test failures because the working directory path
is too long by creating a temporary directory in the (hopefully shorter)
system location, symlinking that to the working directory, and creating
the tablespaces using the shorter path.
---
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 30 +++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index fa2627b..7e9a776 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -2,7 +2,7 @@
use warnings;
use Cwd;
use TestLib;
-use Test::More tests => 33;
+use Test::More tests => 35;
program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup');
@@ -49,8 +49,22 @@
'tar format');
ok(-f "$tempdir/tarbackup/base.tar", 'backup tar was created');
+my $superlongname = "superlongname_" . ("x"x100);
+
+system_or_bail 'touch', "$tempdir/pgdata/$superlongname";
+command_fails([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l1", '-Ft' ],
+ 'pg_basebackup tar with long name fails');
+unlink "$tempdir/pgdata/$superlongname";
+
+# Create a temporary directory in the system location and symlink it
+# to our physical temp location. That way we can use shorter names
+# for the tablespace directories, which hopefully won't run afoul of
+# the 99 character length limit.
+my $shorter_tempdir = tempdir_short . "/tempdir";
+symlink "$tempdir", $shorter_tempdir;
+
mkdir "$tempdir/tblspc1";
-psql 'postgres', "CREATE TABLESPACE tblspc1 LOCATION '$tempdir/tblspc1';";
+psql 'postgres', "CREATE TABLESPACE tblspc1 LOCATION '$shorter_tempdir/tblspc1';";
psql 'postgres', "CREATE TABLE test1 (a int) TABLESPACE tblspc1;";
command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup2", '-Ft' ],
'tar format with tablespaces');
@@ -65,7 +79,7 @@
command_ok(
[ 'pg_basebackup', '-D',
"$tempdir/backup1", '-Fp',
- "-T$tempdir/tblspc1=$tempdir/tbackup/tblspc1" ],
+ "-T$shorter_tempdir/tblspc1=$tempdir/tbackup/tblspc1" ],
'plain format with tablespaces succeeds with tablespace mapping');
ok(-d "$tempdir/tbackup/tblspc1", 'tablespace was relocated');
opendir(my $dh, "$tempdir/pgdata/pg_tblspc") or die;
@@ -81,11 +95,11 @@
mkdir "$tempdir/tbl=spc2";
psql 'postgres', "DROP TABLE test1;";
psql 'postgres', "DROP TABLESPACE tblspc1;";
-psql 'postgres', "CREATE TABLESPACE tblspc2 LOCATION '$tempdir/tbl=spc2';";
+psql 'postgres', "CREATE TABLESPACE tblspc2 LOCATION '$shorter_tempdir/tbl=spc2';";
command_ok(
[ 'pg_basebackup', '-D',
"$tempdir/backup3", '-Fp',
- "-T$tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2" ],
+ "-T$shorter_tempdir/tbl\\=spc2=$tempdir/tbackup/tbl\\=spc2" ],
'mapping tablespace with = sign in path');
ok(-d "$tempdir/tbackup/tbl=spc2", 'tablespace with = sign was relocated');
@@ -110,3 +124,9 @@
command_fails(
[ 'pg_basebackup', '-D', "$tempdir/backup_foo", '-Fp', "-Tfoo" ],
'-T with invalid format fails');
+
+mkdir "$tempdir/$superlongname";
+psql 'postgres', "CREATE TABLESPACE tblspc3 LOCATION '$tempdir/$superlongname';";
+command_fails([ 'pg_basebackup', '-D', "$tempdir/tarbackup_l3", '-Ft' ],
+ 'pg_basebackup tar with long symlink target fails');
+psql 'postgres', "DROP TABLESPACE tblspc3;";
--
2.1.3
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
