Re: [Bug-tar] --keep-old-files bug

[Sergey Poznyakoff]

I have installed the attached patch.

Regards,
Sergey

>From 7a5a3708cb8b33494cd6ac64bfe163f631a33edf Mon Sep 17 00:00:00 2001
From: Sergey Poznyakoff <address@hidden>
Date: Sat, 26 Nov 2011 15:41:43 +0200
Subject: [PATCH] Fix --keep-old-files option.

The regression was introduced by 8f390db9.  This patch implements additional
option --skip-old-files, which silently skips members which would cause
writing over existing files, and restores --keep-old-files to its traditional
behavior.

* NEWS: Update.
* configure.ac: Update.
* doc/tar.texi: Document the changes.
* src/common.h (SKIP_OLD_FILES): New old_files mode.
* src/extract.c (maybe_recoverable): Restore KEEP_OLD_FILES behavior.
Handle SKIP_OLD_FILES.
* src/tar.c: New option --skip-old-files.
* tests/extrac18.at: New file.
* tests/extrac19.at: New file.
* tests/Makefile.am: Add new test cases.
* tests/testsuite.at: Likewise.
---
 NEWS               |   19 ++++++++++-
 configure.ac       |    2 +-
 doc/tar.texi       |   88 +++++++++++++++++++++++++++++++++++++++------------
 src/common.h       |    1 +
 src/extract.c      |    5 ++-
 src/tar.c          |   13 ++++++-
 tests/Makefile.am  |    2 +
 tests/extrac18.at  |   60 +++++++++++++++++++++++++++++++++++
 tests/extrac19.at  |   44 ++++++++++++++++++++++++++
 tests/testsuite.at |    2 +
 10 files changed, 209 insertions(+), 27 deletions(-)
 create mode 100644 tests/extrac18.at
 create mode 100644 tests/extrac19.at

diff --git a/NEWS b/NEWS
index 06955f4..6226d07 100644
--- a/NEWS
+++ b/NEWS
@@ -1,8 +1,8 @@
-GNU tar NEWS - User visible changes. 2011-08-13
+GNU tar NEWS - User visible changes. 2011-11-26
 Please send GNU tar bug reports to <address@hidden>

 
-version ?.? - ?, 201?-??-??
+version 1.26.90 (Git)

 * New features

@@ -13,6 +13,21 @@ NAME:NUM, so that you can specify both symbolic name and 
numeric ID
 for owner and group.  In these options, NAME no longer needs to be
 present in the current host's user and group databases.

+* The --keep-old-files and --skip-old-files options.
+
+This release restores the traditional functionality of the
+--keep-old-files.  This option causes tar to avoid replacing
+existing files while extracting and to treat such files as errors.
+Tar will emit a prominent error message upon encountering such files
+and will exit with code 2 when finished extracting the archive.
+
+A new option --skip-old-files is introduced, which acts exactly as
+--keep-old-files, except that it does not treat existing files as
+errors.  Instead it just silently skips them.  An additional level of
+verbosity can be obtained by using the option --warning=existing-file
+together with this option.
+
+
 version 1.26 - Sergey Poznyakoff, 2011-03-12

 * Bugfixes
diff --git a/configure.ac b/configure.ac
index db69cb8..b4dc871 100644
--- a/configure.ac
+++ b/configure.ac
@@ -19,7 +19,7 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
 # 02110-1301, USA.

-AC_INIT([GNU tar], [1.26], address@hidden)
+AC_INIT([GNU tar], [1.26.90], address@hidden)
 AC_CONFIG_SRCDIR([src/tar.c])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_HEADERS([config.h])
diff --git a/doc/tar.texi b/doc/tar.texi
index cf30592..1a28cb2 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -1877,6 +1877,7 @@ The other operations of @command{tar} (@option{--list},
 @option{--extract}, @option{--compare}, and @option{--update})
 will act on the entire contents of the archive.

address@hidden status}
 @cindex exit status
 @cindex return status
 Besides successful exits, @GNUTAR{} may fail for
@@ -2808,7 +2809,10 @@ when extracting files from an archive.
 @item --keep-old-files
 @itemx -k

-Do not overwrite existing files when extracting files from an archive.
+Do not overwrite existing files when extracting files from an
+archive.  Return error if such files exist.  See also
address@hidden
+
 @xref{Keep Old Files}.

 @opsummary{label}
@@ -3261,6 +3265,20 @@ the archive creation operations it instructs 
@command{tar} to list the
 member names stored in the archive, as opposed to the actual file
 names.  @xref{listing member and file names}.

address@hidden
address@hidden --skip-old-files
+
+Do not overwrite existing files when extracting files from an
+archive.  @xref{Keep Old Files}.
+
+This option differs from @option{--keep-old-files} in that it does not
+treat such files as an error, instead it just silently avoids
+overwriting them.
+
+The @option{--warning=existing-file} option can be used together with
+this option to produce warning messages about existing old files
+(@pxref{warnings}).
+
 @opsummary{sparse}
 @item --sparse
 @itemx -S
