bug-tar
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bug-tar] Need response from Sergey on tarbomb patch


From: Sergey Poznyakoff
Subject: Re: [Bug-tar] Need response from Sergey on tarbomb patch
Date: Tue, 28 Jan 2014 12:47:24 +0200

Hi Connor,

In general, the patch looks quite well, apart from the following:

1. maybe_prepend_name is written rather ineffectively and is used
too late.  This means, in particular, that --show-transformed-names
won't show the actual names of the extracted files.

2. Silently disabling one-top-level mode if unable to deduce the
archive base name seems wrong, given the mode purpose.  This case
is quite common if reading the archive from standard input, for example.
I believe the right thing to do in such case is terminate immediately.
It would be also nice if the --one-top-level option took an optional
argument specifying the desired top level directory. This would allow
to do e.g.

    cat archive | tar -xf - --one-top-level=dir

3. File suffix recognition in extr_init partly duplicates the
stuff in suffix.c.

I've addressed all this in the patch attached to this posting, so you
should not bother with these.

Now, to accept the patch of that size we would need a copyright
assignment to the Free Software Foundation.  Would you be willing
to sign it?  For your information, I'm sending a detailed
description of what it means to your email.

Regards,
Sergey
>From 1689ed43885c8418c29d1365ae3a06450a460830 Mon Sep 17 00:00:00 2001
From: Sergey Poznyakoff <address@hidden>
Date: Tue, 28 Jan 2014 11:04:20 +0200
Subject: [PATCH] Improve one-top-level functionality

Make sure the changes become visible with --show-transformed-names.

* src/common.h (strip_compression_suffix): New function.
(one_top_level): Rename to one_top_level_dir. All uses changed.
* src/extract.c (extr_init): Use strip_compression_suffix.
Bail out if unable to determine top-level directory.
(maybe_prepend_name): Remove. All uses removed.
* src/tar.c (options): --one-top-level takes optional argument.
(parse_opt): Handle it.
* src/list.c (enforce_one_top_level): New function.
(transform_stat_info): Call enforce_one_top_level if required.
* src/suffix.c (compression_suffixes): List "tar" (no compression);
terminate with NULL entry.
(find_compression_suffix): New static.
(strip_compression_suffix): New function.

* doc/tar.1: Update.
* doc/tar.texi: Update.

* tests/onetop01.at: New testcase.
* tests/onetop02.at: New testcase.
* tests/onetop03.at: New testcase.
* tests/Makefile.am: Add new testcases.
* tests/testsuite.at: Likewise.
---
 doc/tar.1          |  7 ++++++-
 doc/tar.texi       | 18 ++++++++++--------
 src/common.h       |  3 ++-
 src/extract.c      | 56 +++++-------------------------------------------------
 src/list.c         | 28 +++++++++++++++++++++++++++
 src/suffix.c       | 53 ++++++++++++++++++++++++++++++++++++++++-----------
 src/tar.c          |  3 ++-
 tests/Makefile.am  |  3 +++
 tests/onetop01.at  | 42 ++++++++++++++++++++++++++++++++++++++++
 tests/onetop02.at  | 45 +++++++++++++++++++++++++++++++++++++++++++
 tests/onetop03.at  | 42 ++++++++++++++++++++++++++++++++++++++++
 tests/testsuite.at |  5 +++++
 12 files changed, 232 insertions(+), 73 deletions(-)
 create mode 100644 tests/onetop01.at
 create mode 100644 tests/onetop02.at
 create mode 100644 tests/onetop03.at

diff --git a/doc/tar.1 b/doc/tar.1
index ec25aa9..bef0e70 100644
--- a/doc/tar.1
+++ b/doc/tar.1
@@ -13,7 +13,7 @@
 .\"
 .\" You should have received a copy of the GNU General Public License
 .\" along with this program.  If not, see <http://www.gnu.org/licenses/>.
-.TH TAR 1 "January 27, 2014" "TAR" "GNU TAR Manual"
+.TH TAR 1 "January 28, 2014" "TAR" "GNU TAR Manual"
 .SH NAME
 tar \- an archiving utility
 .SH SYNOPSIS
@@ -330,6 +330,11 @@ Don't replace existing files that are newer than their 
archive copies.
 \fB\-\-no\-overwrite\-dir\fR
 Preserve metadata of existing directories.
 .TP
+\fB\-\-one\-top\-level\fR[\fB=\fIDIR\fR]
+Extract all files into \fIDIR\fR, or, if used without argument, into a
+subdirectory named by the base name of the archive (minus standard
+compression suffixes recognizable by \fB\-\-auto\-compress).
+.TP
 \fB\-\-overwrite\fR
 Overwrite existing files when extracting.
 .TP
