Browse Source

xattrs: improve error handling, fixes #6988

looks like we can not rely on listxattr only returning
valid, acceptable names for getxattr.

so getxattr can still fail with EINVAL.

also:

- do not raise an exception if getting a single
xattr fails, rather emit a warning and continue
processing the remaining xattrs (and also the whole
file). we already had that for EPERM (and similar
for ENOATTR), just do it for all errors.

- _check, when it raises OSError, gives us a nice
exception object, use it.

- more helpful error msgs, try not to lose error information
Thomas Waldmann 2 years ago
parent
commit
a758fda089
1 changed files with 12 additions and 25 deletions
  1. 12 25
      src/borg/xattr.py

+ 12 - 25
src/borg/xattr.py

@@ -80,21 +80,14 @@ def get_all(path, follow_symlinks=False):
                 # borg always did it like that...
                 result[name] = getxattr(path, name, follow_symlinks=follow_symlinks) or None
             except OSError as e:
-                name_str = name.decode()
-                if isinstance(path, int):
-                    path_str = '<FD %d>' % path
-                else:
-                    path_str = os.fsdecode(path)
-                if e.errno == ENOATTR:
-                    # 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.
+                # note: platform.xattr._check has already made a nice exception e with errno, msg, path/fd
+                if e.errno in (ENOATTR, ):  # errors we just ignore silently
+                    # ENOATTR: a race has happened: xattr names were deleted after list.
                     pass
-                elif e.errno == errno.EPERM:
-                    # we were not permitted to read this attribute, still can continue trying to read others
-                    logger.warning('{}: Operation not permitted when reading extended attribute {}'.format(
-                                   path_str, name_str))
-                else:
-                    raise
+                else:  # all others: warn, skip this single xattr name, continue processing other xattrs
+                    # EPERM: we were not permitted to read this attribute
+                    # EINVAL: maybe xattr name is invalid or other issue, #6988
+                    logger.warning('when getting extended attribute %s: %s', name.decode(errors='replace'), str(e))
     except OSError as e:
         if e.errno in (errno.ENOTSUP, errno.EPERM):
             # if xattrs are not supported on the filesystem, we give up.
@@ -126,24 +119,18 @@ def set_all(path, xattrs, follow_symlinks=False):
             # if we have a None value, it means "empty", so give b'' to setxattr in that case:
             setxattr(path, k, v or b'', follow_symlinks=follow_symlinks)
         except OSError as e:
+            # note: platform.xattr._check has already made a nice exception e with errno, msg, path/fd
             warning = True
-            k_str = k.decode()
-            if isinstance(path, int):
-                path_str = '<FD %d>' % path
-            else:
-                path_str = os.fsdecode(path)
             if e.errno == errno.E2BIG:
-                err_str = 'too big for this filesystem'
-            elif e.errno == errno.ENOTSUP:
-                err_str = 'xattrs not supported on this filesystem'
+                err_str = 'too big for this filesystem (%s)' % str(e)
             elif e.errno == errno.ENOSPC:
                 # 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).
-                err_str = 'no space left on device [xattr len = %d]' % (len(v),)
+                err_str = 'fs full or xattr too big? [xattr len = %d] (%s)' % (len(v), str(e))
             else:
                 # generic handler
                 # EACCES: permission denied to set this specific xattr (this may happen related to security.* keys)
                 # EPERM: operation not permitted
-                err_str = os.strerror(e.errno)
-            logger.warning('%s: when setting extended attribute %s: %s', path_str, k_str, err_str)
+                err_str = str(e)
+            logger.warning('when setting extended attribute %s: %s', k.decode(errors='replace'), err_str)
     return warning