[apparmor] [patch] Raise AppArmorBug on unknown request_mask in logparser.py

[Christian Boltz]

Hello,

Am Freitag, 8. Januar 2016 schrieb Steve Beattie:
> On Thu, Jan 07, 2016 at 03:21:38PM +0100, Christian Boltz wrote:
> > Am Donnerstag, 7. Januar 2016 schrieb Steve Beattie:

> > > Which is what I think you desire.
> > 
> > Yes, that's much better than two backtraces.
> > 
> > In this special case, it's superfluous to display a backtrace at all
> > because the relevant information (the log line) is in the exception
> > message, but it's probably not worth special handling.
> 
> I agree, though I think it *could* be done in apparmor.fail by
> having the AppArmorBug class take an extra initializer option that
> displays tracebacks or not, which apparmor.fail.handle_exception()
> would then honor.

Indeed, that might be an option - but I still think it's not worth 
special handling ;-)

> > +                    if sys.version_info < (3, 0):
> > +                        raise AppArmorBug(ex_msg)
> > +                    else:
> > +                        # Drop the original AppArmorException by
> > passing None as the parent exception 
> > +                        raise AppArmorBug(ex_msg) from None

> NACK, this version won't work with python2 as it doesn't understand
> the python3 from Exception syntax, which it still attempts to parse
> even if it won't invoke it:

Argh, that's what I get from following my golden rules ("never test 
small changes") ;-)

> In order to make this work, you'd need to some kind of conditional
> import based on python major version that defined the functionality of
> a (e.g.) my_raise_from() function. Which is kind of ugly.

Agreed, so let's use the simple way and just display both exceptions in 
py3. I'll change that if we get a bugreport about crashing too verbose ;-)

I'll also add a comment for the "from None" part so that we can add it 
when we go py3-only - but going py3-only because of exception handling 
would be a strange reason ;-)


[ 38-more-useful-logparser-failure-reports.diff ]

=== modified file ./utils/apparmor/logparser.py
--- utils/apparmor/logparser.py 2016-01-07 14:53:52.853018485 +0100
+++ utils/apparmor/logparser.py 2016-01-07 15:11:58.033710237 +0100
@@ -17,7 +17,7 @@
 import sys
 import time
 import LibAppArmor
-from apparmor.common import AppArmorException, open_file_read, DebugLogger
+from apparmor.common import AppArmorException, AppArmorBug, open_file_read, DebugLogger
 
 from apparmor.aamode import validate_log_mode, log_str_to_mode, hide_log_mode, AA_MAY_EXEC
 
@@ -413,7 +413,14 @@
             event = self.parse_log_record(line)
             #print(event)
             if event:
-                self.add_event_to_tree(event)
+                try:
+                    self.add_event_to_tree(event)
+                except AppArmorException as e:
+                    ex_msg = ('%(msg)s\n\nThis error was caused by the log line:\n%(logline)s' %
+                            {'msg': e.value, 'logline': line})
+                    # when py3 only: Drop the original AppArmorException by passing None as the parent exception
+                    raise AppArmorBug(ex_msg)  # py3-only: from None
+
         self.LOG.close()
         self.logmark = ''
         return self.log


I hope this is the final patch ;-) and propose it for trunk, 2.10 and 2.9.

> > As I wrote above, I'll replace \n in ex_msg with spaces in 2.9
> > because it can't handle \n in exception messages.
> 
> Yeah, that part's fine, ugly traceback with log message embedded is
> better than one without.

;-)


Regards,

Christian Boltz
-- 
Let me know when you once find some actual outcome of SUCH a thread...
[Dominique Leuenberger in opensuse-factory]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160109/c20a651c/attachment.pgp>