Hello, this patch converts serialize_parse_profile_start() to use parse_profile_start_line(), and adjusts a test to expect an AppArmorBug instead of an AttributeError exception.
The patch also adds two tests (they succeed with the old and the new code). Note that these tests document interesting[tm] behaviour - I tend to think that those cases should raise an exception, but I'm not sure about this because serialize_profile_from_old_profile() is a good example for interesting[tm] code :-/ I couldn't come up with a real-world test profile that would hit those cases without erroring out aa-logprof earlier - maybe the (more sane-looking) parse_profiles() / serialize_parse_profile_start() protects us from hitting this interesting[tm] behaviour. I propose this patch for trunk and 2.9. [ 14-convert-serialize_parse_profile_start-to-use-parse_profile_start_line.diff ] --- utils/apparmor/aa.py 2015-03-04 23:40:36.994571322 +0100 +++ utils/apparmor/aa.py 2015-03-05 20:55:05.702792046 +0100 @@ -3707,17 +3707,15 @@ def serialize_profile(profile_data, name return string + '\n' def serialize_parse_profile_start(line, file, lineno, profile, hat, prof_data_profile, prof_data_external, correct): - matches = RE_PROFILE_START.search(line).groups() - if profile and profile == hat and matches[3]: - hat = matches[3] + matches = parse_profile_start_line(line, file) + + if profile and profile == hat and matches['profile_keyword']: + hat = matches['profile'] in_contained_hat = True if prof_data_profile: pass else: - if matches[1]: - profile = matches[1] - else: - profile = matches[3] + profile = matches['profile'] if len(profile.split('//')) >= 2: profile, hat = profile.split('//')[:2] else: @@ -3728,10 +3726,7 @@ def serialize_parse_profile_start(line, else: hat = profile - flags = matches[6] - profile = strip_quotes(profile) - if hat: - hat = strip_quotes(hat) + flags = matches['flags'] return (profile, hat, flags, in_contained_hat, correct) --- utils/test/test-aa.py 2015-03-05 19:35:43.910167560 +0100 +++ utils/test/test-aa.py 2015-03-05 20:40:14.951079779 +0100 @@ -261,9 +261,19 @@ class AaTest_serialize_parse_profile_sta expected = ('/foo', '/foo', None, False, True) # note that in_contained_hat == False and that profile == hat == child profile self.assertEqual(result, expected) + def test_serialize_parse_profile_start_14(self): + result = self._parse('/ext//hat {', '/bar', '/bar', True, True) # external hat inside a profile - XXX should this error out? + expected = ('/ext', '/ext', None, False, True) # XXX additionally note that hat == profile, but should be 'hat' + self.assertEqual(result, expected) + + def test_serialize_parse_profile_start_15(self): + result = self._parse('/ext//hat {', '/bar', '/bar', True, False) # external hat inside a profile - XXX should this error out? + expected = ('/ext', 'hat', None, False, False) + self.assertEqual(result, expected) + def test_serialize_parse_profile_start_invalid_01(self): - with self.assertRaises(AttributeError): # XXX change to AppArmorBug? + with self.assertRaises(AppArmorBug): self._parse('xy', '/bar', '/bar', False, False) # not a profile start # XXX not catched as error. See also test_serialize_parse_profile_start_13() - maybe this is wanted behaviour here? Regards, Christian Boltz PS: random sig ;-)) -- > got a patch? -ENOTMYJOB [> Markus Rueckert and Bernhard Walle in opensuse-packaging] -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor