Re: [Bug-tar] [PATCH] Bugfix - fix xattr exclude/include for archive create

[Pavel Raiskup]

Thanks, I would just update the testcase to be more general, but otherwise
it looks OK to me.  Sergey, can you have a look please?

On Monday 30 of May 2016 17:11:35 Ian McLeod wrote:
> * src/xattrs.c (xattrs_xattrs_get): apply exclude/include mask when
>   fetching extended attributes
> * tests/Makefile.am: Add new test case
> * tests/testsuite.at: ditto
> 
> This makes archive create behavior consistent with the documentation.
> Without this change xattr include/exclude options are accepted when
> creating an archive but are silently ignored.

This is nit, but we usually put description above the particular changelog
entries.

> ---
>  src/xattrs.c       | 15 ++++++++++-----
>  tests/Makefile.am  |  1 +
>  tests/testsuite.at |  1 +
>  tests/xattr06.at   | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 63 insertions(+), 5 deletions(-)
>  create mode 100644 tests/xattr06.at
> 
> diff --git a/src/xattrs.c b/src/xattrs.c
> index 8e56168..655dd43 100644
> --- a/src/xattrs.c
> +++ b/src/xattrs.c
> @@ -434,8 +434,12 @@ xattrs_clear_setup (void)
>    clear_mask_map (&xattrs_setup.excl);
>  }
>  
> -/* get all xattrs from file given by FILE_NAME or FD (when non-zero).  This
> -   includes all the user.*, security.*, system.*, etc. available domains */
> +static bool xattrs_masked_out (const char *kw, bool archiving);
> +
> +/* get xattrs from file given by FILE_NAME or FD (when non-zero)
> +   xattrs are checked against the user supplied include/exclude mask
> +   if no mask is given this includes all the user.*, security.*, system.*,
> +   etc. available domains */
>  void
>  xattrs_xattrs_get (int parentfd, char const *file_name,
>                     struct tar_stat_info *st, int fd)
> @@ -480,8 +484,6 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
>                size_t len = strlen (attr);
>                ssize_t aret = 0;
>  
> -              /* Archive all xattrs during creation, decide at extraction 
> time
> -               * which ones are of interest/use for the target filesystem. */
>                while (((fd == 0)
>                        ? ((aret = lgetxattrat (parentfd, file_name, attr,
>                                                val, asz)) == -1)
> @@ -492,7 +494,10 @@ xattrs_xattrs_get (int parentfd, char const *file_name,
>                  }
>  
>                if (aret != -1)
> -                xheader_xattr_add (st, attr, val, aret);
> +                {
> +                  if (!xattrs_masked_out(attr, true))

s/xattrs_masked_out(/xattrs_masked_out (/

> +                    xheader_xattr_add (st, attr, val, aret);
> +                }
>                else if (errno != ENOATTR)
>                  call_arg_warn ((fd == 0) ? "lgetxattrat"
>                                 : "fgetxattr", file_name);
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index 0ea6d17..5c13c8f 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -244,6 +244,7 @@ TESTSUITE_AT = \
>   xattr03.at\
>   xattr04.at\
>   xattr05.at\
> + xattr06.at\
>   acls01.at\
>   acls02.at\
>   acls03.at\
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index 11c39c9..126725c 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -443,6 +443,7 @@ m4_include([xattr02.at])
>  m4_include([xattr03.at])
>  m4_include([xattr04.at])
>  m4_include([xattr05.at])
> +m4_include([xattr06.at])
>  
>  m4_include([acls01.at])
>  m4_include([acls02.at])
> diff --git a/tests/xattr06.at b/tests/xattr06.at
> new file mode 100644
> index 0000000..42204af
> --- /dev/null
> +++ b/tests/xattr06.at
> @@ -0,0 +1,51 @@
> +# Process this file with autom4te to create testsuite. -*- Autotest -*-
> +#
> +# Test suite for GNU tar.
> +# Copyright 2012-2014, 2016 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/>.
> +#
> +# Test description:  Test for exclude of xattr during archive creation
> +#
> +# Relevant mailing list thread:
> +#
> +# http://lists.gnu.org/archive/html/bug-tar/2016-05/msg00031.html
+
> +AT_SETUP([xattrs: exclude xattrs on create ])
> +AT_KEYWORDS([xattrs xattr06])
> +
> +AT_TAR_CHECK([
> +AT_XATTRS_PREREQ
> +
> +mkdir dir
> +mkdir output
> +genfile --file dir/file
> +
> +setfattr -n user.excludedxattr -v value dir/file
> +
> +# exclude the attribute we just set
> +tar --xattrs --xattrs-exclude=user.excludedxattr -cf archive.tar -C dir .
> +
> +tar --xattrs -xf archive.tar -C output

Could we use something like this? (to test include/exclude during
archiving/extraction):

for attr in excl incla inclb inclc incl_excluded
do
  setfattr -n user.${attr} -v value dir/file
done

tar --xattrs-include=incl'*' --xattrs-exclude=incl_excluded -cf archive.tar -C 
dir .

tar -xf archive.tar '--xattrs-include=incl[ab]' --xattrs-exclude=inclb

> +getfattr -d output/file | grep -v excludedxattr > without
> +getfattr -d output/file > with
> +# if they differ then the attribute is still present
> +diff without with
> +],
> +[0],
> +[])
> +
> +AT_CLEANUP
>