Re: [Bug-tar] --backup can destroy an extracted member when followed by an existing directory

[Sergey Poznyakoff]

Carl Worth <address@hidden> ha escrit:

> I've traced through the code a bit and the problem is due to the
> handling of directory entries in extract.c:extract_archive().

Thanks for the in-depth analysis. I have applied the attached patch.

Regards,
Sergey

>From 755c246588092d0b281cb610a8371c2c9b32de59 Mon Sep 17 00:00:00 2001
From: Sergey Poznyakoff <address@hidden>
Date: Wed, 5 Aug 2009 10:38:50 +0300
Subject: [PATCH] Fix backup handling and restoring file modes of existing 
directories

* NEWS, THANKS: Update
* src/extract.c (extract_dir): reset status to 0 if the
directory already exists.
* src/misc.c (maybe_backup_file): Assign before_backup_name
and clear after_backup_name before checking if we really need
to backup the file.
* tests/backup01.at: New testcase.
* tests/extrac08.at: New testcase.
* tests/Makefile.am, tests/testsuite.at: Add extrac08.at and
backup01.at
---
 NEWS               |    8 +++++++-
 THANKS             |    1 +
 src/extract.c      |    1 +
 src/misc.c         |   17 +++++++++--------
 tests/Makefile.am  |    2 ++
 tests/backup01.at  |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 tests/extrac08.at  |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/testsuite.at |    3 +++
 8 files changed, 123 insertions(+), 9 deletions(-)
 create mode 100644 tests/backup01.at
 create mode 100644 tests/extrac08.at

diff --git a/NEWS b/NEWS
index e9a31ca..9c15ad7 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-GNU tar NEWS - User visible changes. 2009-05-25
+GNU tar NEWS - User visible changes. 2009-08-05
 Please send GNU tar bug reports to <address@hidden>
 
 
@@ -10,6 +10,12 @@ When listing or extracting archives, the actual record size 
is
 reported only if the archive is read from a device (as opposed
 to regular files and pipes).
 
+* Bugfixes
+** Fix handling of hard link targets by -c --transform.
+** Fix hard links recognition with -c --remove-files.
+** Fix restoring files from backup (debian bug #508199).
+** Correctly restore modes and permissions on existing directories.
+
 
 version 1.22 - Sergey Poznyakoff, 2009-03-05
 
diff --git a/THANKS b/THANKS
index dbb64d2..9d918ba 100644
--- a/THANKS
+++ b/THANKS
@@ -70,6 +70,7 @@ Burkhard Plache               address@hidden
 Calvin Cliff           address@hidden
 Cameron Elliott                address@hidden
 Carl Streeter          address@hidden
+Carl Worth              address@hidden
 Carsten Heyl           address@hidden
 Catrin Urbanneck       address@hidden
 Cesar Romani           address@hidden
diff --git a/src/extract.c b/src/extract.c
index 63f3525..5361506 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -682,6 +682,7 @@ extract_dir (char *file_name, int typeflag)
                }
              if (S_ISDIR (st.st_mode))
                {
+                 status = 0;
                  mode = st.st_mode;
                  break;
                }
diff --git a/src/misc.c b/src/misc.c
index 951449e..b609b86 100644
--- a/src/misc.c
+++ b/src/misc.c
@@ -417,6 +417,15 @@ maybe_backup_file (const char *file_name, bool 
this_is_the_archive)
 {
   struct stat file_stat;
 
+  assign_string (&before_backup_name, file_name);
+
+  /* A run situation may exist between Emacs or other GNU programs trying to
+     make a backup for the same file simultaneously.  If theoretically
+     possible, real problems are unlikely.  Doing any better would require a
+     convention, GNU-wide, for all programs doing backups.  */
+
+  assign_string (&after_backup_name, 0);
+
   /* Check if we really need to backup the file.  */
 
   if (this_is_the_archive && _remdev (file_name))
@@ -438,14 +447,6 @@ maybe_backup_file (const char *file_name, bool 
this_is_the_archive)
       && (S_ISBLK (file_stat.st_mode) || S_ISCHR (file_stat.st_mode)))
     return true;
 
-  assign_string (&before_backup_name, file_name);
-
-  /* A run situation may exist between Emacs or other GNU programs trying to
-     make a backup for the same file simultaneously.  If theoretically
-     possible, real problems are unlikely.  Doing any better would require a
-     convention, GNU-wide, for all programs doing backups.  */
-
-  assign_string (&after_backup_name, 0);
   after_backup_name = find_backup_file_name (file_name, backup_type);
   if (! after_backup_name)
     xalloc_die ();
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 2001834..64b1730 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -52,6 +52,7 @@ TESTSUITE_AT = \
  append.at\
  append01.at\
  append02.at\
+ backup01.at\
  chtype.at\
  comprec.at\
  delete01.at\
@@ -67,6 +68,7 @@ TESTSUITE_AT = \
  extrac05.at\
  extrac06.at\
  extrac07.at\
+ extrac08.at\
  gzip.at\
  grow.at\
  incremental.at\
diff --git a/tests/backup01.at b/tests/backup01.at
new file mode 100644
index 0000000..538dd3d
--- /dev/null
+++ b/tests/backup01.at
@@ -0,0 +1,49 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+# Description:
+# When extracting, tar (<1.22) sometimes renamed a backup back to the
+# original name (overwriting the newly-extracted file) for no good reason.
+#
+# Reported by: Carl Worth <address@hidden>,
+#              Eric Lammerts <address@hidden>
+#
+# References: <address@hidden>
+#             http://lists.gnu.org/archive/html/bug-tar/2009-08/msg00005.html
+#             http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508199
+#
+
+AT_SETUP([extracting existing dir with --backup])
+AT_KEYWORDS([extract backup backup01])
+
+AT_TAR_CHECK([
+mkdir dir1 dir2
+echo bla > dir1/file1
+tar cf test.tar dir1 dir2
+tar xfv test.tar --backup
+],
+[0],
+[dir1/
+dir1/file1
+Renaming `dir1/file1' to `dir1/file1~'
+dir2/
+])
+
+AT_CLEANUP
\ No newline at end of file
diff --git a/tests/extrac08.at b/tests/extrac08.at
new file mode 100644
index 0000000..2bcd10c
--- /dev/null
+++ b/tests/extrac08.at
@@ -0,0 +1,51 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+
+# Test suite for GNU tar.
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3, or (at your option)
+# any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+# 02110-1301, USA.
+
+# Description:
+# When extracting, tar (<1.22) did not restore directory permissions on
+# existing directories.
+# This was discovered when fixing debian bug #508199. See also backup01.at.
+#
+# Reported by: Carl Worth <address@hidden>,
+#
+# References: <address@hidden>
+#             http://lists.gnu.org/archive/html/bug-tar/2009-08/msg00005.html
+#             http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=508199
+#
+
+AT_SETUP([restoring mode on existing directory])
+AT_KEYWORDS([extract extrac08])
+
+AT_TAR_CHECK([
+mkdir dir
+chmod 755 dir
+echo bla > dir/file
+tar cf test.tar dir
+chmod 700 dir
+tar xfv test.tar
+genfile --stat=mode.777 dir
+],
+[0],
+[dir/
+dir/file
+755
+])
+
+AT_CLEANUP
\ No newline at end of file
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 634133b..2c87281 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -139,6 +139,9 @@ m4_include([extrac04.at])
 m4_include([extrac05.at])
 m4_include([extrac06.at])
 m4_include([extrac07.at])
+m4_include([extrac08.at])
+
+m4_include([backup01.at])
 
 m4_include([gzip.at])
 
-- 
1.6.0