From 23a78352c0a0dc21d6120bd868f0b2d07395b537 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 24 Feb 2015 13:41:07 -0500 Subject: [PATCH] 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. Add tests that fail when a too-long file name or symlink is attempted to be backed up. Reviewed-by: Robert Hass --- src/backend/replication/basebackup.c | 21 +++++++++++++++++++- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 15 +++++++++++++- src/include/pgtar.h | 10 +++++++++- src/port/tar.c | 10 +++++++++- 4 files changed, 52 insertions(+), 4 deletions(-) diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index 3058ce921b..3563fd997f 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -1258,11 +1258,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/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index c966de0b74..7e9a776840 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 strict; 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,6 +49,13 @@ command_ok([ 'pg_basebackup', '-D', "$tempdir/tarbackup", '-Ft' ], '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 @@ -117,3 +124,9 @@ command_fails( 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;"; diff --git a/src/include/pgtar.h b/src/include/pgtar.h index 20e461091a..906db7cebc 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 8ef4f9c388..4721df3ddc 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.40.0