Merge ProfileStorage: Store empty xattrs as empty string

... instead of None.

This avoids the need to allow type changes (None vs. str).

Also adjust the tests accordingly.

While on it, simplify the tests for attachment.

attachment is always a str, therefore adjust the test to expect an empty
str ('') instead of None - and later converting that None to ''.

MR: https://gitlab.com/apparmor/apparmor/-/merge_requests/786
Acked-by: John Johansen <john.johansen@canonical.com>
This commit is contained in:
John Johansen 2021-08-20 21:55:22 +00:00
commit 5068f54cf1
2 changed files with 12 additions and 15 deletions

View file

@ -107,7 +107,7 @@ class ProfileStorage:
raise AppArmorBug('Attempt to change type of "%s" from %s to %s, value %s' % (key, type(self.data[key]), type(value), value))
# allow writing str or None to some keys
elif key in ('xattrs', 'flags', 'filename'):
elif key in ('flags', 'filename'):
if type_is_str(value) or value is None:
self.data[key] = value
else:
@ -249,7 +249,7 @@ class ProfileStorage:
else:
prof_storage['profile_keyword'] = matches['profile_keyword']
prof_storage['attachment'] = matches['attachment'] or ''
prof_storage['xattrs'] = matches['xattrs']
prof_storage['xattrs'] = matches['xattrs'] or ''
return (profile, hat, prof_storage)

View file

@ -122,15 +122,15 @@ class TestSetInvalid(AATest):
class AaTest_parse_profile_start(AATest):
tests = [
# profile start line profile hat profile hat attachment xattrs flags pps_set_hat_external
(('/foo {', None, None), ('/foo', '/foo', None, None, None, False)),
(('/foo (complain) {', None, None), ('/foo', '/foo', None, None, 'complain', False)),
(('profile foo /foo {', None, None), ('foo', 'foo', '/foo', None, None, False)), # named profile
(('profile /foo {', '/bar', None), ('/bar', '/foo', None, None, None, False)), # child profile
(('/foo//bar {', None, None), ('/foo', 'bar', None, None, None, True )), # external hat
(('profile "/foo" (complain) {', None, None), ('/foo', '/foo', None, None, 'complain', False)),
(('profile "/foo" xattrs=(user.bar=bar) {', None, None), ('/foo', '/foo', None, 'user.bar=bar', None, False)),
(('profile "/foo" xattrs=(user.bar=bar user.foo=*) {', None, None), ('/foo', '/foo', None, 'user.bar=bar user.foo=*', None, False)),
(('/usr/bin/xattrs-test xattrs=(myvalue="foo.bar") {', None, None), ('/usr/bin/xattrs-test', '/usr/bin/xattrs-test', None, 'myvalue="foo.bar"', None, False)),
(('/foo {', None, None), ('/foo', '/foo', '', '', None, False)),
(('/foo (complain) {', None, None), ('/foo', '/foo', '', '', 'complain', False)),
(('profile foo /foo {', None, None), ('foo', 'foo', '/foo', '', None, False)), # named profile
(('profile /foo {', '/bar', None), ('/bar', '/foo', '', '', None, False)), # child profile
(('/foo//bar {', None, None), ('/foo', 'bar', '', '', None, True )), # external hat
(('profile "/foo" (complain) {', None, None), ('/foo', '/foo', '', '', 'complain', False)),
(('profile "/foo" xattrs=(user.bar=bar) {', None, None), ('/foo', '/foo', '', 'user.bar=bar', None, False)),
(('profile "/foo" xattrs=(user.bar=bar user.foo=*) {', None, None), ('/foo', '/foo', '', 'user.bar=bar user.foo=*', None, False)),
(('/usr/bin/xattrs-test xattrs=(myvalue="foo.bar") {', None, None), ('/usr/bin/xattrs-test', '/usr/bin/xattrs-test', '', 'myvalue="foo.bar"', None, False)),
]
def _run_test(self, params, expected):
@ -138,10 +138,7 @@ class AaTest_parse_profile_start(AATest):
self.assertEqual(profile, expected[0])
self.assertEqual(hat, expected[1])
if expected[2] is None:
self.assertEqual(prof_storage['attachment'], '')
else:
self.assertEqual(prof_storage['attachment'], expected[2])
self.assertEqual(prof_storage['attachment'], expected[2])
self.assertEqual(prof_storage['xattrs'], expected[3])
self.assertEqual(prof_storage['flags'], expected[4])
self.assertEqual(prof_storage['is_hat'], False)