[apparmor] [patch] [02/38] check_and_split_list: optionally allow empty list

[Christian Boltz]

Hello,

Am Dienstag, 13. September 2016, 22:08:00 CEST schrieb Seth Arnold:
> On Fri, Aug 12, 2016 at 10:44:14PM +0200, Christian Boltz wrote:
> > $subject.
> > 
> > File permissions can be an empty list (if only exec permissions are
> > specified). This patch adds the optional allow_empty_list parameter
> > so that the function can handle this case.
> > 
> > 
> > 
> > [ 02-check_and_split_list-optionally-allow-empty-list.diff ]
> 
> This change looks fine but I've got questions about the overall function:
> > --- utils/apparmor/rule/__init__.py	2016-01-07 20:41:32.718787664
> > +0100 +++ utils/apparmor/rule/__init__.py	2016-01-15
> > 23:00:18.765000352 +0100 @@ -431,14 +431,14 @@
> > 
> >          raise NotImplementedError("get_glob_ext is not available
> >          for this rule type!")> 
> > -def check_and_split_list(lst, allowed_keywords, all_obj, classname,
> > keyword_name):> 
> > +def check_and_split_list(lst, allowed_keywords, all_obj, classname, keyword_name, allow_empty_list=False):
> >      '''check if lst is all_obj or contains only items listed in
> >      allowed_keywords'''>      
> >      if lst == all_obj:
> >          return None, True, None
> >      
> >      elif type_is_str(lst):
> >          result_list = {lst}
> > 
> > -    elif (type(lst) == list or type(lst) == tuple) and len(lst) >
> > 0:
> > 
> > +    elif (type(lst) == list or type(lst) == tuple) and (len(lst) > 0 or allow_empty_list):
> >          result_list = set(lst)
> >      
> >      else:
> >          raise AppArmorBug('Passed unknown %(type)s object to
> >          %(classname)s: %(unknown_object)s' %
> Is there a reason why a string input is turned into a list, 

This is a simplification for callers - you can hand over
a) a list, tuple or set [1] [2] with any number of items
b) a string if you have only one item

The reason for

      elif type_is_str(lst):
          result_list = {lst}

is to wrap the string into a set (with this string as only item).
Without that, the for loop would run once for each char in the string.

> but a list input is turned into a set?

That gives us de-duplication for free ;-)


BTW: 

    >>> x = {'foo'}
    >>> type(x)
    <type 'set'>

So a string also gets wrapped into a set (not a list) ;-)
(don't worry about getting it wrong - I also have to ask type() how an 
array is named ;-)

> How should an empty string be treated?

It will error out here (for empty string and whitespace-only string):

    for item in result_list: 
        if not item.strip():
            raise AppArmorBug('Passed empty %(keyword_name)s to %(classname)s' %
                    {'keyword_name': keyword_name, 'classname': classname})


Regards,

Christian Boltz


[1] set is added in 03/38

[2] basically "an array"
-- 
Wenn's eine kaputte Platte ist: Entsorgen, Backup zurückspielen.
Wenn's kein Backup gibt - nennt sich das ganze "lernen" ;-)
[Arno Lehmann in suse-linux]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160914/a286bd10/attachment.pgp>