[apparmor] new profile tools: preserve full initial comment

[Christian Boltz]

Hello,

Am Freitag, 21. Februar 2014 schrieb Steve Beattie:
> My apologies for the delay in reviewing this.

no problem - we'll see if you are faster with the follow-up patch ;-)

> On Wed, Feb 05, 2014 at 11:58:24PM +0100, Christian Boltz wrote:
...
> Acked-by: Steve Beattie <steve at nxnw.org>, though please kill the
> commented out lines before committing. Thanks!

Done.

> > BTW: It might be a good idea to use a different variable name for
> > the
> > result of line.split() to avoid confusion.
> 
> Yes, that's a bit confusing.

The patch below removes the confusion - it uses the (new) "parts" 
variable for the line.split result.

Additional changes:
- change the line.startswith to check for "REPOSITORY:" (note the 
  added ":") like it was in the code before my last patch.
- make the check for "NEVERSUBMIT" more exact
- print a warning on invalid REPOSITORY: lines and make sure to keep
  them as unmodified line (it might just be a "normal" comment someone
  added manually)


=== modified file 'utils/apparmor/aa.py'
--- utils/apparmor/aa.py        2014-02-22 12:09:15 +0000
+++ utils/apparmor/aa.py        2014-02-22 12:52:56 +0000
@@ -2939,14 +2939,17 @@
             if not profile:
                 if line.startswith('# Last Modified:'):
                     continue
-                elif line.startswith('# REPOSITORY'): # TODO: allow any number of spaces/tabs
-                    line = line.split()
-                    if len(line) == 3:
+                elif line.startswith('# REPOSITORY:'): # TODO: allow any number of spaces/tabs
+                    parts = line.split()
+                    if len(parts) == 3 and parts[2] == 'NEVERSUBMIT':
                         repo_data = {'neversubmit': True}
-                    elif len(line) == 5:
-                        repo_data = {'url': line[2],
-                                     'user': line[3],
-                                     'id': line[4]}
+                    elif len(parts) == 5:
+                        repo_data = {'url': parts[2],
+                                     'user': parts[3],
+                                     'id': parts[4]}
+                    else:
+                        aaui.UI_Important(_('Warning: invalid "REPOSITORY:" line in %s, ignoring.') % file)
+                        initial_comment = initial_comment + line + '\n'
                 else:
                     initial_comment = initial_comment + line + '\n'
 


That said - I tested aa-cleanprof with a REPOSITORY: line [1] added to a 
profile, which resulted in a nice backtrace (in a different section of 
the code) :-(  - so for now I recommend to a) avoid REPOSITORY: lines 
or b) fix it ;-)


Traceback (most recent call last):
  File "./aa-cleanprof", line 31, in <module>
    clean.act()
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/tools.py", line 71, in act
    apparmor.read_profiles()
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2529, in read_profiles
    read_profile(profile_dir + '/' + file, True)
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2555, in read_profile
    profile_data = parse_profile_data(data, file, 0)
  File "/home/cb/apparmor/HEAD-CLEAN/utils/apparmor/aa.py", line 2659, in parse_profile_data
    profile_data[profile][profile]['repo']['url'] = repo_data['url']
KeyError: 'url'



Regards,

Christian Boltz

[1] tested with (in different profiles):

    # REPOSITORY: NEVERSUBMIT
    # REPOSITORY: http://apparmor.opensuse.org/backend/api cboltz 3923
    # REPOSITORY: BROKEN
    (needless to say that the last one is expected to triggers the warning, 
    but at least it avoids the backtrace)

-- 
> [submit-request #65647 declined by saschpe:]
>   description is >400 lines, too long :-)
Where is a limit documented?
[Stephan Kulow in opensuse-packaging]