xattr: use bytes typed values for listattr, getxattr, setxattr

- getxattr should only return bytes, not None
- setxattr should not get a None value, just bytes
- remove unneeded tmp vars

src/borg/archive.py

@@ -726,7 +726,8 @@ Utilization of max. archive size: {csize_max:.0%}
         xattrs = item.get('xattrs', {})
         for k, v in xattrs.items():
             try:
-                xattr.setxattr(fd or path, k, v, follow_symlinks=False)
+                # 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:
                 if e.errno == errno.E2BIG:
                     # xattr is too big

src/borg/platform/base.py

@@ -37,13 +37,14 @@ def listxattr(path, *, follow_symlinks=True):
 
 def getxattr(path, name, *, follow_symlinks=True):
     """
-    Read xattr and return its value (as bytes) or None if its empty.
+    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).
     *follow_symlinks* indicates whether symlinks should be followed
     and only applies when *path* is not an open file descriptor.
     """
+    return b''
 
 
 def setxattr(path, name, value, *, follow_symlinks=True):
@@ -52,8 +53,7 @@ def setxattr(path, name, value, *, follow_symlinks=True):
 
     *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).
-    *value* is the value to write. It is either bytes or None. The latter
-    signals that the value shall be empty (size equals zero).
+    *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

@@ -37,14 +37,13 @@ cdef extern from "sys/acl.h":
 
 def listxattr(path, *, follow_symlinks=True):
     def func(path, buf, size):
-        cdef char *buffer = <char *> buf
         if isinstance(path, int):
-            return c_flistxattr(path, buffer, size, XATTR_NOFLAGS)
+            return c_flistxattr(path, <char *> buf, size, XATTR_NOFLAGS)
         else:
             if follow_symlinks:
-                return c_listxattr(path, buffer, size, XATTR_NOFLAGS)
+                return c_listxattr(path, <char *> buf, size, XATTR_NOFLAGS)
             else:
-                return c_listxattr(path, buffer, size, XATTR_NOFOLLOW)
+                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]
@@ -52,29 +51,27 @@ def listxattr(path, *, follow_symlinks=True):
 
 def getxattr(path, name, *, follow_symlinks=True):
     def func(path, name, buf, size):
-        cdef char *buffer = <char *> buf
         if isinstance(path, int):
-            return c_fgetxattr(path, name, buffer, size, 0, XATTR_NOFLAGS)
+            return c_fgetxattr(path, name, <char *> buf, size, 0, XATTR_NOFLAGS)
         else:
             if follow_symlinks:
-                return c_getxattr(path, name, buffer, size, 0, XATTR_NOFLAGS)
+                return c_getxattr(path, name, <char *> buf, size, 0, XATTR_NOFLAGS)
             else:
-                return c_getxattr(path, name, buffer, size, 0, XATTR_NOFOLLOW)
+                return c_getxattr(path, name, <char *> buf, size, 0, XATTR_NOFOLLOW)
 
     n, buf = _getxattr_inner(func, path, name)
-    return buf[:n] or None
+    return bytes(buf[:n])
 
 
 def setxattr(path, name, value, *, follow_symlinks=True):
     def func(path, name, value, size):
-        cdef char *val = NULL if value is None else <char *> value
         if isinstance(path, int):
-            return c_fsetxattr(path, name, val, size, 0, XATTR_NOFLAGS)
+            return c_fsetxattr(path, name, <char *> value, size, 0, XATTR_NOFLAGS)
         else:
             if follow_symlinks:
-                return c_setxattr(path, name, val, size, 0, XATTR_NOFLAGS)
+                return c_setxattr(path, name, <char *> value, size, 0, XATTR_NOFLAGS)
             else:
-                return c_setxattr(path, name, val, size, 0, XATTR_NOFOLLOW)
+                return c_setxattr(path, name, <char *> value, size, 0, XATTR_NOFOLLOW)
 
     _setxattr_inner(func, path, name, value)
 

src/borg/platform/freebsd.pyx

@@ -50,14 +50,13 @@ cdef extern from "unistd.h":
 
 def listxattr(path, *, follow_symlinks=True):
     def func(path, buf, size):
-        cdef char *buffer = <char *> buf
         if isinstance(path, int):
-            return c_extattr_list_fd(path, EXTATTR_NAMESPACE_USER, buffer, size)
+            return c_extattr_list_fd(path, EXTATTR_NAMESPACE_USER, <char *> buf, size)
         else:
             if follow_symlinks:
-                return c_extattr_list_file(path, EXTATTR_NAMESPACE_USER, buffer, size)
+                return c_extattr_list_file(path, EXTATTR_NAMESPACE_USER, <char *> buf, size)
             else:
-                return c_extattr_list_link(path, EXTATTR_NAMESPACE_USER, buffer, size)
+                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]
@@ -65,29 +64,27 @@ def listxattr(path, *, follow_symlinks=True):
 
 def getxattr(path, name, *, follow_symlinks=True):
     def func(path, name, buf, size):
-        cdef char *buffer = <char *> buf
         if isinstance(path, int):
-            return c_extattr_get_fd(path, EXTATTR_NAMESPACE_USER, name, buffer, size)
+            return c_extattr_get_fd(path, EXTATTR_NAMESPACE_USER, name, <char *> buf, size)
         else:
             if follow_symlinks:
