[apparmor] [patch] [2/9] Add strip_parenthesis() to regex.py

[Kshitij Gupta]

On Tue, Feb 23, 2016 at 1:45 AM, Christian Boltz <apparmor at cboltz.de> wrote:

> Hello,
>
> Am Montag, 22. Februar 2016, 01:55:35 CET schrieb Kshitij Gupta:
> > On Mon, Feb 22, 2016 at 1:07 AM, Kshitij Gupta wrote:
> > > On Sun, Dec 27, 2015 at 8:34 PM, Christian Boltz wrote:
> > >> some dbus rule conditionals come with optional parenthesis. Instead
> > >> of making the regex even more complicated, use a small function to
> > >> strip those parenthesis.
> > >>
> > >> Also add some tests for strip_parenthesis() to test-regex.py.
> > >>
> > >>
> > >> [ 53-add-strip_parenthesis.diff ]
> > >>
> > >> === modified file ./utils/apparmor/regex.py
> > >> --- utils/apparmor/regex.py     2015-12-21 00:13:57.207799592 +0100
> > >> +++ utils/apparmor/regex.py     2015-12-24 01:19:47.916978461 +0100
> > >> @@ -128,6 +128,15 @@
> > >>
> > >>      return matches.group('magicpath')
> > >>
> > >> +def strip_parenthesis(data):
> > >> +    '''strips parenthesis from the given string and returns the
> > >> strip()ped result.
> > >> +       The parenthesis must be the first and last char, otherwise
> > >> they won't be removed.
> > >> +       Even if no parenthesis get removed, the result will be
> > >> strip()ped. +       '''
> > >> +    if data[0] + data[-1] == '()':
> > >> +        return data[1:-1].strip()
> > >> +    else:
> > >> +        return data.strip()
> > >
> > > Might be useful to do the data.strip() before the "data[0] +
> > > data[-1] == '()'" check, that way you don't miss on something like
> > > '(hey) '. This is because there doesn't seem to be a strong
> > > assumption that the data will already be stripped coz we return
> > > data.strip()
>
> I'm following the strip_quotes() behaviour in not stripping space before
> removing parenthesis, and I'm quite sure the regexes that "feed"
> strip_parenthesis() make sure there's no whitespace around.
> See for example RE_ACCESS_KEYWORDS and RE_FLAG in apparmor/rule/dbus.py
> (from the dbus patch series, not in bzr yet)
>
> OTOH, whitespace inside parenthesis is allowed, so it makes sense to
> strip() it after removing the parenthesis.
>
> > > Also, I don't see any harm in having the .strip before the check.
>
> In theory it could "hide" whitespace included by (IMHO) broken regexes,
> with all sort of follow-up problems. I hope not-removed parenthesis (if
> the string starts or ends with whitespace) are more visible and easier
> to find, so I prefer not to strip() whe whole string.
>
umm ok.

Acked-by: Kshitij Gupta <kgupta8592 at gmail.com>

>
> > >>  def strip_quotes(data):
> > >>      if data[0] + data[-1] == '""':
> > >> === modified file ./utils/test/test-regex_matches.py
> ...
>
>
> Regards,
>
> Christian Boltz
> --
> > Ich hätte auch nie geglaubt, das es 10 Minuten dauern kann, bis jemand
> > ohne Fehler einmal ein  lspci -v  fehlerfrei eingegeben hat.
> Hast Du auch erwähnt, daß man anschließend die RETURN-Taste drücken muß?
>                   [> Torsten Hallmann und Bernd Brodesser in suse-linux]
>
> --
> AppArmor mailing list
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at:
> https://lists.ubuntu.com/mailman/listinfo/apparmor
>
>


-- 
Regards,

Kshitij Gupta
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160223/c1bb145b/attachment.html>