hoist more sanity checks to cond_promote_values()

and ensure that for regexes, LHS is always cast to string.

This ensures less work for the run-time evaluator.

diff --git a/src/lib/server/cond_tokenize.c b/src/lib/server/cond_tokenize.c
index fccd2a2dd73aa050492429186c6d8dda68a0f2c0..edb11e2435e52617aad467dd45a5e9a70e7d005f 100644 (file)
--- a/src/lib/server/cond_tokenize.c
+++ b/src/lib/server/cond_tokenize.c
@@ -244,30 +244,52 @@ int fr_cond_promote_types(fr_cond_t *c, fr_sbuff_t *in, fr_sbuff_marker_t *m_lhs
        /*
         *      Regular expressions have their own casting rules.
         */
-       if (tmpl_is_regex(c->data.map->rhs)) {
+       if (tmpl_contains_regex(c->data.map->rhs)) {
                fr_assert((c->data.map->op == T_OP_REG_EQ) || (c->data.map->op == T_OP_REG_NE));
                fr_assert(!tmpl_is_list(c->data.map->lhs));
+               fr_assert(c->data.map->rhs->cast == FR_TYPE_INVALID);
 
                /*
-                *      If the LHS is unresolved data, then cast it to
-                *      a string.
+                *      Can't use casts with regular expressions, on
+                *      LHS or RHS.  Instead, the regular expression
+                *      returns a true/false match.
+                *
+                *      It's OK to have a redundant cast to string on
+                *      the LHS.  We also allow a cast to octets, in
+                *      which case we do a binary regex comparison.
+                *
+                *      @todo - ensure that the regex interpreter is
+                *      binary-safe!
                 */
-               if (tmpl_is_unresolved(c->data.map->lhs)) {
-                       if (tmpl_cast_in_place(c->data.map->lhs, FR_TYPE_STRING, NULL) < 0) return -1;
-
-                       (void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_INVALID);
-                       return 0;
+               cast_type = c->data.map->lhs->cast;
+               if (cast_type != FR_TYPE_INVALID) {
+                       if ((cast_type != FR_TYPE_STRING) &&
+                           (cast_type != FR_TYPE_OCTETS)) {
+                               fr_strerror_const("Invalid cast used with regular expression");
+                               if (in) fr_sbuff_set(in, m_lhs);
+                               return -1;
+                       }
+               } else {
+                       cast_type = FR_TYPE_STRING;
                }
 
                /*
-                *      The LHS doesn't have a cast, so we coerce it
-                *      to string.  This means one fewer check at
-                *      run-time.
+                *      Ensure that the data is cast appropriately.
                 */
-               if (c->data.map->lhs->cast == FR_TYPE_INVALID) {
-                       (void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_STRING);
+               if (tmpl_is_unresolved(c->data.map->lhs) || tmpl_is_data(c->data.map->lhs)) {
+                       if (tmpl_cast_in_place(c->data.map->lhs, FR_TYPE_STRING, NULL) < 0) {
+                               if (in) fr_sbuff_set(in, m_lhs);
+                               return -1;
+                       }
+
+                       (void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_INVALID);
                }
 
+               /*
+                *      Force a cast to 'string', so the conditional
+                *      evaluator has less work to do.
+                */
+               (void) tmpl_cast_set(c->data.map->lhs, FR_TYPE_STRING);
                return 0;
        }
 #endif
@@ -1041,6 +1063,12 @@ static ssize_t cond_tokenize_operand(fr_cond_t *c, tmpl_t **out,
         *      at the start of the flags.
         */
        if (type == T_SOLIDUS_QUOTED_STRING) {
+               if (cast != FR_TYPE_INVALID) {
+                       fr_strerror_const("Invalid cast used with regular expression");
+                       fr_sbuff_set(&our_in, &m);
+                       goto error;
+               }
+
                if (!tmpl_contains_regex(vpt)) {
                        fr_strerror_const("Expected regex");
                        fr_sbuff_set(&our_in, &m);
@@ -1391,34 +1419,6 @@ static ssize_t cond_tokenize(TALLOC_CTX *ctx, fr_cond_t **out,
                                fr_sbuff_set(&our_in, &m_rhs);
                                goto error;
                        }
-
-                       /*
-                        *      Can't use casts with regular
-                        *      expressions, on LHS or RHS.  Instead,
-                        *      the regular expression returns a
-                        *      true/false match.
-                        *
-                        *      It's OK to have a redundant cast to
-                        *      string on the LHS.  We also allow a
-                        *      cast to octets, in which case we do a
-                        *      binary regex comparison.
-                        *
-                        *      @todo - ensure that the regex
-                        *      interpreter is binary-safe!
-                        */
-                       if ((lhs->cast != FR_TYPE_INVALID) &&
-                           (lhs->cast != FR_TYPE_STRING) &&
-                           (lhs->cast != FR_TYPE_OCTETS)) {
-                               fr_strerror_const("Invalid cast used with regular expression");
-                               fr_sbuff_set(&our_in, &m_lhs);
-                               goto error;
-                       }
-
-                       if (rhs->cast != FR_TYPE_INVALID) {
-                               fr_strerror_const("Invalid cast used with regular expression");
-                               fr_sbuff_set(&our_in, &m_rhs);
-                               goto error;
-                       }
                }
 #endif
 

diff --git a/src/tests/unit/condition/regex.txt b/src/tests/unit/condition/regex.txt
index f61ef0b38b5d0d3afe6630f27073e7a9da10f1e0..5062fc9c5391a976718ee7e65bd103e44fb8fea9 100644 (file)
--- a/src/tests/unit/condition/regex.txt
+++ b/src/tests/unit/condition/regex.txt
@@ -59,6 +59,22 @@ match &User-Name =~ /@|\\/
 condition &User-Name =~ /^([^\\]*)\\(.*)$/
 match &User-Name =~ /^([^\\]*)\\(.*)$/
 
+#
+#  Non-integer types get cast to string.
+#
+condition &Tmp-Integer-0 =~ /%{Tmp-Integer-0}/
+match <string>&Tmp-Integer-0 =~ /%{Tmp-Integer-0}/
+
+#
+#  Cannot add a bad cast
+#
+condition <integer>&Tmp-String-0 =~ /foo/
+match ERROR offset 9: Invalid cast used with regular expression
+
+condition &Tmp-String-0 =~ <integer>/foo/
+match ERROR offset 26: Invalid cast used with regular expression
+
+
 xlat %{1}
 match %{1}
 
@@ -69,4 +85,4 @@ condition &User-Name == /foo/
 match ERROR offset 14: Unexpected regular expression
 
 count
-match 37
+match 43