improve acl_get / acl_set error handling, see #4049

src/borg/platform/darwin.pyx

@@ -1,6 +1,7 @@
 import os
 
 from libc.stdint cimport uint32_t
+from libc cimport errno
 
 from .posix import user2uid, group2gid
 from ..helpers import safe_decode, safe_encode
@@ -115,20 +116,25 @@ def _remove_non_numeric_identifier(acl):
 def acl_get(path, item, st, numeric_ids=False, fd=None):
     cdef acl_t acl = NULL
     cdef char *text = NULL
+    if isinstance(path, str):
+        path = os.fsencode(path)
     try:
         if fd is not None:
             acl = acl_get_fd_np(fd, ACL_TYPE_EXTENDED)
         else:
-            if isinstance(path, str):
-                path = os.fsencode(path)
             acl = acl_get_link_np(path, ACL_TYPE_EXTENDED)
-        if acl is not NULL:
+        if acl == NULL:
-            text = acl_to_text(acl, NULL)
+            if errno.errno == errno.ENOENT:
-            if text is not NULL:
+                # macOS weirdness: if a file has no ACLs, it sets errno to ENOENT. :-(
-                if numeric_ids:
+                return
-                    item['acl_extended'] = _remove_non_numeric_identifier(text)
+            raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
-                else:
+        text = acl_to_text(acl, NULL)
-                    item['acl_extended'] = text
+        if text == NULL:
+            return
+        if numeric_ids:
+            item['acl_extended'] = _remove_non_numeric_identifier(text)
+        else:
+            item['acl_extended'] = text
     finally:
         acl_free(text)
         acl_free(acl)
@@ -143,12 +149,15 @@ def acl_set(path, item, numeric_ids=False, fd=None):
                 acl = acl_from_text(acl_text)
             else:
                 acl = acl_from_text(<bytes>_remove_numeric_id_if_possible(acl_text))
-            if acl is not NULL:
+            if acl == NULL:
-                if fd is not None:
+                return
-                    acl_set_fd_np(fd, acl, ACL_TYPE_EXTENDED)
+            if isinstance(path, str):
-                else:
+                path = os.fsencode(path)
-                    if isinstance(path, str):
+            if fd is not None:
-                        path = os.fsencode(path)
+                if acl_set_fd_np(fd, acl, ACL_TYPE_EXTENDED) == -1:
-                    acl_set_link_np(path, ACL_TYPE_EXTENDED, acl)
+                    raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
+            else:
+                if acl_set_link_np(path, ACL_TYPE_EXTENDED, acl) == -1:
+                    raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
         finally:
             acl_free(acl)

src/borg/platform/freebsd.pyx

@@ -1,15 +1,13 @@
 import os
 
+from libc cimport errno
+
 from .posix import posix_acl_use_stored_uid_gid
 from ..helpers import safe_encode, safe_decode
 from .xattr import _listxattr_inner, _getxattr_inner, _setxattr_inner, split_lstring
 
 API_VERSION = '1.2_05'
 
-cdef extern from "errno.h":
-    int errno
-    int EINVAL
-
 cdef extern from "sys/extattr.h":
     ssize_t c_extattr_list_file "extattr_list_file" (const char *path, int attrnamespace, void *data, size_t nbytes)
     ssize_t c_extattr_list_link "extattr_list_link" (const char *path, int attrnamespace, void *data, size_t nbytes)
@@ -138,6 +136,8 @@ cdef _get_acl(p, type, item, attribute, flags, fd=None):
     finally:
         acl_free(text)
         acl_free(acl)
+    else:
+        raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(p))
 
 
 def acl_get(path, item, st, numeric_ids=False, fd=None):
@@ -149,7 +149,7 @@ def acl_get(path, item, st, numeric_ids=False, fd=None):
     if isinstance(path, str):
         path = os.fsencode(path)
     ret = lpathconf(path, _PC_ACL_NFS4)
