commit dd648b44760b9041d5ea4279e78ee7fb4e5aa13d Author: Arran Cudbard-Bell Date: Tue May 13 11:52:21 2014 +0100 Fix case insensitive matching in compiled regular expressions diff --git a/src/include/map.h b/src/include/map.h index ed73093..59732a1 100644 --- a/src/include/map.h +++ b/src/include/map.h @@ -155,11 +155,14 @@ typedef struct value_pair_tmpl_t { union { struct { - value_data_t const *value; //!< actual data - size_t length; //!< of the vpd data + value_data_t const *value; //!< actual data + size_t length; //!< of the vpd data } literal; xlat_exp_t *xlat; //!< pre-parsed xlat_exp_t - regex_t *preg; //!< pre-parsed regex_t + struct { + regex_t *comp; //!< pre-parsed regex_t + bool iflag; //!< Case insensitive + } preg; } data; } value_pair_tmpl_t; @@ -170,7 +173,9 @@ typedef struct value_pair_tmpl_t { #define vpt_tag attribute.tag #define vpt_xlat data.xlat -#define vpt_preg data.preg + +#define vpt_preg data.preg.comp +#define vpt_iflag data.preg.iflag #define vpt_value data.literal.value #define vpt_length data.literal.length diff --git a/src/include/parser.h b/src/include/parser.h index 5126edd..6aa858a 100644 --- a/src/include/parser.h +++ b/src/include/parser.h @@ -73,7 +73,6 @@ struct fr_cond_t { fr_cond_t *child; } data; - int regex_i; int negate; int pass2_fixup; diff --git a/src/main/evaluate.c b/src/main/evaluate.c index 60351c9..c21ed4f 100644 --- a/src/main/evaluate.c +++ b/src/main/evaluate.c @@ -240,16 +240,13 @@ int radius_evaluate_tmpl(REQUEST *request, int modreturn, UNUSED int depth, } -static int do_regex(REQUEST *request, value_pair_map_t const *map, bool iflag) +static int do_regex(REQUEST *request, value_pair_map_t const *map) { int compare, rcode, ret; - int cflags = REG_EXTENDED; regex_t reg, *preg; char *lhs, *rhs; regmatch_t rxmatch[REQUEST_MAX_REGEX + 1]; - if (iflag) cflags |= REG_ICASE; - /* * Expand and then compile it. */ @@ -262,7 +259,7 @@ static int do_regex(REQUEST *request, value_pair_map_t const *map, bool iflag) } rad_assert(rhs != NULL); - compare = regcomp(®, rhs, cflags); + compare = regcomp(®, rhs, REG_EXTENDED | (map->src->vpt_iflag ? REG_ICASE : 0)); if (compare != 0) { if (debug_flag) { char errbuf[128]; @@ -635,7 +632,7 @@ int radius_evaluate_map(REQUEST *request, UNUSED int modreturn, UNUSED int depth */ if ((map->src->type == VPT_TYPE_REGEX) || (map->src->type == VPT_TYPE_REGEX_STRUCT)) { - return do_regex(request, map, c->regex_i); + return do_regex(request, map); } #endif diff --git a/src/main/modcall.c b/src/main/modcall.c index 22ab00b..915c370 100644 --- a/src/main/modcall.c +++ b/src/main/modcall.c @@ -2829,7 +2829,7 @@ static bool pass2_regex_compile(CONF_ITEM const *ci, value_pair_tmpl_t *vpt) talloc_set_destructor(preg, _free_compiled_regex); if (!preg) return false; - rcode = regcomp(preg, vpt->name, REG_EXTENDED); + rcode = regcomp(preg, vpt->name, REG_EXTENDED | (vpt->vpt_iflag ? REG_ICASE : 0)); if (rcode != 0) { char buffer[256]; regerror(rcode, preg, buffer, sizeof(buffer)); diff --git a/src/main/parser.c b/src/main/parser.c index fcfd16e..738ca7f 100644 --- a/src/main/parser.c +++ b/src/main/parser.c @@ -340,7 +340,8 @@ static ssize_t condition_tokenize_cast(char const *start, DICT_ATTR const **pda, * @param[out] error the parse error (if any) * @return length of the string skipped, or when negative, the offset to the offending error */ -static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *start, int brace, fr_cond_t **pcond, char const **error, int flags) +static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *start, int brace, + fr_cond_t **pcond, char const **error, int flags) { ssize_t slen; char const *p = start; @@ -477,7 +478,8 @@ static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *st rad_assert(c->data.vpt->type != VPT_TYPE_REGEX); } else { /* it's an operator */ - int regex; + bool regex; + bool i_flag = false; /* * The next thing should now be a comparison operator. @@ -613,7 +615,7 @@ static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *st * Allow /foo/i */ if (p[slen] == 'i') { - c->regex_i = true; + i_flag = true; slen++; } @@ -637,6 +639,10 @@ static ssize_t condition_tokenize(TALLOC_CTX *ctx, CONF_ITEM *ci, char const *st return_0("Syntax error"); } + if (c->data.map->src->type == VPT_TYPE_REGEX) { + c->data.map->src->vpt_iflag = i_flag; + } + /* * Could have been a reference to an attribute which is registered later. * Mark it as being checked in pass2. @@ -1074,7 +1080,6 @@ done: rcode = radius_evaluate_map(NULL, 0, 0, c); TALLOC_FREE(c->data.map); c->cast = NULL; - c->regex_i = false; if (rcode) { c->type = COND_TYPE_TRUE; } else { @@ -1097,7 +1102,6 @@ done: int rcode; rad_assert(c->cast == NULL); - rad_assert(c->regex_i == false); rcode = radius_evaluate_map(NULL, 0, 0, c); if (rcode) { diff --git a/src/tests/keywords/if-regex-match b/src/tests/keywords/if-regex-match index f15e74f..f3e6aa9 100644 --- a/src/tests/keywords/if-regex-match +++ b/src/tests/keywords/if-regex-match @@ -1,16 +1,96 @@ # PRE: if # -# May as well exercise the regular expression engine -if (User-Name !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) { - reject + +# Non matching on attribute ref +if (User-Name !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])%{Tmp-String-0}/) { + update reply { + Filter-Id += 'Fail 0' + } } -if ("%{User-Name}" !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) { - reject +# Matching on xlat expanded value +if ("%{User-Name}" !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])%{Tmp-String-0}/) { + update reply { + Filter-Id += 'Fail 1' + } } -if (User-Name =~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) { +# Matching on attribute ref with capture groups +if (User-Name =~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])%{Tmp-String-0}/) { + # Test all the capture groups update { reply:User-Name := "%{7}_%{6}_%{5}_%{4}_%{3}_%{2}_%{1}_%{0}" } } +else { + update reply { + Filter-Id += 'Fail 2' + } +} + +# Checking capture groups are cleared out correctly +if (User-Name =~ /^([0-9])_%{Tmp-String-0}/) { + if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1_1') { + update reply { + Filter-Id += 'Fail 3' + } + } +} +else { + update reply { + Filter-Id += 'Fail 3.5' + } +} + +# Checking capture groups are cleared out correctly when there are no matches +if (User-Name =~ /^.%{Tmp-String-0}/) { + if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1') { + update reply { + Filter-Id += 'Fail 4' + } + } +} +else { + update reply { + Filter-Id += 'Fail 4.5' + } +} + +# compiled - ref - insensitive +if (Calling-Station-Id !~ /:roamyroam%{Tmp-String-0}$/i) { + update reply { + Filter-Id += 'Fail 5' + } +} + +# compiled - expansion - insensitive +if ("%{Calling-Station-Id}" !~ /:roamyroam%{Tmp-String-0}$/i) { + update reply { + Filter-Id += 'Fail 6' + } +} + +# compiled - enum - ref - insensitive +if (Service-Type !~ /^framed-user%{Tmp-String-0}$/i) { + update reply { + Filter-Id += 'Fail 7' + } +} + +# compiled - enum - expansion - insensitive +if ("%{Service-Type}" !~ /^framed-user%{Tmp-String-0}$/i) { + update reply { + Filter-Id += 'Fail 8' + } +} + +# compiled - enum - ref +if (Service-Type =~ /^framed-user%{Tmp-String-0}$/) { + update reply { + Filter-Id += 'Fail 9' + } +} + + + + diff --git a/src/tests/keywords/if-regex-match-comp b/src/tests/keywords/if-regex-match-comp new file mode 100644 index 0000000..8d68142 --- /dev/null +++ b/src/tests/keywords/if-regex-match-comp @@ -0,0 +1,94 @@ +# PRE: if +# + +# Non matching on attribute ref +if (User-Name !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) { + update reply { + Filter-Id += 'Fail 0' + } +} + +# Matching on xlat expanded value +if ("%{User-Name}" !~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) { + update reply { + Filter-Id += 'Fail 1' + } +} + +# Matching on attribute ref with capture groups +if (User-Name =~ /^([0-9])_([0-9])?_([0-9]*)_([0-9]+)_([^_])_(6)_([7-8])/) { + # Test all the capture groups + update { + reply:User-Name := "%{7}_%{6}_%{5}_%{4}_%{3}_%{2}_%{1}_%{0}" + } +} +else { + update reply { + Filter-Id += 'Fail 2' + } +} + +# Checking capture groups are cleared out correctly +if (User-Name =~ /^([0-9])_/) { + if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1_1') { + update reply { + Filter-Id += 'Fail 3' + } + } +} +else { + update reply { + Filter-Id += 'Fail 3.5' + } +} + +# Checking capture groups are cleared out correctly when there are no matches +if (User-Name =~ /^./) { + if ("%{0}%{1}%{2}%{3}%{4}%{5}%{6}%{7}" != '1') { + update reply { + Filter-Id += 'Fail 4' + } + } +} +else { + update reply { + Filter-Id += 'Fail 4.5' + } +} + +# compiled - ref - insensitive +if (Calling-Station-Id !~ /:roamyroam$/i) { + update reply { + Filter-Id += 'Fail 5' + } +} + +# compiled - expansion - insensitive +if ("%{Calling-Station-Id}" !~ /:roamyroam$/i) { + update reply { + Filter-Id += 'Fail 6' + } +} + +# compiled - enum - ref - insensitive +if (Service-Type !~ /^framed-user$/i) { + update reply { + Filter-Id += 'Fail 7' + } +} + +# compiled - enum - expansion - insensitive +if ("%{Service-Type}" !~ /^framed-user$/i) { + update reply { + Filter-Id += 'Fail 8' + } +} + +# compiled - enum - ref +if (Service-Type =~ /^framed-user$/) { + update reply { + Filter-Id += 'Fail 9' + } +} + + diff --git a/src/tests/keywords/if-regex-match-comp.attrs b/src/tests/keywords/if-regex-match-comp.attrs new file mode 100644 index 0000000..ba7188d --- /dev/null +++ b/src/tests/keywords/if-regex-match-comp.attrs @@ -0,0 +1,7 @@ +User-Name = '1_2_3_4_5_6_7' +User-Password = 'hello' +Service-Type := 'Framed-User' +Calling-Station-ID := '00:11:22:33:44:55:66:ROAMYROAM' + +Response-Packet-Type == Access-Accept +User-Name == '7_6_5_4_3_2_1_1_2_3_4_5_6_7' diff --git a/src/tests/keywords/if-regex-match.attrs b/src/tests/keywords/if-regex-match.attrs index ab03050..ba7188d 100644 --- a/src/tests/keywords/if-regex-match.attrs +++ b/src/tests/keywords/if-regex-match.attrs @@ -1,5 +1,7 @@ User-Name = '1_2_3_4_5_6_7' User-Password = 'hello' +Service-Type := 'Framed-User' +Calling-Station-ID := '00:11:22:33:44:55:66:ROAMYROAM' Response-Packet-Type == Access-Accept User-Name == '7_6_5_4_3_2_1_1_2_3_4_5_6_7'