xattr: use bytes typed names for listxattr, getxattr, setxattr

src/borg/archive.py

@@ -726,28 +726,27 @@ Utilization of max. archive size: {csize_max:.0%}
         xattrs = item.get('xattrs', {})
         for k, v in xattrs.items():
             try:
+                # the key k is a bytes object due to msgpack unpacking it as such.
                 # if we have a None value, it means "empty", so give b'' to setxattr in that case:
                 xattr.setxattr(fd or path, k, v or b'', follow_symlinks=False)
             except OSError as e:
+                k_str = k.decode()
                 if e.errno == errno.E2BIG:
-                    # xattr is too big
                     logger.warning('%s: Value or key of extended attribute %s is too big for this filesystem' %
-                                   (path, k.decode()))
+                                   (path, k_str))
                     set_ec(EXIT_WARNING)
                 elif e.errno == errno.ENOTSUP:
-                    # xattrs not supported here
                     logger.warning('%s: Extended attributes are not supported on this filesystem' % path)
                     set_ec(EXIT_WARNING)
                 elif e.errno == errno.EACCES:
                     # permission denied to set this specific xattr (this may happen related to security.* keys)
-                    logger.warning('%s: Permission denied when setting extended attribute %s' % (path, k.decode()))
+                    logger.warning('%s: Permission denied when setting extended attribute %s' % (path, k_str))
                     set_ec(EXIT_WARNING)
                 elif e.errno == errno.ENOSPC:
-                    # no space left on device while setting this specific xattr
                     # ext4 reports ENOSPC when trying to set an xattr with >4kiB while ext4 can only support 4kiB xattrs
                     # (in this case, this is NOT a "disk full" error, just a ext4 limitation).
                     logger.warning('%s: No space left on device while setting extended attribute %s (len = %d)' % (
-                        path, k.decode(), len(v)))
+                        path, k_str, len(v)))
                     set_ec(EXIT_WARNING)
                 else:
                     raise

src/borg/platform/base.py

@@ -26,7 +26,7 @@ from .xattr import ENOATTR
 
 def listxattr(path, *, follow_symlinks=True):
     """
-    Return list of xattr names on a file.
+    Return xattr names of a file (list of bytes objects).
 
     *path* can either be a path (str or bytes) or an open file descriptor (int).
     *follow_symlinks* indicates whether symlinks should be followed
@@ -40,7 +40,7 @@ def getxattr(path, name, *, follow_symlinks=True):
     Read xattr and return its value (as bytes).
 
     *path* can either be a path (str or bytes) or an open file descriptor (int).
-    *name* is the name of the xattr to read (str).
+    *name* is the name of the xattr to read (bytes).
     *follow_symlinks* indicates whether symlinks should be followed
     and only applies when *path* is not an open file descriptor.
     """
@@ -52,7 +52,7 @@ def setxattr(path, name, value, *, follow_symlinks=True):
     Write xattr on *path*.
 
     *path* can either be a path (str or bytes) or an open file descriptor (int).
-    *name* is the name of the xattr to read (str).
+    *name* is the name of the xattr to read (bytes).
     *value* is the value to write (bytes).
     *follow_symlinks* indicates whether symlinks should be followed
     and only applies when *path* is not an open file descriptor.

src/borg/platform/darwin.pyx

@@ -46,7 +46,7 @@ def listxattr(path, *, follow_symlinks=True):
                 return c_listxattr(path, <char *> buf, size, XATTR_NOFOLLOW)
 
     n, buf = _listxattr_inner(func, path)
-    return [os.fsdecode(name) for name in split_string0(buf[:n]) if name]
+    return [name for name in split_string0(buf[:n]) if name]
 
 
 def getxattr(path, name, *, follow_symlinks=True):

src/borg/platform/freebsd.pyx

@@ -59,7 +59,7 @@ def listxattr(path, *, follow_symlinks=True):
                 return c_extattr_list_link(path, EXTATTR_NAMESPACE_USER, <char *> buf, size)
 
     n, buf = _listxattr_inner(func, path)