-    if ret < 0 and errno == EINVAL:
+    if ret < 0 and errno.errno == errno.EINVAL:
         return
     flags |= ACL_TEXT_NUMERIC_IDS if numeric_ids else 0
     if ret > 0:
@@ -167,15 +167,17 @@ cdef _set_acl(path, type, item, attribute, numeric_ids=False, fd=None):
             text = _nfs4_use_stored_uid_gid(text)
         elif numeric_ids and type in (ACL_TYPE_ACCESS, ACL_TYPE_DEFAULT):
             text = posix_acl_use_stored_uid_gid(text)
-        try:
+        acl = acl_from_text(<bytes>text)
-            acl = acl_from_text(<bytes> text)
+        if acl:
-            if acl is not NULL:
+            try:
                 if fd is not None:
-                    acl_set_fd_np(fd, acl, type)
+                    if acl_set_fd_np(fd, acl, type) == -1:
+                        raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
                 else:
-                    acl_set_link_np(path, type, acl)
+                    if acl_set_link_np(path, type, acl) == -1:
-        finally:
+                        raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
-            acl_free(acl)
+            finally:
+                acl_free(acl)
 
 
 cdef _nfs4_use_stored_uid_gid(acl):

src/borg/platform/linux.pyx

@@ -252,23 +252,24 @@ def acl_get(path, item, st, numeric_ids=False, fd=None):
             access_acl = acl_get_fd(fd)
         else:
             access_acl = acl_get_file(path, ACL_TYPE_ACCESS)
-        if access_acl is not NULL:
+        if access_acl == NULL:
-            access_text = acl_to_text(access_acl, NULL)
+            raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
-            if access_text is not NULL:
-                item['acl_access'] = converter(access_text)
-    finally:
-        acl_free(access_text)
-        acl_free(access_acl)
-
-    try:
         if stat.S_ISDIR(st.st_mode):
             # only directories can have a default ACL. there is no fd-based api to get it.
             default_acl = acl_get_file(path, ACL_TYPE_DEFAULT)
-            if default_acl is not NULL:
+            if default_acl == NULL:
-                default_text = acl_to_text(default_acl, NULL)
+                raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
-                if default_text is not NULL:
+        if access_acl:
-                    item['acl_default'] = converter(default_text)
+            access_text = acl_to_text(access_acl, NULL)
+            if access_text:
+                item['acl_access'] = converter(access_text)
+        if default_acl:
+            default_text = acl_to_text(default_acl, NULL)
+            if default_text:
+                item['acl_default'] = converter(default_text)
     finally:
+        acl_free(access_text)
+        acl_free(access_acl)
         acl_free(default_text)
         acl_free(default_acl)
 
@@ -293,9 +294,11 @@ def acl_set(path, item, numeric_ids=False, fd=None):
             access_acl = acl_from_text(<bytes> converter(access_text))
             if access_acl is not NULL:
                 if fd is not None:
-                    acl_set_fd(fd, access_acl)
+                    if acl_set_fd(fd, access_acl) == -1:
+                        raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
                 else:
-                    acl_set_file(path, ACL_TYPE_ACCESS, access_acl)
+                    if acl_set_file(path, ACL_TYPE_ACCESS, access_acl) == -1:
+                        raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
         finally:
             acl_free(access_acl)
     default_text = item.get('acl_default')
@@ -304,7 +307,8 @@ def acl_set(path, item, numeric_ids=False, fd=None):
             default_acl = acl_from_text(<bytes> converter(default_text))
             if default_acl is not NULL:
                 # only directories can get a default ACL. there is no fd-based api to set it.
-                acl_set_file(path, ACL_TYPE_DEFAULT, default_acl)
+                if acl_set_file(path, ACL_TYPE_DEFAULT, default_acl) == -1:
+                    raise OSError(errno.errno, os.strerror(errno.errno), os.fsdecode(path))
         finally:
             acl_free(default_acl)