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 <[email protected]>
---
 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 <[email protected]>
---
 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);
                        }

-- 
Steve Beattie
<[email protected]>
http://NxNW.org/~steve/

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
[email protected]
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to