[apparmor] [PATCH] [parsers] allow for nested alternations expressions
Seth Arnold
seth.arnold at canonical.com
Tue Nov 5 01:42:29 UTC 2013
On Mon, Nov 04, 2013 at 04:34:40PM -0800, 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>
Acked-by: Seth Arnold <seth.arnold at canonical.com>
Note that the strdup() strings aren't ever free()d. It's probably fine for
tests this large, but if we go ten or hundred times more involved, it
might be more important. Meh.
> ---
> 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);
> + 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:
>
> Signed-off-by: Steve Beattie <steve at nxnw.org>
Acked-by: Seth Arnold <seth.arnold at canonical.com>
I suspect there is more work to be done in this block of code; '[' may
need a corresponding if (incharclass == 1) test, unless this is supposed
to work: [[] /* character class with a [ inside the class */
But this fix looks good on its own, so ack. :)
Thanks
> ---
> 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);
> }
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20131104/b1957f93/attachment-0001.pgp>
More information about the AppArmor
mailing list