@@ -4436,11 +4454,11 @@ in the archive; the most recently archived members will 
be extracted
 last.  Additionally, an extracted member will @emph{replace} a file of
 the same name which existed in the directory already, and @command{tar}
 will not prompt you about address@hidden you give it
address@hidden option, or the disk copy is newer than
-the one in the archive and you invoke @command{tar} with
address@hidden option.}.  Thus, only the most recently archived
-member will end up being extracted, as it will replace the one
-extracted before it, and so on.
address@hidden (or @option{--skip-old-files}) option, or
+the disk copy is newer than the one in the archive and you invoke
address@hidden with @option{--keep-newer-files} option.}.  Thus, only
+the most recently archived member will end up being extracted, as it
+will replace the one extracted before it, and so on.

 @cindex extracting @var{n}th copy of the file
 @xopindex{occurrence, described}
@@ -5131,10 +5149,25 @@ such a directory, use the @option{--no-overwrite-dir} 
option.
 @cindex Overwriting old files, prevention
 @xopindex{keep-old-files, introduced}
 To be even more cautious and prevent existing files from being replaced, use
-the @option{--keep-old-files} (@option{-k}) option.  It causes @command{tar} 
to refuse
-to replace or update a file that already exists, i.e., a file with the
-same name as an archive member prevents extraction of that archive
-member.  Instead, it reports an error.
+the @option{--keep-old-files} (@option{-k}) option.  It causes
address@hidden to refuse to replace or update a file that already
+exists, i.e., a file with the same name as an archive member prevents
+extraction of that archive member.  Instead, it reports an error.  For
+example:
+
address@hidden
+$ @kbd{ls}
+blues
+$ @kbd{tar -x -k -f archive.tar}
+tar: blues: Cannot open: File exists
+tar: Exiting with failure status due to previous errors
address@hidden example
+
address@hidden, introduced}
+If you wish to preserve old files untouched, but don't want
address@hidden to treat them as errors, use the
address@hidden option.  This option causes @command{tar} to
+silently skip extracting over existing files.

 @xopindex{overwrite, introduced}
 To be more aggressive about altering existing files, use the
@@ -5200,16 +5233,24 @@ archive, but remove other files before extracting.
 @node Keep Old Files
 @unnumberedsubsubsec Keep Old Files

address@hidden provides two options to control its actions in a situation
+when it is about to extract a file which already exists on disk.
+
 @table @option
 @opindex keep-old-files
 @item --keep-old-files
 @itemx -k
-Do not replace existing files from archive.  The
address@hidden (@option{-k}) option prevents @command{tar}
-from replacing existing files with files with the same name from the
-archive. The @option{--keep-old-files} option is meaningless with
address@hidden (@option{-t}).  Prevents @command{tar} from replacing
-files in the file system during extraction.
+Do not replace existing files from archive.  When such a file is
+encountered, @command{tar} issues an error message.  Upon end of
+extraction, @command{tar} exits with code 2 (@pxref{exit status}).
+
address@hidden --skip-old-files
+Do not replace existing files from archive, but do not treat that
+as error.  Such files are silently skipped and do not affect
address@hidden exit status.
+
+Additional verbosity can be obtained using @option{--warning=existing-file}
+together with that option (@pxref{warnings}).
 @end table

 @node Keep Newer Files
@@ -11937,11 +11978,16 @@ lets the archive overwrite any file in your system 
that you can write,
 the @option{--absolute-names} (@option{-P}) option should be used only
 for trusted archives.

-Conversely, with the @option{--keep-old-files} (@option{-k}) option,
address@hidden refuses to replace existing files when extracting; and
-with the @option{--no-overwrite-dir} option, @command{tar} refuses to
-replace the permissions or ownership of already-existing directories.
-These options may help when extracting from untrusted archives.
+Conversely, with the @option{--keep-old-files} (@option{-k}) and
address@hidden options, @command{tar} refuses to replace
+existing files when extracting.  The difference between the two
+options is that the former treats existing files as errors whereas the
+latter just silently ignores them.
+
+Finally, with the @option{--no-overwrite-dir} option, @command{tar}
+refuses to replace the permissions or ownership of already-existing
+directories.  These options may help when extracting from untrusted
+archives.

 @node Live untrusted data
 @subsection Dealing with Live Untrusted Data
diff --git a/src/common.h b/src/common.h
index b60c4a0..1429b22 100644
--- a/src/common.h
+++ b/src/common.h
@@ -183,6 +183,7 @@ enum old_files
   OVERWRITE_OLD_FILES,        /* --overwrite */
   UNLINK_FIRST_OLD_FILES,     /* --unlink-first */
   KEEP_OLD_FILES,             /* --keep-old-files */
+  SKIP_OLD_FILES,             /* --skip-old-files */
   KEEP_NEWER_FILES           /* --keep-newer-files */
 };
 GLOBAL enum old_files old_files_option;
