Don't touch correct target xattrs; remove inappropriate target xattrs.

When applying xattr metadata, make sure to remove any target xattrs
that aren't in the metadata record, but don't touch target xattrs that
already match the metadata record.  Add corresponding tests.

Throw an ApplyError() if xattr set() or remove() return EPERM from
within _apply_linux_xattr_rec().

Remove lib/bup/t/testfs and lib/bup/t/testfs.img in the clean target.

diff --git a/Makefile b/Makefile
index cbf548c420bfaf624e6a06901828f0c3f5ccf0dd..2069099e91d7253f7f5c17bae5f87071f1fe0a64 100644 (file)
--- a/Makefile
+++ b/Makefile
@@ -145,6 +145,7 @@ clean: Documentation/clean
                *.pyc */*.pyc lib/*/*.pyc lib/*/*/*.pyc \
                bup bup-* cmd/bup-* lib/bup/_version.py randomgen memtest \
                out[12] out2[tc] tags[12] tags2[tc] \
-               testfs.img
+               testfs.img lib/bup/t/testfs.img
        rm -rf *.tmp t/*.tmp lib/*/*/*.tmp build lib/bup/build
        if test -e testfs; then rmdir testfs; fi
+       if test -e lib/bup/t/testfs; then rmdir lib/bup/t/testfs; fi

diff --git a/lib/bup/metadata.py b/lib/bup/metadata.py
index bb16892b6fa499fb46f41c906b79ee13e44adb88..a04d01f1e25dc39bd78108bb78ee1bc51a5f3f5b 100644 (file)
--- a/lib/bup/metadata.py
+++ b/lib/bup/metadata.py
@@ -465,9 +465,27 @@ class Metadata:
         self.linux_xattr = result
 
     def _apply_linux_xattr_rec(self, path, restore_numeric_ids=False):
+        existing_xattrs = set(xattr.list(path, nofollow=True))
         if(self.linux_xattr):
             for k, v in self.linux_xattr:
-                xattr.set(path, k, v, nofollow=True)
+                if k not in existing_xattrs \
+                        or v != xattr.get(path, k, nofollow=True):
+                    try:
+                        xattr.set(path, k, v, nofollow=True)
+                    except IOError, e:
+                        if e.errno == errno.EPERM:
+                            raise ApplyError('xattr.set: %s' % e)
+                        else:
+                            raise
+                existing_xattrs -= frozenset([k])
+            for k in existing_xattrs:
+                try:
+                    xattr.remove(path, k, nofollow=True)
+                except IOError, e:
+                    if e.errno == errno.EPERM:
+                        raise ApplyError('xattr.remove: %s' % e)
+                    else:
+                        raise
 
     def __init__(self):
         # optional members

diff --git a/lib/bup/t/tmetadata.py b/lib/bup/t/tmetadata.py
index 24e43f780005eb79ddf4bbce8c958bd6b4e086e7..be583dcb52ce4634b8d854962a8382bd11a34ada 100644 (file)
--- a/lib/bup/t/tmetadata.py
+++ b/lib/bup/t/tmetadata.py
@@ -3,12 +3,50 @@ import pwd
 import stat
 import subprocess
 import tempfile
+import xattr
 import bup.helpers as helpers
 from bup import metadata
 from bup.helpers import clear_errors, detect_fakeroot
 from wvtest import *
 
 
+top_dir = os.getcwd()
+
+
+def ex(*cmd):
+    try:
+        cmd_str = ' '.join(cmd)
+        print >> sys.stderr, cmd_str
+        rc = subprocess.call(cmd)
+        if rc < 0:
+            print >> sys.stderr, 'terminated by signal', - rc
+            sys.exit(1)
+        elif rc > 0:
+            print >> sys.stderr, 'returned exit status', rc
+            sys.exit(1)
+    except OSError, e:
+        print >> sys.stderr, 'subprocess call failed:', e
+        sys.exit(1)
+
+
+def setup_testfs():
+    # Set up testfs with user_xattr, etc.
+    subprocess.call(['umount', 'testfs'])
+    ex('dd', 'if=/dev/zero', 'of=testfs.img', 'bs=1M', 'count=32')
+    ex('mke2fs', '-F', '-j', '-m', '0', 'testfs.img')
+    ex('rm', '-rf', 'testfs')
+    os.mkdir('testfs')
+    ex('mount', '-o', 'loop,acl,user_xattr', 'testfs.img', 'testfs')
+    # Hide, so that tests can't create risks.
+    ex('chown', 'root:root', 'testfs')
+    os.chmod('testfs', 0700)
+
+
+def cleanup_testfs():
+    subprocess.call(['umount', 'testfs'])
+    subprocess.call(['rm', '-f', 'testfs.img'])
+
+
 @wvtest
 def test_clean_up_archive_path():
     cleanup = metadata._clean_up_path_for_archive
@@ -194,3 +232,29 @@ def test_restore_over_existing_target():
         WVEXCEPT(Exception, dir_m.create_path, path, create_symlinks=True)
     finally:
         subprocess.call(['rm', '-rf', tmpdir])
+
+
+@wvtest
+def test_handling_of_incorrect_existing_linux_xattrs():
+    if os.geteuid() != 0 or detect_fakeroot():
+        return
+    setup_testfs()
+    subprocess.check_call('rm -rf testfs/*', shell=True)
+    path = 'testfs/foo'
+    open(path, 'w').close()
+    xattr.set(path, 'foo', 'bar', namespace=xattr.NS_USER)
+    m = metadata.from_path(path, archive_path=path, save_symlinks=True)
+    xattr.set(path, 'baz', 'bax', namespace=xattr.NS_USER)
+    m.apply_to_path(path, restore_numeric_ids=False)
+    WVPASSEQ(xattr.list(path), ['user.foo'])
+    WVPASSEQ(xattr.get(path, 'user.foo'), 'bar')
+    xattr.set(path, 'foo', 'baz', namespace=xattr.NS_USER)
+    m.apply_to_path(path, restore_numeric_ids=False)
+    WVPASSEQ(xattr.list(path), ['user.foo'])
+    WVPASSEQ(xattr.get(path, 'user.foo'), 'bar')
+    xattr.remove(path, 'foo', namespace=xattr.NS_USER)
+    m.apply_to_path(path, restore_numeric_ids=False)
+    WVPASSEQ(xattr.list(path), ['user.foo'])
+    WVPASSEQ(xattr.get(path, 'user.foo'), 'bar')
+    os.chdir(top_dir)
+    cleanup_testfs()