diff --git a/doc/tar.texi b/doc/tar.texi
index ece40f4..48c22a3 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -3087,15 +3087,17 @@ directories that are on different file systems from the 
current
 directory.

 @opsummary{one-top-level}
address@hidden --one-top-level
address@hidden address@hidden
 Tells @command{tar} to create a new directory beneath the extraction directory
-(or the one passed to @option{-C}) and use it to guard against tarbombs.  The
-name of the new directory will be equal to the name of the archive with the
-extension stripped off.  If any archive names (after transformations from
address@hidden and @option{--strip-components}) do not already begin with
-it, the new directory will be prepended to the names immediately before
-extraction.  Recognized extensions are @samp{.tar}, @samp{.taz}, @samp{.tbz},
address@hidden, @samp{.tgz}, @samp{.tlz} and @samp{.txz}.
+(or the one passed to @option{-C}) and use it to guard against
+tarbombs.  In the absence of @var{dir} argument, the name of the new directory
+will be equal to the base name of the archive (file name minus the
+archive suffix, if recognized).  Any member names that do not begin
+with that directory name (after
+transformations from @option{--transform} and
address@hidden) will be prefixed with it.  Recognized
+file name suffixes are @samp{.tar}, and any compression suffixes
+recognizable by @xref{--auto-compress}.

 @opsummary{overwrite}
 @item --overwrite
diff --git a/src/common.h b/src/common.h
index 365379a..04466ae 100644
--- a/src/common.h
+++ b/src/common.h
@@ -237,7 +237,7 @@ GLOBAL bool one_file_system_option;

 /* Create a top-level directory for extracting based on the archive name.  */
 GLOBAL bool one_top_level_option;
-GLOBAL char *one_top_level;
+GLOBAL char *one_top_level_dir;

 /* Specified value to be put into tar file in place of stat () results, or
    just null and -1 if such an override should not take place.  */
@@ -860,6 +860,7 @@ bool transform_program_p (void);

 /* Module suffix.c */
 void set_compression_program_by_suffix (const char *name, const char *defprog);
+char *strip_compression_suffix (const char *name);

 /* Module checkpoint.c */
 void checkpoint_compile_action (const char *str);
diff --git a/src/extract.c b/src/extract.c
index b6fdbbb..859146c 100644
--- a/src/extract.c
+++ b/src/extract.c
@@ -194,31 +194,15 @@ extr_init (void)

   /* If the user wants to guarantee that everything is under one directory,
      determine its name now and let it be created later.  */
-  if (one_top_level_option)
+  if (one_top_level_option && !one_top_level_dir)
     {
-      int i;
       char *base = base_name (archive_name_array[0]);

-      for (i = strlen (base) - 1; i > 2; i--)
-        if (!strncmp (base + i - 3, ".tar", 4) ||
-           !strncmp (base + i - 3, ".taz", 4) ||
-           !strncmp (base + i - 3, ".tbz", 4) ||
-           !strncmp (base + i - 3, ".tb2", 4) ||
-           !strncmp (base + i - 3, ".tgz", 4) ||
-           !strncmp (base + i - 3, ".tlz", 4) ||
-           !strncmp (base + i - 3, ".txz", 4)) break;
-
-      if (i <= 3)
-        {
-         one_top_level_option = false;
-         free (base);
-         return;
-       }
-
-      one_top_level = xmalloc (i - 2);
-      strncpy (one_top_level, base, i - 3);
-      one_top_level[i - 3] = '\0';
+      one_top_level_dir = strip_compression_suffix (base);
       free (base);
+
+      if (!one_top_level_dir)
+       USAGE_ERROR ((0, 0, _("Cannot deduce top-level directory name; please 
set it explicitly with --one-top-level=DIR")));
     }
 }

@@ -1607,33 +1591,6 @@ prepare_to_extract (char const *file_name, int typeflag, 
tar_extractor_t *fun)
   return 1;
 }

-void
-maybe_prepend_name (char **file_name)
-{
-  int i;
-
-  for (i = 0; i < strlen (*file_name); i++)
-    if (!ISSLASH ((*file_name)[i]) && (*file_name)[i] != '.') break;
-
-  if (i == strlen (*file_name))
-    return;
-
-  if (!strncmp (*file_name + i, one_top_level, strlen (one_top_level)))
-    {
-      int pos = i + strlen (one_top_level);
-      if (ISSLASH ((*file_name)[pos]) || (*file_name)[pos] == '\0') return;
-    }
-
-  char *new_name = xmalloc (strlen (one_top_level) + strlen (*file_name) + 2);
-
-  strcpy (new_name, one_top_level);
-  strcat (new_name, "/");
-  strcat (new_name, *file_name);
-
-  free (*file_name);
-  *file_name = new_name;
-}
-
 /* Extract a file from the archive.  */
 void
 extract_archive (void)
@@ -1684,9 +1641,6 @@ extract_archive (void)
   typeflag = sparse_member_p (&current_stat_info) ?
                   GNUTYPE_SPARSE : current_header->header.typeflag;

-  if (one_top_level_option)
-    maybe_prepend_name (&current_stat_info.file_name);
-
   if (prepare_to_extract (current_stat_info.file_name, typeflag, &fun))
     {
       if (fun && (*fun) (current_stat_info.file_name, typeflag)
diff --git a/src/list.c b/src/list.c
index 3a59f29..3655260 100644
--- a/src/list.c
+++ b/src/list.c
@@ -115,6 +115,30 @@ transform_member_name (char **pinput, int type)
   return transform_name_fp (pinput, type, decode_xform, &type);
 }

+static void
+enforce_one_top_level (char **pfile_name)
+{
+  char *file_name = *pfile_name;
+  char *p;
+
+  for (p = file_name; *p && (ISSLASH (*p) || *p == '.'); p++)
+    ;
+
+  if (!*p)
+    return;
+
+  if (strncmp (p, one_top_level_dir, strlen (one_top_level_dir)) == 0)
+    {
+      int pos = strlen (one_top_level_dir);
+      if (ISSLASH (p[pos]) || p[pos] == 0)
+       return;
+    }
+
+  *pfile_name = new_name (one_top_level_dir, file_name);
+  normalize_filename_x (*pfile_name);
+  free (file_name);
+}
+
 void
 transform_stat_info (int typeflag, struct tar_stat_info *stat_info)
 {
@@ -132,6 +156,9 @@ transform_stat_info (int typeflag, struct tar_stat_info 
*stat_info)
     case LNKTYPE:
       transform_member_name (&stat_info->link_name, XFORM_LINK);
     }
+
+  if (one_top_level_option)
+    enforce_one_top_level (&current_stat_info.file_name);
 }

 /* Main loop for reading an archive.  */
@@ -194,6 +221,7 @@ read_and (void (*do_something) (void))
                  continue;
                }
            }
