Comment 7 for bug 1528139

Revision history for this message
Christian Boltz (cboltz) wrote :

So I have good and bad news.

Let me start with the bad news:

profile_data / write_prof_data (in serialize_profile_from_old_profile()) contain only one profile with its hats. This will explode if a file contains multiple profiles, as reported in this bug.

Fixing this needs lots of write_prof_data[hat] -> write_prof_data[profile][hat] changes (and of course also a change in the calling code) or, better option, a full rewrite of serialize_profile_from_old_profile().

Unfortunately I don't have the time to do the rewrite at the moment (I have other things on my TODO list), and doing the write_prof_data[hat] -> write_prof_data[profile][hat] is something that might introduce more breakage, so I'm not too keen to do that.

The good news - at least I have a way to avoid the crash ;-)

I'll wrap the serialize_profile_from_old_profile() in try/except. If it fails, the diff will include an error message and recommend to use 'View Changes b/w (C)lean profiles' instead, which is known to work even with the testcase in this bug.

=== modified file ./utils/apparmor/aa.py
--- utils/apparmor/aa.py 2015-12-21 00:13:57.215799543 +0100
+++ utils/apparmor/aa.py 2015-12-21 23:55:01.858211661 +0100
@@ -2368,7 +2368,12 @@
                         oldprofile = aa[which][which]['filename']
                     else:
                         oldprofile = get_profile_filename(which)
- newprofile = serialize_profile_from_old_profile(aa[which], which, '')
+
+ try:
+ newprofile = serialize_profile_from_old_profile(aa[which], which, '')
+ except AttributeError:
+ # see https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1528139
+ newprofile = "###\n###\n### Internal error while generating diff, please use '%s' instead\n###\n###\n" % _('View Changes b/w (C)lean profiles')

                     display_changes_with_comments(oldprofile, newprofile)

Sorry that this isn't a perfect solution, but I'm not too keen to spent lots of time on a function that needs to be rewritten anyway.

For the records: this bug causes a crash in 2.10 and bzr trunk. 2.9.x "only" displays a wrong diff.