From a758fda0899f4654d6274db1b3c59427b6042dac Mon Sep 17 00:00:00 2001 From: Thomas Waldmann Date: Wed, 24 Aug 2022 12:03:51 +0200 Subject: [PATCH] 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 --- src/borg/xattr.py | 37 ++++++++++++------------------------- 1 file changed, 12 insertions(+), 25 deletions(-) diff --git a/src/borg/xattr.py b/src/borg/xattr.py index 8a731340c..7bc63a75d 100644 --- a/src/borg/xattr.py +++ b/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 = '' % 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 = '' % 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