+
          transform_stat_info (current_header->header.typeflag,
                               &current_stat_info);
          (*do_something) ();
diff --git a/src/suffix.c b/src/suffix.c
index cf8056c..f623451 100644
--- a/src/suffix.c
+++ b/src/suffix.c
@@ -29,6 +29,7 @@ struct compression_suffix
 static struct compression_suffix compression_suffixes[] = {
 #define __CAT2__(a,b) a ## b
 #define S(s,p) #s, sizeof (#s) - 1, __CAT2__(p,_PROGRAM)
+  { "tar", 3, NULL },
   { S(gz,   GZIP) },
   { S(tgz,  GZIP) },
   { S(taz,  GZIP) },
@@ -44,33 +45,43 @@ static struct compression_suffix compression_suffixes[] = {
   { S(lzo,  LZOP) },
   { S(xz,   XZ) },
   { S(txz,  XZ) }, /* Slackware */
+  { NULL }
 #undef S
 #undef __CAT2__
 };

-static int nsuffixes = sizeof (compression_suffixes) /
-                        sizeof (compression_suffixes[0]);
-
-static const char *
-find_compression_program (const char *name, const char *defprog)
+static struct compression_suffix const *
+find_compression_suffix (const char *name, size_t *base_len)
 {
   char *suf = strrchr (name, '.');

   if (suf)
     {
-      int i;
       size_t len;
-
+      struct compression_suffix *p;
+
       suf++;
       len = strlen (suf);

-      for (i = 0; i < nsuffixes; i++)
+      for (p = compression_suffixes; p->suffix; p++)
        {
-         if (compression_suffixes[i].length == len
-             && memcmp (compression_suffixes[i].suffix, suf, len) == 0)
-           return compression_suffixes[i].program;
+         if (p->length == len && memcmp (p->suffix, suf, len) == 0)
+           {
+             if (*base_len)
+               *base_len = strlen (name) - len - 1;
+             return p;
+           }
        }
     }
+  return NULL;
+}
+
+static const char *
+find_compression_program (const char *name, const char *defprog)
+{
+  struct compression_suffix const *p = find_compression_suffix (name, NULL);
+  if (p)
+    return p->program;
   return defprog;
 }

@@ -81,3 +92,23 @@ set_compression_program_by_suffix (const char *name, const 
char *defprog)
   if (program)
     use_compress_program_option = program;
 }
