[apparmor] [patch] simplify serialize_parse_profile_start()

[Christian Boltz]

Hello,

this patch changes serialize_parse_profile_start() to use 
parse_profile_start() instead of using duplicated code.

The behaviour is mostly kept, with the exception that the function is 
more strict now and raises exceptions instead of ignoring errors.

In practise, this won't change anything because the profiles are parsed
with parse_profile() (which calls parse_profile_start()) - and that 
already errors out.

The tests are updated to match the more strict behaviour.


The next step would be to drop serialize_parse_profile_start() 
completely, but this isn't urgent and can/should be done when we have 
test coverage for serialize_profile_from_old_profile() one day ;-)

As usual, this patch is for trunk and 2.9.


[ 27-simplify-serialize_parse_profile_data.diff ]

=== modified file utils/apparmor/aa.py
--- utils/apparmor/aa.py        2015-03-19 21:00:27.922067165 +0100
+++ utils/apparmor/aa.py        2015-03-19 21:00:00.992643780 +0100
@@ -3724,27 +3724,10 @@
     return string + '\n'
 
 def serialize_parse_profile_start(line, file, lineno, profile, hat, prof_data_profile, prof_data_external, correct):
-    matches = parse_profile_start_line(line, file)
+    (profile, hat, attachment, flags, in_contained_hat, pps_set_profile, pps_set_hat_external) = parse_profile_start(line, file, lineno, profile, hat)
 
-    if profile and profile == hat and matches['profile_keyword']:
-        hat = matches['profile']
-        in_contained_hat = True
-        if prof_data_profile:
-            pass
-    else:
-        profile = matches['profile']
-        if len(profile.split('//')) >= 2:
-            profile, hat = profile.split('//')[:2]
-        else:
-            hat = None
-        in_contained_hat = False
-        if hat and not prof_data_external:
-            correct = False
-        else:
-            hat = profile
-
-    attachment = matches['attachment']
-    flags = matches['flags']
+    if hat and profile != hat and '%s//%s'%(profile, hat) in line and not prof_data_external:
+        correct = False
 
     return (profile, hat, attachment, flags, in_contained_hat, correct)
 
=== modified file utils/test/test-aa.py
--- utils/test/test-aa.py       2015-03-19 21:00:27.941066053 +0100
+++ utils/test/test-aa.py       2015-03-19 21:31:20.465327311 +0100
@@ -454,30 +454,27 @@
         expected = ('/bar', '/foo', None, None, True, True)
         self.assertEqual(result, expected)
 
-    def test_serialize_parse_profile_start_13(self):
-        result = self._parse('/foo {', '/bar', '/bar', False, False) # child profile without 'profile' keyword - XXX should this error out?
-        expected = ('/foo', '/foo', None, 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, 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, None, False, False)
-        self.assertEqual(result, expected)
-
-
-    def test_serialize_parse_profile_start_invalid_01(self):
-        with self.assertRaises(AppArmorBug):
-            self._parse('xy', '/bar', '/bar', False, False) # not a profile start
+class AaTestInvalid_serialize_parse_profile_start(AATest):
+    tests = [
+        # line              profile     hat     p_d_profile p_d_external   expected
+        (['/foo {',         '/bar',     '/bar', False,      False       ], AppArmorException), # child profile without 'profile' keyword
+        (['profile /foo {', '/bar',     '/xy',  False,      False       ], AppArmorException), # already inside a child profile - nesting level reached
+        (['/ext//hat {',    '/bar',     '/bar', True,       True        ], AppArmorException), # external hat inside a profile
+        (['/ext//hat {',    '/bar',     '/bar', True,       False       ], AppArmorException), # external hat inside a profile
+        (['xy',             '/bar',     '/bar', False,      False       ], AppArmorBug      ), # not a profile start
+    ]
+
+    def _run_test(self, params, expected):
+        line = params[0]
+        profile = params[1]
+        hat = params[2]
+        prof_data_profile = params[3]
+        prof_data_external = params[4]
+
+        with self.assertRaises(expected):
+            # 'correct' is always True in the code that uses serialize_parse_profile_start() (set some lines above the function call)
+            serialize_parse_profile_start(line, 'somefile', 1, profile, hat, prof_data_profile, prof_data_external, True)
 
-    # XXX not catched as error. See also test_serialize_parse_profile_start_13() - maybe this is wanted behaviour here?
-    #def test_serialize_parse_profile_start_invalid_02(self):
-    #    with self.assertRaises(AppArmorException):
-    #        self._parse('/foo {', '/bar', '/bar', False, False) # child profile without profile keyword
 
 if __name__ == '__main__':
     setup_all_tests()



Regards,

Christian Boltz
-- 
> Was ist ein "umbrella bug"?
Eine Regenschirm-Wanze ;-)
[> Al Bogner und Andreas Winkelmann in suse-linux]