<div dir="ltr">Hello, <div class="gmail_extra"> <div class="gmail_quote">On Mon, Jun 15, 2015 at 12:44 AM, Christian Boltz <<a href="mailto:apparmor@cboltz.de" target="_blank">apparmor@cboltz.de</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello, this patch moves re_match_include() to regex.py. The function is basically a wrapper around a regex, so regex.py is a much better home. While on it, rename the regex to RE_INCLUDE and compile it outside the function, which should result in a (small) performance improvement. Also adjust code calling it to the new location. Finally, add some tests for re_match_include() [ 49-move-re_match_include.diff ] === modified file utils/aa-mergeprof --- utils/aa-mergeprof 2015-06-14 20:33:26.203498911 +0200 +++ utils/aa-mergeprof 2015-06-14 20:13:24.204740154 +0200 @@ -17,9 +17,10 @@ import os import apparmor.aa -from apparmor.aa import available_buttons, combine_name, delete_duplicates, is_known_rule, match_includes, re_match_include +from apparmor.aa import available_buttons, combine_name, delete_duplicates, is_known_rule, match_includes import apparmor.aamode from apparmor.common import AppArmorException +from apparmor.regex import re_match_include import apparmor.severity import apparmor.cleanprofile as cleanprofile import apparmor.ui as aaui @@ -267,7 +268,7 @@ done = True elif ans == 'CMD_ALLOW': selection = options[selected] - inc = apparmor.aa.re_match_include(selection) + inc = re_match_include(selection) self.user.filelist[self.user.filename]['include'][inc] = True options.pop(selected) aaui.UI_Info(_('Adding %s to the file.') % selection) @@ -302,7 +303,7 @@ done = True elif ans == 'CMD_ALLOW': selection = options[selected] - inc = apparmor.aa.re_match_include(selection) + inc = re_match_include(selection) deleted = apparmor.aa.delete_duplicates(aa[profile][hat], inc) aa[profile][hat]['include'][inc] = True options.pop(selected) @@ -524,7 +525,7 @@ elif ans == 'CMD_ALLOW': path = options[selected] done = True - match = apparmor.aa.re_match_include(path) + match = re_match_include(path) if match: inc = match deleted = apparmor.aa.delete_duplicates(aa[profile][hat], inc) @@ -589,7 +590,7 @@ elif ans == 'CMD_NEW': arg = options[selected] - if not apparmor.aa.re_match_include(arg): + if not re_match_include(arg): ans = aaui.UI_GetString(_('Enter new path: '), arg) # if ans: # if not matchliteral(ans, path): @@ -603,7 +604,7 @@ elif ans == 'CMD_GLOB': newpath = options[selected].strip() - if not apparmor.aa.re_match_include(newpath): + if not re_match_include(newpath): newpath = apparmor.aa.glob_path(newpath) if newpath not in options: @@ -614,7 +615,7 @@ elif ans == 'CMD_GLOBEXT': newpath = options[selected].strip() - if not apparmor.aa.re_match_include(newpath): + if not re_match_include(newpath): newpath = apparmor.aa.glob_path_withext(newpath) if newpath not in options: === modified file utils/apparmor/aa.py --- utils/apparmor/aa.py 2015-06-14 20:33:26.205498791 +0200 +++ utils/apparmor/aa.py 2015-06-14 20:32:10.987010769 +0200 @@ -49,7 +49,7 @@ RE_PROFILE_HAT_DEF, RE_PROFILE_DBUS, RE_PROFILE_MOUNT, RE_PROFILE_SIGNAL, RE_PROFILE_PTRACE, RE_PROFILE_PIVOT_ROOT, RE_PROFILE_UNIX, RE_RULE_HAS_COMMA, RE_HAS_COMMENT_SPLIT, - strip_quotes, parse_profile_start_line ) + strip_quotes, parse_profile_start_line, re_match_include ) import apparmor.rules as aarules @@ -2110,15 +2110,6 @@ return newincludes -def re_match_include(path): - """Matches the path for include and returns the include path""" - regex_include = re.compile('^\s*#?include\s*<(.*)>\s*(#.*)?$') - match = regex_include.search(path) - if match: - return match.groups()[0] - else: - return None - def valid_include(profile, incname): if profile and profile['include'].get(incname, False): return False === modified file utils/apparmor/cleanprofile.py --- utils/apparmor/cleanprofile.py 2015-06-08 22:23:14.887806000 +0200 +++ utils/apparmor/cleanprofile.py 2015-06-14 20:29:20.957207783 +0200 @@ -12,6 +12,7 @@ # # ---------------------------------------------------------------------- import apparmor.aa as apparmor +from apparmor.regex import re_match_include class Prof(object): def __init__(self, filename): @@ -90,7 +91,7 @@ if not same_profile: deleted.append(entry) continue - if apparmor.re_match_include(rule) or apparmor.re_match_include(entry): + if re_match_include(rule) or re_match_include(entry): continue # Check if the rule implies entry if apparmor.matchliteral(rule, entry): === modified file utils/apparmor/regex.py --- utils/apparmor/regex.py 2015-06-14 20:33:26.205498791 +0200 +++ utils/apparmor/regex.py 2015-06-14 20:14:39.423202789 +0200 @@ -112,6 +112,19 @@ return result + +RE_INCLUDE = re.compile('^\s*#?include\s*<(.*)>\s*(#.*)?$') </blockquote><div>How about replacing the end part using RE_EOL? </div><div>Also the .* for include name means we can have #include<>? I would believe the check happens somewhere for valid paths this but why not deal with the easy case in regex itself? </div><div>Also, about should the regex be placed directly above the function using it or along with all the other regexes? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + +def re_match_include(path): + """Matches the path for include and returns the include path""" + match = RE_INCLUDE.search(path) + if match: + return match.groups()[0] </blockquote><div>hmm... for the matters of this function the second comment group in regex is wasted. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + else: + return None + + + </blockquote><div>Bikeshedding: 3 newlines or 2 newlines for separation? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> def strip_quotes(data): if data[0] + data[-1] == '""': return data[1:-1] === modified file utils/test/test-regex_matches.py --- utils/test/test-regex_matches.py 2015-06-14 20:33:26.206498731 +0200 +++ utils/test/test-regex_matches.py 2015-06-14 20:14:57.120135248 +0200 @@ -14,7 +14,7 @@ from common_test import AATest, setup_all_loops from apparmor.common import AppArmorBug -from apparmor.regex import strip_quotes, parse_profile_start_line, RE_PROFILE_START, RE_PROFILE_CAP +from apparmor.regex import strip_quotes, parse_profile_start_line, re_match_include, RE_PROFILE_START, RE_PROFILE_CAP class AARegexTest(AATest): @@ -465,6 +465,23 @@ with self.assertRaises(AppArmorBug): parse_profile_start_line(line, 'somefile') +class Test_re_match_include(AATest): + tests = [ + ('#include <abstractions/base>', 'abstractions/base' ), + ('#include <abstractions/base> # comment', 'abstractions/base' ), + (' #include <abstractions/base> ', 'abstractions/base' ), </blockquote><div>could probably also use #include<abstractions/base> the no space version? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + ('include <abstractions/base>', 'abstractions/base' ), # not supported by parser + # ('include foo', 'foo' ), # XXX not supported in tools yet + # ('include /foo/bar', '/foo/bar' ), # XXX not supported in tools yet + # ('include "foo"', 'foo' ), # XXX not supported in tools yet + # ('include "/foo/bar"', '/foo/bar' ), # XXX not supported in tools yet + (' some #include <abstractions/base>', None, ), + (' /etc/fstab r,', None, ), + ] + + def _run_test(self, params, expected): + self.assertEqual(re_match_include(params), expected) + </blockquote><div>You could probably add just copy-paste (with little tweaking) the above tests with the regex instead of the function and cover the comment part. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> class TestStripQuotes(AATest): def test_strip_quotes_01(self): </blockquote><div><div>With suggestions for regex and tests addressed. </div><div> Acked-by: Kshitij Gupta <<a href="mailto:kgupta8592@gmail.com" target="_blank">kgupta8592@gmail.com</a>>. </div><div>Thanks. </div>Regards, </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Regards, Christian Boltz -- > > [OOo splash] kroete should turn their eyes during load ;-) > It should roll with his eyes if user is doing something completely > stupid. ...like touching the mouse, or do you know even better heuristics? [> Robert Schiele and Hans-Peter Jansen] -- AppArmor mailing list <a href="mailto:AppArmor@lists.ubuntu.com">AppArmor@lists.ubuntu.com</a> Modify settings or unsubscribe at: <a href="https://lists.ubuntu.com/mailman/listinfo/apparmor" rel="noreferrer" target="_blank">https://lists.ubuntu.com/mailman/listinfo/apparmor</a> </blockquote></div> </div></div>