-                return c_extattr_get_file(path, EXTATTR_NAMESPACE_USER, name, buffer, size)
+                return c_extattr_get_file(path, EXTATTR_NAMESPACE_USER, name, <char *> buf, size)
             else:
-                return c_extattr_get_link(path, EXTATTR_NAMESPACE_USER, name, buffer, size)
+                return c_extattr_get_link(path, EXTATTR_NAMESPACE_USER, name, <char *> buf, size)
 
     n, buf = _getxattr_inner(func, path, name)
-    return buf[:n] or None
+    return bytes(buf[:n])
 
 
 def setxattr(path, name, value, *, follow_symlinks=True):
     def func(path, name, value, size):
-        cdef char *val = NULL if value is None else <char *> value
         if isinstance(path, int):
-            return c_extattr_set_fd(path, EXTATTR_NAMESPACE_USER, name, val, size)
+            return c_extattr_set_fd(path, EXTATTR_NAMESPACE_USER, name, <char *> value, size)
         else:
             if follow_symlinks:
-                return c_extattr_set_file(path, EXTATTR_NAMESPACE_USER, name, val, size)
+                return c_extattr_set_file(path, EXTATTR_NAMESPACE_USER, name, <char *> value, size)
             else:
-                return c_extattr_set_link(path, EXTATTR_NAMESPACE_USER, name, val, size)
+                return c_extattr_set_link(path, EXTATTR_NAMESPACE_USER, name, <char *> value, size)
 
     _setxattr_inner(func, path, name, value)
 

src/borg/platform/linux.pyx

@@ -78,14 +78,13 @@ _comment_re = re.compile(' *#.*', re.M)
 
 def listxattr(path, *, follow_symlinks=True):
     def func(path, buf, size):
-        cdef char *buffer = <char *> buf
         if isinstance(path, int):
-            return c_flistxattr(path, buffer, size)
+            return c_flistxattr(path, <char *> buf, size)
         else:
             if follow_symlinks:
-                return c_listxattr(path, buffer, size)
+                return c_listxattr(path, <char *> buf, size)
             else:
-                return c_llistxattr(path, buffer, size)
+                return c_llistxattr(path, <char *> buf, size)
 
     n, buf = _listxattr_inner(func, path)
     return [os.fsdecode(name) for name in split_string0(buf[:n])
@@ -94,30 +93,28 @@ def listxattr(path, *, follow_symlinks=True):
 
 def getxattr(path, name, *, follow_symlinks=True):
     def func(path, name, buf, size):
-        cdef char *buffer = <char *> buf
         if isinstance(path, int):
-            return c_fgetxattr(path, name, buffer, size)
+            return c_fgetxattr(path, name, <char *> buf, size)
         else:
             if follow_symlinks:
-                return c_getxattr(path, name, buffer, size)
+                return c_getxattr(path, name, <char *> buf, size)
             else:
-                return c_lgetxattr(path, name, buffer, size)
+                return c_lgetxattr(path, name, <char *> buf, size)
 
     n, buf = _getxattr_inner(func, path, name)
-    return buf[:n] or None
+    return bytes(buf[:n])
 
 
 def setxattr(path, name, value, *, follow_symlinks=True):
     def func(path, name, value, size):
-        cdef char *val = NULL if value is None else <char *> value
         flags = 0
         if isinstance(path, int):
-            return c_fsetxattr(path, name, val, size, flags)
+            return c_fsetxattr(path, name, <char *> value, size, flags)
         else:
             if follow_symlinks:
-                return c_setxattr(path, name, val, size, flags)
+                return c_setxattr(path, name, <char *> value, size, flags)
             else:
-                return c_lsetxattr(path, name, val, size, flags)
+                return c_lsetxattr(path, name, <char *> value, size, flags)
 
     _setxattr_inner(func, path, name, value)
 

src/borg/testsuite/archiver.py

@@ -1229,7 +1229,7 @@ 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', None, follow_symlinks=False)
+            xattr.setxattr(fd, 'security.capability', b'', follow_symlinks=False)
             fchown(fd, uid, gid)
 
         # The capability descriptor used here is valid and taken from a /usr/bin/ping
@@ -2187,8 +2187,7 @@ class ArchiverTestCase(ArchiverTestCaseBase):
                 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'
-                    # Special case: getxattr returns None (not b'') when reading an empty xattr.
-                    assert xattr.getxattr(out_fn, 'user.empty') is None
+                    assert xattr.getxattr(out_fn, 'user.empty') == b''
                 else:
                     assert xattr.listxattr(out_fn) == []
                     try:

src/borg/testsuite/xattr.py

@@ -32,7 +32,7 @@ class XattrTestCase(BaseTestCase):
         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', None)
+        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'])
@@ -40,7 +40,7 @@ class XattrTestCase(BaseTestCase):
         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'), None)
+        self.assert_equal(getxattr(self.tmpfile.name, 'user.empty'), b'')
 
     def test_listxattr_buffer_growth(self):
         # make it work even with ext4, which imposes rather low limits

src/borg/xattr.py

@@ -35,7 +35,9 @@ def get_all(path, follow_symlinks=True):
         names = listxattr(path, follow_symlinks=follow_symlinks)
         for name in names:
             try:
-                result[name] = getxattr(path, name, follow_symlinks=follow_symlinks)
+                # 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:
                 # if we get ENOATTR, a race has happened: xattr names were deleted after list.
                 # we just ignore the now missing ones. if you want consistency, do snapshots.