[apparmor] [PATCH] parser - fix trailing \\ in regex (was Re: [PATCH] [parsers] allow for nested alternations expressions)

[John Johansen]

On 11/06/2013 04:10 PM, Steve Beattie wrote:
> On Tue, Nov 05, 2013 at 12:29:56PM -0800, John Johansen wrote:
>> On 11/05/2013 11:33 AM, Steve Beattie wrote:
>>> On Mon, Nov 04, 2013 at 05:28:22PM -0800, John Johansen wrote:
>>>> On 11/04/2013 04:34 PM, Steve Beattie wrote:
>>>>> Well, part of the slowdown was me writing some unit tests for that
>>>>> function. Here's the patch that does that:
>>> [SNIP]
>>>>> +	//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?
>>>
>>> Ah, right, I'd forgotten about these. They're commented out
>>> because as-is, they fail; however I wasn't sure if that was the
>>> correct expected output. Basically, what happens is that if whatever
>>> follows isn't expecting an escape character, then the escaping '\' is
>>> dropped. Thus, the current behavior is that '\\' becomes '' and both
>>> '\\blort' and 'blort\\' become 'blort'.
>>>
>>> The question is, is this a bug? I think so... but I'm willing to hear
>>> countering arguments.
>>>
>> O_o  Its a bug
> 
> Alright, here's a patch that fixes the issue:
> 
> Signed-off-by: Steve Beattie <steve at nxnw.org>
> ---
>  parser/parser_regex.c |  140 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 110 insertions(+), 30 deletions(-)
> 
> Index: b/parser/parser_regex.c
> ===================================================================
> --- a/parser/parser_regex.c
> +++ b/parser/parser_regex.c
> @@ -326,9 +347,14 @@ static pattern_t convert_aaregex_to_pcre
>  		case '(':
>  		case ')':
>  			STORE("\\", dptr, 1);
> -			// fall through to default
> +			STORE(sptr, dptr, 1);
> +			break;
>  
>  		default:
> +			if (bEscape) {
> +				/* just a regular backslash */
> +				STORE("\\", dptr, 1);
> +			}
>  			STORE(sptr, dptr, 1);
>  			break;
>  		}	/* switch (*sptr) */
> @@ -344,6 +370,10 @@ static pattern_t convert_aaregex_to_pcre
>  		       progname);
>  	}
>  
> +	if ((error == e_no_error) && bEscape) {
> +		/* just a regular backslash */
> +		STORE("\\", dptr, 1);
> +	}

hrmmm, I am thinking this should be a bug otherwise if we ever expand the set
of supported escape characters we will have issue.

Oh and btw I do have plans to expand the supported set, well at least on the
pcre side which I also plan to expose



>  	/* anchor end and terminate pattern string */
>  	if ((error == e_no_error) && anchor) {
>  		STORE("$" , dptr, 1);
> 
> 
> 
>