[apparmor] [PATCH] [parsers] allow for nested alternations expressions

[John Johansen]

On 11/04/2013 04:34 PM, Steve Beattie wrote:
> On Mon, Nov 04, 2013 at 01:30:19AM -0800, John Johansen wrote:
>> On 11/01/2013 04:31 PM, Steve Beattie wrote:
>>> (Sorry it took so long to get to the review of this.)
>>>
>> np. its a bit ugly, thanks
> 
> Well, part of the slowdown was me writing some unit tests for that
> function. Here's the patch that does that:
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
>  parser/parser_regex.c |  102 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
> 
> Index: b/parser/parser_regex.c
> ===================================================================
> --- a/parser/parser_regex.c
> +++ b/parser/parser_regex.c
> @@ -1262,6 +1262,104 @@ static int test_filter_slashes(void)
>  	return rc;
>  }
>  
> +#define MY_REGEX_TEST(input, expected_str, expected_type)						\
> +	do {												\
> +		char tbuf[PATH_MAX + 3];								\
> +		char *test_string;									\
> +		pattern_t ptype;									\
> +		int pos;										\
> +													\
> +		test_string = strdup((input)); 								\
> +		ptype = convert_aaregex_to_pcre(test_string, 0, tbuf, PATH_MAX + 3, &pos);		\
> +		MY_TEST(strcmp(tbuf, (expected_str)) == 0, "simple regex conversion for '" input "'")	\
> +		MY_TEST(ptype == (expected_type), "simple regex conversion type check for '" input "'")	\
> +	}												\
> +	while (0)
> +
> +#define MY_REGEX_FAIL_TEST(input)						\
> +	do {												\
> +		char tbuf[PATH_MAX + 3];								\
> +		char *test_string;									\
> +		pattern_t ptype;									\
> +		int pos;										\
> +													\
> +		test_string = strdup((input)); 								\
> +		ptype = convert_aaregex_to_pcre(test_string, 0, tbuf, PATH_MAX + 3, &pos);		\
> +		MY_TEST(ptype == ePatternInvalid, "simple regex conversion invalid type check for '" input "'")	\
> +	}												\
> +	while (0)
> +
> +static int test_aaregex_to_pcre(void)
> +{
> +	int rc = 0;
> +
> +	MY_REGEX_TEST("/most/basic/test", "/most/basic/test", ePatternBasic);
> +
> +	//MY_REGEX_TEST("\\", "\\", ePatternBasic);
> +	MY_REGEX_TEST("\\\\", "\\\\", ePatternBasic);
> +	//MY_REGEX_TEST("\\blort", "\\blort", ePatternBasic);
> +	MY_REGEX_TEST("\\\\blort", "\\\\blort", ePatternBasic);
> +	//MY_REGEX_TEST("blort\\", "blort\\", ePatternBasic);
why are these 3 commented out?

> +	MY_REGEX_TEST("blort\\\\", "blort\\\\", ePatternBasic);
> +	MY_REGEX_TEST("*", "[^/\\x00]*", ePatternRegex);
> +	MY_REGEX_TEST("blort*", "blort[^/\\x00]*", ePatternRegex);
> +	MY_REGEX_TEST("*blort", "[^/\\x00]*blort", ePatternRegex);
> +	MY_REGEX_TEST("\\*", "\\*", ePatternBasic);
> +	MY_REGEX_TEST("blort\\*", "blort\\*", ePatternBasic);
> +	MY_REGEX_TEST("\\*blort", "\\*blort", ePatternBasic);
> +
> +	/* simple quoting */
> +	MY_REGEX_TEST("\\[", "\\[", ePatternBasic);
> +	MY_REGEX_TEST("\\]", "\\]", ePatternBasic);
> +	MY_REGEX_TEST("\\?", "?", ePatternBasic);
> +	MY_REGEX_TEST("\\{", "\\{", ePatternBasic);
> +	MY_REGEX_TEST("\\}", "\\}", ePatternBasic);
> +	MY_REGEX_TEST("\\,", ",", ePatternBasic);
> +	MY_REGEX_TEST("^", "\\^", ePatternBasic);
> +	MY_REGEX_TEST("$", "\\$", ePatternBasic);
> +	MY_REGEX_TEST(".", "\\.", ePatternBasic);
> +	MY_REGEX_TEST("+", "\\+", ePatternBasic);
> +	MY_REGEX_TEST("|", "\\|", ePatternBasic);
> +	MY_REGEX_TEST("(", "\\(", ePatternBasic);
> +	MY_REGEX_TEST(")", "\\)", ePatternBasic);
> +	MY_REGEX_TEST("\\^", "\\^", ePatternBasic);
> +	MY_REGEX_TEST("\\$", "\\$", ePatternBasic);
> +	MY_REGEX_TEST("\\.", "\\.", ePatternBasic);
> +	MY_REGEX_TEST("\\+", "\\+", ePatternBasic);
> +	MY_REGEX_TEST("\\|", "\\|", ePatternBasic);
> +	MY_REGEX_TEST("\\(", "\\(", ePatternBasic);
> +	MY_REGEX_TEST("\\)", "\\)", ePatternBasic);
> +
> +	/* simple character class tests */
> +	MY_REGEX_TEST("[blort]", "[blort]", ePatternRegex);
> +	MY_REGEX_FAIL_TEST("[blort");
> +	MY_REGEX_FAIL_TEST("b[lort");
> +	MY_REGEX_FAIL_TEST("blort[");
> +	MY_REGEX_FAIL_TEST("blort]");
> +	MY_REGEX_FAIL_TEST("blo]rt");
> +	MY_REGEX_FAIL_TEST("]blort");
> +
> +	/* simple alternation tests */
> +	MY_REGEX_TEST("{alpha,beta}", "(alpha|beta)", ePatternRegex);
> +	MY_REGEX_TEST("baz{alpha,beta}blort", "baz(alpha|beta)blort", ePatternRegex);
> +	MY_REGEX_FAIL_TEST("{beta}");
> +	MY_REGEX_FAIL_TEST("biz{beta");
> +	MY_REGEX_FAIL_TEST("biz}beta");
> +	MY_REGEX_FAIL_TEST("biz{be,ta");
> +	MY_REGEX_FAIL_TEST("biz,be}ta");
> +	MY_REGEX_FAIL_TEST("biz{}beta");
> +
> +	/* nested alternations */
> +	MY_REGEX_TEST("{{alpha,blort,nested},beta}", "((alpha|blort|nested)|beta)", ePatternRegex);
> +	MY_REGEX_FAIL_TEST("{{alpha,blort,nested}beta}");
> +	MY_REGEX_TEST("{{alpha,{blort,nested}},beta}", "((alpha|(blort|nested))|beta)", ePatternRegex);
> +	MY_REGEX_TEST("{{alpha,alpha{blort,nested}}beta,beta}", "((alpha|alpha(blort|nested))beta|beta)", ePatternRegex);
> +	MY_REGEX_TEST("{{alpha,alpha{blort,nested}}beta,beta}", "((alpha|alpha(blort|nested))beta|beta)", ePatternRegex);
> +	MY_REGEX_TEST("{{a,b{c,d}}e,{f,{g,{h{i,j,k},l}m},n}o}", "((a|b(c|d))e|(f|(g|(h(i|j|k)|l)m)|n)o)", ePatternRegex);
> +
> +	return rc;
> +}
> +
>  int main(void)
>  {
>  	int rc = 0;
> @@ -1271,6 +1369,10 @@ int main(void)
>  	if (retval != 0)
>  		rc = retval;
>  
> +	retval = test_aaregex_to_pcre();
> +	if (retval != 0)
> +		rc = retval;
> +
>  	return rc;
>  }
>  #endif /* UNIT_TEST */
> 
> which then fails in three cases where an unquoted ']' is given without a matching
> '[' (the quoted cases are accepted properly). Here's the patch to fix
> that:
> 
other than the comment question, this is great thanks

Acked-by: John Johansen <john.johansen at canonical.com>

> Signed-off-by: Steve Beattie <steve at nxnw.org>
this looks good too

Acked-by: John Johansen <john.johansen at canonical.com>

> ---
>  parser/parser_regex.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> Index: b/parser/parser_regex.c
> ===================================================================
> --- a/parser/parser_regex.c
> +++ b/parser/parser_regex.c
> @@ -236,6 +236,10 @@ static pattern_t convert_aaregex_to_pcre
>  				/* ] is a PCRE special character */
>  				STORE("\\]", dptr, 2);
>  			} else {
> +				if (incharclass == 0) {
> +					error = e_parse_error;
> +					PERROR(_("%s: Regex grouping error: Invalid close ], no matching open [ detected\n"), progname);
> +				}
>  				incharclass = 0;
>  				STORE(sptr, dptr, 1);
>  			}
> 
> 
>