diff --git a/src/extract.c b/src/extract.c
index 60ec747..6c38492 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -642,11 +642,14 @@ maybe_recoverable (char *file_name, bool regular, bool 
*interdir_made)

       switch (old_files_option)
        {
-       case KEEP_OLD_FILES:
+       case SKIP_OLD_FILES:
          WARNOPT (WARN_EXISTING_FILE,
                   (0, 0, _("%s: skipping existing file"), file_name));
          return RECOVER_SKIP;

+       case KEEP_OLD_FILES:
+         return RECOVER_NO;
+
        case KEEP_NEWER_FILES:
          if (file_newer_p (file_name, stp, &current_stat_info))
            break;
diff --git a/src/tar.c b/src/tar.c
index f0d8f5b..21d9910 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -328,6 +328,7 @@ enum
   SHOW_DEFAULTS_OPTION,
   SHOW_OMITTED_DIRS_OPTION,
   SHOW_TRANSFORMED_NAMES_OPTION,
+  SKIP_OLD_FILES_OPTION,
   SPARSE_VERSION_OPTION,
   STRIP_COMPONENTS_OPTION,
   SUFFIX_OPTION,
@@ -452,7 +453,11 @@ static struct argp_option options[] = {
   {"remove-files", REMOVE_FILES_OPTION, 0, 0,
    N_("remove files after adding them to the archive"), GRID+1 },
   {"keep-old-files", 'k', 0, 0,
-   N_("don't replace existing files when extracting"), GRID+1 },
+   N_("don't replace existing files when extracting, "
+      "treat them as errors"), GRID+1 },
+  {"skip-old-files", SKIP_OLD_FILES_OPTION, 0, 0,
+   N_("don't replace existing files when extracting, silently skip over them"),
+   GRID+1 },
   {"keep-newer-files", KEEP_NEWER_FILES_OPTION, 0, 0,
    N_("don't replace existing files that are newer than their archive 
copies"), GRID+1 },
   {"overwrite", OVERWRITE_OPTION, 0, 0,
@@ -1544,7 +1549,7 @@ parse_opt (int key, char *arg, struct argp_state *state)
       /* Don't replace existing files.  */
       old_files_option = KEEP_OLD_FILES;
       break;
-
+
     case 'K':
       starting_file_option = true;
       addname (arg, 0, true, NULL);
@@ -1674,6 +1679,10 @@ parse_opt (int key, char *arg, struct argp_state *state)
       sparse_option = true;
       break;

+    case SKIP_OLD_FILES_OPTION:
+      old_files_option = SKIP_OLD_FILES;
+      break;
+
     case SPARSE_VERSION_OPTION:
       sparse_option = true;
       {
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7acc9d6..4601e0e 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -86,6 +86,8 @@ TESTSUITE_AT = \
  extrac15.at\
  extrac16.at\
  extrac17.at\
+ extrac18.at\
+ extrac19.at\
  filerem01.at\
  filerem02.at\
  gzip.at\
diff --git a/tests/extrac18.at b/tests/extrac18.at
new file mode 100644
index 0000000..8b42ef7
--- /dev/null
+++ b/tests/extrac18.at
@@ -0,0 +1,60 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright (C) 2011 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, see <http://www.gnu.org/licenses/>.
+
+# Description: Check the functionality of the --keep-old-files option.
+# It should report an error and cause tar to exit with status 2.
+#
+# There was a regression in versions 1.23 to 1.26 inclusive, where
+# this option silently skipped such files.
+# Reported by: Doug McLaren <address@hidden>,
+#              Gary Partis <address@hidden>,
+#              Jim Meyering <address@hidden>
+#
+# References: <address@hidden>,
+#             <address@hidden>,
+#             <address@hidden>
+
+AT_SETUP([keep-old-files])
+AT_KEYWORDS([extract extrac18 old-files keep-old-files])
+
+AT_TAR_CHECK([
+mkdir dir
+cd dir
+echo 'Old file a' > a
+echo 'Old file b' > b
+
+tar cf ../archive .
+
+rm b
+echo 'File a' > a
+
+tar -x -k -f ../archive
+echo status=$?
+
+cat a
+],
+[0],
+[status=2
+File a
+],
+[tar: ./a: Cannot open: File exists
+tar: Exiting with failure status due to previous errors
+])
+
+AT_CLEANUP
+
diff --git a/tests/extrac19.at b/tests/extrac19.at
new file mode 100644
index 0000000..43c4c50
--- /dev/null
+++ b/tests/extrac19.at
@@ -0,0 +1,44 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright (C) 2011 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, see <http://www.gnu.org/licenses/>.
+
+AT_SETUP([skip-old-files])
+AT_KEYWORDS([extract extrac19 old-files skip-old-files])
+
+AT_TAR_CHECK([
+mkdir dir
+cd dir
+echo 'Old file a' > a
+echo 'Old file b' > b
+
+tar cf ../archive .
+
+rm b
+echo 'File a' > a
+
+tar -x --skip-old-files -f ../archive
+echo status=$?
+
+cat a
+],
+[0],
+[status=0
+File a
+])
+
+AT_CLEANUP
+
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 35f8c2f..97f5e41 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -166,6 +166,8 @@ m4_include([extrac14.at])
 m4_include([extrac15.at])
 m4_include([extrac16.at])
 m4_include([extrac17.at])
+m4_include([extrac18.at])
+m4_include([extrac19.at])

 m4_include([label01.at])
 m4_include([label02.at])
--
1.7.3.2