Hello,

as Kshitij mentioned, abstract methods should use NotImplementedError
instead of AppArmorBug.

While changing this, I noticed that __repr__() needs to be robust against
NotImplementedError because get_raw() is not available in BaseRule.
Therefore the patch changes __repr__() to catch NotImplementedError.

Of course the change to NotImplementedError also needs several
adjustments in the tests.


[ 19-baserule-use-NotImplementedError.diff ]

=== modified file ./utils/apparmor/rule/__init__.py
--- utils/apparmor/rule/__init__.py     2015-10-29 13:11:10.731878625 +0100
+++ utils/apparmor/rule/__init__.py     2015-10-29 13:12:50.523041470 +0100
@@ -52,7 +52,11 @@
 
     def __repr__(self):
         classname = self.__class__.__name__
-        return '<%s> ' % classname + self.get_raw()
+        try:
+            raw_content = self.get_raw()  # will fail for BaseRule
+            return '<%s> %s' % (classname, raw_content)
+        except NotImplementedError:
+            return '<%s (NotImplementedError - get_clean() not implemented?)>' 
% classname
 
     @classmethod
     def match(cls, raw_rule):
@@ -69,7 +73,7 @@
     @classmethod
     def _match(cls, raw_rule):
         '''parse raw_rule and return regex match object'''
-        raise AppArmorBug("'%s' needs to implement _match(), but didn't" % 
(str(cls)))
+        raise NotImplementedError("'%s' needs to implement _match(), but 
didn't" % (str(cls)))
 
     @classmethod
     def parse(cls, raw_rule):
@@ -83,12 +87,12 @@
     def _parse(cls, raw_rule):
         '''returns a Rule object created from parsing the raw rule.
            required to be implemented by subclasses; raise exception if not'''
-        raise AppArmorBug("'%s' needs to implement _parse(), but didn't" % 
(str(cls)))
+        raise NotImplementedError("'%s' needs to implement _parse(), but 
didn't" % (str(cls)))
 
     # @abstractmethod  FIXME - uncomment when python3 only
     def get_clean(self, depth=0):
         '''return clean rule (with default formatting, and leading whitespace 
as specified in the depth parameter)'''
-        raise AppArmorBug("'%s' needs to implement get_clean(), but didn't" % 
(str(self.__class__)))
+        raise NotImplementedError("'%s' needs to implement get_clean(), but 
didn't" % (str(self.__class__)))
 
     def get_raw(self, depth=0):
         '''return raw rule (with original formatting, and leading whitespace 
in the depth parameter)'''
@@ -121,7 +125,7 @@
     # @abstractmethod  FIXME - uncomment when python3 only
     def is_covered_localvars(self, other_rule):
         '''check if the rule-specific parts of other_rule is covered by this 
rule object'''
-        raise AppArmorBug("'%s' needs to implement is_covered_localvars(), but 
didn't" % (str(self)))
+        raise NotImplementedError("'%s' needs to implement 
is_covered_localvars(), but didn't" % (str(self)))
 
     def is_equal(self, rule_obj, strict=False):
         '''compare if rule_obj == self
@@ -142,7 +146,7 @@
     # @abstractmethod  FIXME - uncomment when python3 only
     def is_equal_localvars(self, other_rule):
         '''compare if rule-specific variables are equal'''
-        raise AppArmorBug("'%s' needs to implement is_equal_localvars(), but 
didn't" % (str(self)))
+        raise NotImplementedError("'%s' needs to implement 
is_equal_localvars(), but didn't" % (str(self)))
 
     def severity(self, sev_db):
         '''return severity of this rule, which can be:
@@ -178,7 +182,7 @@
     def logprof_header_localvars(self):
         '''return the headers (human-readable version of the rule) to display 
in aa-logprof for this rule object
            returns {'label1': 'value1', 'label2': 'value2'} '''
-        raise AppArmorBug("'%s' needs to implement logprof_header(), but 
didn't" % (str(self)))
+        raise NotImplementedError("'%s' needs to implement logprof_header(), 
but didn't" % (str(self)))
 
     def modifiers_str(self):
         '''return the allow/deny and audit keyword as string, including 
whitespace'''
@@ -336,7 +340,7 @@
     def get_glob_ext(self, path_or_rule):
         '''returns the next possible glob with extension (for file rules only).
            For all other rule types, raise an exception'''
-        raise AppArmorBug("get_glob_ext is not available for this rule type!")
+        raise NotImplementedError("get_glob_ext is not available for this rule 
type!")
 
 
 def parse_comment(matches):
=== modified file ./utils/test/test-baserule.py
--- utils/test/test-baserule.py 2015-10-29 13:11:10.802874470 +0100
+++ utils/test/test-baserule.py 2015-10-29 12:12:19.971584508 +0100
@@ -20,34 +20,34 @@
 
 class TestBaserule(AATest):
     def test_abstract__parse(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             BaseRule._parse('foo')
 
     def test_abstract__parse_2(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             BaseRule.parse('foo')
 
     def test_abstract__match(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             BaseRule._match('foo')
 
     def test_abstract__match2(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             BaseRule.match('foo')
 
     def test_abstract_get_clean(self):
         obj = BaseRule()
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             obj.get_clean()
 
     def test_is_equal_localvars(self):
         obj = BaseRule()
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             obj.is_equal_localvars(BaseRule())
 
     def test_is_covered_localvars(self):
         obj = BaseRule()
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             obj.is_covered_localvars(None)
 
     def test_parse_modifiers_invalid(self):
@@ -65,7 +65,7 @@
 
     def test_logprof_header_localvars(self):
         obj = BaseRule()
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             obj.logprof_header_localvars()
 
 
=== modified file ./utils/test/test-capability.py
--- utils/test/test-capability.py       2015-10-29 13:11:10.802874470 +0100
+++ utils/test/test-capability.py       2015-10-29 12:21:13.288600004 +0100
@@ -635,7 +635,7 @@
         self.assertEqual(self.ruleset.get_glob('capability net_raw,'), 
'capability,')
 
     def test_glob_ext(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             self.ruleset.get_glob_ext('capability net_raw,')
 
 class CapabilityDeleteTest(AATest):
=== modified file ./utils/test/test-change_profile.py
--- utils/test/test-change_profile.py   2015-10-29 13:11:10.803874412 +0100
+++ utils/test/test-change_profile.py   2015-10-29 12:16:05.183514365 +0100
@@ -449,7 +449,7 @@
     #     self.assertEqual(self.ruleset.get_glob('change_profile /foo -> 
/bar,'), 'change_profile -> /bar,')
 
     def test_glob_ext(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             # get_glob_ext is not available for change_profile rules
             self.ruleset.get_glob_ext('change_profile /foo -> /bar,')
 
=== modified file ./utils/test/test-network.py
--- utils/test/test-network.py  2015-10-29 13:11:10.844872012 +0100
+++ utils/test/test-network.py  2015-10-29 12:19:48.363533637 +0100
@@ -441,7 +441,7 @@
     #     self.assertEqual(self.ruleset.get_glob('network inet raw,'), 
'network inet,')
 
     def test_glob_ext(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             # get_glob_ext is not available for network rules
             self.ruleset.get_glob_ext('network inet raw,')
 
=== modified file ./utils/test/test-rlimit.py
--- utils/test/test-rlimit.py   2015-10-29 13:11:10.844872012 +0100
+++ utils/test/test-rlimit.py   2015-10-29 12:19:13.128581461 +0100
@@ -411,7 +411,7 @@
     #     self.assertEqual(self.ruleset.get_glob('rlimit /foo -> /bar,'), 
'rlimit -> /bar,')
 
     def test_glob_ext(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             # get_glob_ext is not available for rlimit rules
             self.ruleset.get_glob_ext('set rlimit cpu <= 100,')
 
=== modified file ./utils/test/test-signal.py
--- utils/test/test-signal.py   2015-10-29 13:11:10.844872012 +0100
+++ utils/test/test-signal.py   2015-10-29 12:20:30.940059830 +0100
@@ -581,7 +581,7 @@
     #     self.assertEqual(self.ruleset.get_glob('signal send raw,'), 'signal 
send,')
 
     def test_glob_ext(self):
-        with self.assertRaises(AppArmorBug):
+        with self.assertRaises(NotImplementedError):
             # get_glob_ext is not available for signal rules
             self.ruleset.get_glob_ext('signal send set=int,')
 


Regards,

Christian Boltz
-- 
> > Gibt's mehr Gehalt? ;)
> Du glaubst doch nicht wirklich, dass ich über solche Dinge öffentlich
> rede, oder?
Du brauchst ja keine Summe nennen. Es reicht wenn Du uns alle hier mal
auf eine Pizza einlädst ;-)))
[>> David Haller, > Philipp Thomas und Thorsten Körner in suse-linux]


-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to