-    return [os.fsdecode(name) for name in split_lstring(buf[:n]) if name]
+    return [name for name in split_lstring(buf[:n]) if name]
 
 
 def getxattr(path, name, *, follow_symlinks=True):

src/borg/platform/linux.pyx

@@ -87,7 +87,7 @@ def listxattr(path, *, follow_symlinks=True):
                 return c_llistxattr(path, <char *> buf, size)
 
     n, buf = _listxattr_inner(func, path)
-    return [os.fsdecode(name) for name in split_string0(buf[:n])
+    return [name for name in split_string0(buf[:n])
             if name and not name.startswith(b'system.posix_acl_')]
 
 

src/borg/platform/xattr.py

@@ -79,7 +79,7 @@ def _listxattr_inner(func, path):
 def _getxattr_inner(func, path, name):
     if isinstance(path, str):
         path = os.fsencode(path)
-    name = os.fsencode(name)
+    assert isinstance(name, bytes)
     size = len(buffer)
     while True:
         buf = buffer.get(size)
@@ -94,7 +94,6 @@ def _getxattr_inner(func, path, name):
 def _setxattr_inner(func, path, name, value):
     if isinstance(path, str):
         path = os.fsencode(path)
-    name = os.fsencode(name)
-    value = value and os.fsencode(value)
-    size = len(value) if value else 0
-    _check(func(path, name, value, size), path, detect_buffer_too_small=False)
+    assert isinstance(name, bytes)
+    assert isinstance(value, bytes)
+    _check(func(path, name, value, len(value)), path, detect_buffer_too_small=False)

src/borg/testsuite/__init__.py

@@ -140,7 +140,7 @@ def is_birthtime_fully_supported():
 
 def no_selinux(x):
     # selinux fails our FUSE tests, thus ignore selinux xattrs
-    SELINUX_KEY = 'security.selinux'
+    SELINUX_KEY = b'security.selinux'
     if isinstance(x, dict):
         return {k: v for k, v in x.items() if k != SELINUX_KEY}
     if isinstance(x, list):

src/borg/testsuite/archiver.py

@@ -340,14 +340,14 @@ class ArchiverTestCaseBase(BaseTestCase):
             # This is because fakeroot with xattr-support does not propagate xattrs of the underlying file
             # into "fakeroot space". Because the xattrs exposed by borgfs are these of an underlying file
             # (from fakeroots point of view) they are invisible to the test process inside the fakeroot.
-            xattr.setxattr(os.path.join(self.input_path, 'fusexattr'), 'user.foo', b'bar')
-            xattr.setxattr(os.path.join(self.input_path, 'fusexattr'), 'user.empty', b'')
+            xattr.setxattr(os.path.join(self.input_path, 'fusexattr'), b'user.foo', b'bar')
+            xattr.setxattr(os.path.join(self.input_path, 'fusexattr'), b'user.empty', b'')
             # XXX this always fails for me
             # ubuntu 14.04, on a TMP dir filesystem with user_xattr, using fakeroot
             # same for newer ubuntu and centos.
             # if this is supported just on specific platform, platform should be checked first,
             # so that the test setup for all tests using it does not fail here always for others.
-            # xattr.setxattr(os.path.join(self.input_path, 'link1'), 'user.foo_symlink', b'bar_symlink', follow_symlinks=False)
+            # xattr.setxattr(os.path.join(self.input_path, 'link1'), b'user.foo_symlink', b'bar_symlink', follow_symlinks=False)
         # FIFO node
         if are_fifos_supported():
             os.mkfifo(os.path.join(self.input_path, 'fifo1'))
@@ -1229,19 +1229,19 @@ class ArchiverTestCase(ArchiverTestCaseBase):
         # We need to manually patch chown to get the behaviour Linux has, since fakeroot does not
         # accurately model the interaction of chown(2) and Linux capabilities, i.e. it does not remove them.
         def patched_fchown(fd, uid, gid):
-            xattr.setxattr(fd, 'security.capability', b'', follow_symlinks=False)
+            xattr.setxattr(fd, b'security.capability', b'', follow_symlinks=False)
             fchown(fd, uid, gid)
 
         # The capability descriptor used here is valid and taken from a /usr/bin/ping
         capabilities = b'\x01\x00\x00\x02\x00 \x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
         self.create_regular_file('file')
-        xattr.setxattr('input/file', 'security.capability', capabilities)
+        xattr.setxattr('input/file', b'security.capability', capabilities)
         self.cmd('init', '--encryption=repokey', self.repository_location)
         self.cmd('create', self.repository_location + '::test', 'input')
         with changedir('output'):
             with patch.object(os, 'fchown', patched_fchown):
                 self.cmd('extract', self.repository_location + '::test')
-            assert xattr.getxattr('input/file', 'security.capability') == capabilities
+            assert xattr.getxattr('input/file', b'security.capability') == capabilities
 
     @pytest.mark.skipif(not xattr.XATTR_FAKEROOT, reason='xattr not supported on this system or on this version of'
                                                          'fakeroot')
@@ -1256,7 +1256,7 @@ class ArchiverTestCase(ArchiverTestCaseBase):
             raise OSError(errno.EACCES, 'EACCES')
 
         self.create_regular_file('file')
-        xattr.setxattr('input/file', 'attribute', 'value')
+        xattr.setxattr('input/file', b'attribute', b'value')
         self.cmd('init', self.repository_location, '-e' 'none')
         self.cmd('create', self.repository_location + '::test', 'input')
         with changedir('output'):
@@ -2185,13 +2185,13 @@ class ArchiverTestCase(ArchiverTestCaseBase):
                 in_fn = 'input/fusexattr'
                 out_fn = os.path.join(mountpoint, 'input', 'fusexattr')
                 if not xattr.XATTR_FAKEROOT and xattr.is_enabled(self.input_path):
-                    assert sorted(no_selinux(xattr.listxattr(out_fn))) == ['user.empty', 'user.foo', ]
-                    assert xattr.getxattr(out_fn, 'user.foo') == b'bar'
-                    assert xattr.getxattr(out_fn, 'user.empty') == b''
+                    assert sorted(no_selinux(xattr.listxattr(out_fn))) == [b'user.empty', b'user.foo', ]
+                    assert xattr.getxattr(out_fn, b'user.foo') == b'bar'
+                    assert xattr.getxattr(out_fn, b'user.empty') == b''
                 else:
                     assert xattr.listxattr(out_fn) == []
                     try:
-                        xattr.getxattr(out_fn, 'user.foo')
+                        xattr.getxattr(out_fn, b'user.foo')
                     except OSError as e:
                         assert e.errno == llfuse.ENOATTR
                     else:

src/borg/testsuite/xattr.py

@@ -22,7 +22,7 @@ class XattrTestCase(BaseTestCase):
 
     def assert_equal_se(self, is_x, want_x):
         # check 2 xattr lists for equality, but ignore security.selinux attr
-        is_x = set(is_x) - {'security.selinux'}
+        is_x = set(is_x) - {b'security.selinux'}
         want_x = set(want_x)
         self.assert_equal(is_x, want_x)
 
@@ -30,23 +30,23 @@ class XattrTestCase(BaseTestCase):
         self.assert_equal_se(listxattr(self.tmpfile.name), [])
         self.assert_equal_se(listxattr(self.tmpfile.fileno()), [])
         self.assert_equal_se(listxattr(self.symlink), [])
-        setxattr(self.tmpfile.name, 'user.foo', b'bar')
-        setxattr(self.tmpfile.fileno(), 'user.bar', b'foo')
-        setxattr(self.tmpfile.name, 'user.empty', b'')
-        self.assert_equal_se(listxattr(self.tmpfile.name), ['user.foo', 'user.bar', 'user.empty'])
-        self.assert_equal_se(listxattr(self.tmpfile.fileno()), ['user.foo', 'user.bar', 'user.empty'])
-        self.assert_equal_se(listxattr(self.symlink), ['user.foo', 'user.bar', 'user.empty'])
+        setxattr(self.tmpfile.name, b'user.foo', b'bar')
+        setxattr(self.tmpfile.fileno(), b'user.bar', b'foo')
+        setxattr(self.tmpfile.name, b'user.empty', b'')
+        self.assert_equal_se(listxattr(self.tmpfile.name), [b'user.foo', b'user.bar', b'user.empty'])
+        self.assert_equal_se(listxattr(self.tmpfile.fileno()), [b'user.foo', b'user.bar', b'user.empty'])
+        self.assert_equal_se(listxattr(self.symlink), [b'user.foo', b'user.bar', b'user.empty'])
         self.assert_equal_se(listxattr(self.symlink, follow_symlinks=False), [])
-        self.assert_equal(getxattr(self.tmpfile.name, 'user.foo'), b'bar')
-        self.assert_equal(getxattr(self.tmpfile.fileno(), 'user.foo'), b'bar')
-        self.assert_equal(getxattr(self.symlink, 'user.foo'), b'bar')
-        self.assert_equal(getxattr(self.tmpfile.name, 'user.empty'), b'')
+        self.assert_equal(getxattr(self.tmpfile.name, b'user.foo'), b'bar')
+        self.assert_equal(getxattr(self.tmpfile.fileno(), b'user.foo'), b'bar')
+        self.assert_equal(getxattr(self.symlink, b'user.foo'), b'bar')
+        self.assert_equal(getxattr(self.tmpfile.name, b'user.empty'), b'')
 
     def test_listxattr_buffer_growth(self):
         # make it work even with ext4, which imposes rather low limits
         buffer.resize(size=64, init=True)
         # xattr raw key list will be size 9 * (10 + 1), which is > 64
-        keys = ['user.attr%d' % i for i in range(9)]
+        keys = [b'user.attr%d' % i for i in range(9)]
         for key in keys:
             setxattr(self.tmpfile.name, key, b'x')
         got_keys = listxattr(self.tmpfile.name)
@@ -57,8 +57,8 @@ class XattrTestCase(BaseTestCase):
         # make it work even with ext4, which imposes rather low limits
         buffer.resize(size=64, init=True)
         value = b'x' * 126
-        setxattr(self.tmpfile.name, 'user.big', value)
-        got_value = getxattr(self.tmpfile.name, 'user.big')
+        setxattr(self.tmpfile.name, b'user.big', value)
+        got_value = getxattr(self.tmpfile.name, b'user.big')
         self.assert_equal(value, got_value)
         self.assert_equal(len(buffer), 128)
 

src/borg/xattr.py

@@ -13,10 +13,10 @@ def is_enabled(path=None):
     """
     with tempfile.NamedTemporaryFile(dir=path, prefix='borg-tmp') as fd:
         try:
-            setxattr(fd.fileno(), 'user.name', b'value')
+            setxattr(fd.fileno(), b'user.name', b'value')
         except OSError:
             return False
-        return getxattr(fd.fileno(), 'user.name') == b'value'
+        return getxattr(fd.fileno(), b'user.name') == b'value'
 
 
 def get_all(path, follow_symlinks=True):
@@ -27,7 +27,7 @@ def get_all(path, follow_symlinks=True):
     *follow_symlinks* indicates whether symlinks should be followed
     and only applies when *path* is not an open file descriptor.
 
-    The returned mapping maps xattr names (str) to values (bytes or None).
+    The returned mapping maps xattr names (bytes) to values (bytes or None).
     None indicates, as a xattr value, an empty value, i.e. a value of length zero.
     """
     try:
@@ -35,7 +35,8 @@ def get_all(path, follow_symlinks=True):
         names = listxattr(path, follow_symlinks=follow_symlinks)
         for name in names:
             try:
-                # if we get an empty xattr value (b''), we store None into the result dict.
+                # xattr name is a bytes object, we directly use it.
+                # if we get an empty xattr value (b''), we store None into the result dict -
                 # borg always did it like that...
                 result[name] = getxattr(path, name, follow_symlinks=follow_symlinks) or None
             except OSError as e: