[PATCH 09/11] Fix hash(metadata) and add an explicit test; posix1e acls were lists

[Rob Browning]

Metadata.__hash__() was broken because the POSIX1e ACLs were being
loaded as lists, which can't be hashed. Change them to a tuple and
while we're at it, have bup_read_acl() return tuples too.

This was revealed by failing test_resolve and test_remote_resolve
tests when run as root on an ubuntu system with ext4 and a default
ACL.

Add test_maximal_metadata to create a metadata instance with as many
records as possible and ensure it can be hashed.

Signed-off-by: Rob Browning <r...@defaultvalue.org>
Tested-by: Rob Browning <r...@defaultvalue.org>
---
lib/bup/_helpers.c | 4 ++--
lib/bup/metadata.py | 2 +-
test/int/test_metadata.py | 40 +++++++++++++++++++++++++++++++++++++--
3 files changed, 41 insertions(+), 5 deletions(-)

diff --git a/lib/bup/_helpers.c b/lib/bup/_helpers.c
index 78e7b1df..9aa40101 100644
--- a/lib/bup/_helpers.c
+++ b/lib/bup/_helpers.c
@@ -1625,7 +1625,7 @@ static PyObject *bup_read_acl(PyObject *self, PyObject *args)
if (rv)
goto out;

- ret = Py_BuildValue("[" cstr_argf cstr_argf cstr_argf cstr_argf "]",
+ ret = Py_BuildValue("(" cstr_argf cstr_argf cstr_argf cstr_argf ")",
acl_txt, acl_num, def_txt, def_num);

if (def_txt)
@@ -1633,7 +1633,7 @@ static PyObject *bup_read_acl(PyObject *self, PyObject *args)
if (def_num)
acl_free((acl_t)def_num);
} else {
- ret = Py_BuildValue("[" cstr_argf cstr_argf "]",
+ ret = Py_BuildValue("(" cstr_argf cstr_argf ")",
acl_txt, acl_num);
}

diff --git a/lib/bup/metadata.py b/lib/bup/metadata.py
index 99d34101..7c3d947e 100644
--- a/lib/bup/metadata.py
+++ b/lib/bup/metadata.py
@@ -606,7 +606,7 @@ class Metadata:
acl_rep = acl_rep[:2]
if version == 1:
acl_rep = self._correct_posix1e_v1_delimiters(acl_rep, self.path)
- self.posix1e_acl = acl_rep
+ self.posix1e_acl = tuple(acl_rep)

def _apply_posix1e_acl_rec(self, path, restore_numeric_ids=False):
if not self.posix1e_acl:
diff --git a/test/int/test_metadata.py b/test/int/test_metadata.py
index c076408c..0fd0c3c0 100644
--- a/test/int/test_metadata.py
+++ b/test/int/test_metadata.py
@@ -3,6 +3,7 @@ import errno, stat, subprocess
import os, sys
import pytest

+from buptest import exc
from wvpytest import *
import buptest

@@ -198,8 +199,8 @@ def test_apply_to_path_restricted_access(tmpdir):
if metadata.xattr:
try:
metadata.xattr.set(path, b'user.buptest', b'bup')
- except IOError as exc: # matches _apply_linux_xattr_rec
- if exc.errno not in (errno.EPERM, errno.EOPNOTSUPP):
+ except IOError as e: # matches _apply_linux_xattr_rec
+ if e.errno not in (errno.EPERM, errno.EOPNOTSUPP):
raise
print("failed to set test xattr")
m = metadata.from_path(path, archive_path=path, save_symlinks=True)
@@ -284,3 +285,38 @@ if xattr:
WVPASSEQ(xattr.get(path, b'user.foo'), b'bar')
finally:
cleanup_testfs(b'testfs.img', b'testfs')
+
+
+def test_maximal_metadata(tmpdir):
+ # Currently just tests that the hash computation isn't broken
+ if not sys.platform.startswith('linux'):
+ pytest.skip('skipping test -- not linux')
+ return
+ if not is_superuser() or detect_fakeroot():
+ pytest.skip('skipping test -- not superuser')
+ return
+ os.chdir(tmpdir) # reverted by common_test_environment
+ if not setup_testfs(b'testfs.img', b'testfs'):
+ pytest.skip('unable to set up test fs; skipping dependent tests')
+ return
+ try:
+ os.chdir(b'testfs')
+ with open('canary', 'wb') as f:
+ f.write(b'something')
+ try: # linux xattrs
+ exc((b'attr', b'-s', b'foo', b'-V', b'bar', b'canary'))
+ except FileNotFoundError:
+ pass
+ try: # posix1e acls
+ exc((b'setfacl', '-m', 'u:root:r', b'canary'))
+ except FileNotFoundError:
+ pass
+ try: # linux attrs
+ exc((b'chattr', b'+acd', b'canary'))
+ except FileNotFoundError:
+ pass
+ m = metadata.from_path(b'canary')
+ # Check that __hash__ works properly
+ assert isinstance(hash(m), int)
+ finally:
+ cleanup_testfs(b'testfs.img', b'testfs')
--
2.47.3