commit dd648b44760b9041d5ea4279e78ee7fb4e5aa13d
Author: Arran Cudbard-Bell <a.cudbardb@freeradius.org>
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'