+
+char *
+strip_compression_suffix (const char *name)
+{
+  char *s = NULL;
+  size_t len;
+
+  if (find_compression_suffix (name, &len))
+    {
+      if (strncmp (name + len - 4, ".tar", 4) == 0)
+       len -= 4;
+      if (len == 0)
+       return NULL;
+      s = xmalloc (len + 1);
+      memcpy (s, name, len);
+      s[len] = 0;
+    }
+  return s;
+}
+
diff --git a/src/tar.c b/src/tar.c
index 0dfa9c1..a034b34 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -490,7 +490,7 @@ static struct argp_option options[] = {
   {"keep-directory-symlink", KEEP_DIRECTORY_SYMLINK_OPTION, 0, 0,
    N_("preserve existing symlinks to directories when extracting"),
    GRID+1 },
-  {"one-top-level", ONE_TOP_LEVEL_OPTION, 0, 0,
+  {"one-top-level", ONE_TOP_LEVEL_OPTION, N_("DIR"), OPTION_ARG_OPTIONAL,
    N_("create a subdirectory to avoid having loose files extracted"),
    GRID+1 },
 #undef GRID
@@ -1447,6 +1447,7 @@ parse_opt (int key, char *arg, struct argp_state *state)

     case ONE_TOP_LEVEL_OPTION:
       one_top_level_option = true;
+      one_top_level_dir = arg;
       break;

     case 'l':
diff --git a/tests/Makefile.am b/tests/Makefile.am
index fc72c51..1f17a23 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -144,6 +144,9 @@ TESTSUITE_AT = \
  multiv07.at\
  multiv08.at\
  old.at\
+ onetop01.at\
+ onetop02.at\
+ onetop03.at\
  opcomp01.at\
  opcomp02.at\
  opcomp03.at\
diff --git a/tests/onetop01.at b/tests/onetop01.at
new file mode 100644
index 0000000..a970a99
--- /dev/null
+++ b/tests/onetop01.at
@@ -0,0 +1,42 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2014 Free Software Foundation, Inc.
+#
+# This file is part of GNU tar.
+#
+# GNU tar 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 of the License, or
+# (at your option) any later version.
+#
+# GNU tar 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([tar --one-top-level])
+AT_KEYWORDS([extract onetop onetop01])
+
+AT_TAR_CHECK([
+AT_SORT_PREREQ
+mkdir a
+genfile --file a/b
+genfile --file c
+tar cf a.tar a c
+mkdir out
+cd out
+tar --one-top-level -x -f ../a.tar
+find . | sort
+],
+[0],
+[.
+./a
+./a/b
+./a/c
+])
+
+AT_CLEANUP
diff --git a/tests/onetop02.at b/tests/onetop02.at
new file mode 100644
index 0000000..454f692
--- /dev/null
+++ b/tests/onetop02.at
@@ -0,0 +1,45 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2014 Free Software Foundation, Inc.
+#
+# This file is part of GNU tar.
+#
+# GNU tar 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 of the License, or
+# (at your option) any later version.
+#
+# GNU tar 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([tar --one-top-level --show-transformed])
+AT_KEYWORDS([extract onetop onetop02])
+
+AT_TAR_CHECK([
+AT_SORT_PREREQ
+mkdir a
+genfile --file a/b
+genfile --file c
+tar cf a.tar a c
+mkdir out
+cd out
+tar --one-top-level --show-transformed  -v -x -f ../a.tar
+find . | sort
+],
+[0],
+[a/
+a/b
+a/c
+.
+./a
+./a/b
+./a/c
+])
+
+AT_CLEANUP
diff --git a/tests/onetop03.at b/tests/onetop03.at
new file mode 100644
index 0000000..3ffc71d
--- /dev/null
+++ b/tests/onetop03.at
@@ -0,0 +1,42 @@
+# Process this file with autom4te to create testsuite. -*- Autotest -*-
+#
+# Test suite for GNU tar.
+# Copyright 2014 Free Software Foundation, Inc.
+#
+# This file is part of GNU tar.
+#
+# GNU tar 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 of the License, or
+# (at your option) any later version.
+#
+# GNU tar 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([tar --one-top-level --transform])
+AT_KEYWORDS([extract onetop onetop02])
+
+AT_TAR_CHECK([
+AT_SORT_PREREQ
+mkdir a
+genfile --file a/b
+genfile --file c
+tar cf a.tar a c
+mkdir out
+cd out
+tar --one-top-level --transform 's/c/d/' -x -f ../a.tar
+find . | sort
+],
+[0],
+[.
+./a
+./a/b
+./a/d
+])
+
+AT_CLEANUP
diff --git a/tests/testsuite.at b/tests/testsuite.at
index 1aab6f7..100062d 100644
--- a/tests/testsuite.at
+++ b/tests/testsuite.at
@@ -413,6 +413,11 @@ m4_include([selacl01.at])

 m4_include([capabs_raw01.at])

+AT_BANNER([One top level])
+m4_include([onetop01.at])
+m4_include([onetop02.at])
+m4_include([onetop03.at])
+
 AT_BANNER([Star tests])
 m4_include([star/gtarfail.at])
 m4_include([star/gtarfail2.at])
--
1.7.12.1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]