update conditional evaluator to return true/false

there is no error.  "attribute not found" is "not match"

diff --git a/src/lib/server/cond_eval.c b/src/lib/server/cond_eval.c
index 878f848fcdc0d4d281aa7c63579c314cc9b92e1d..674572e533d0f347fd45b221e029a80f4530dc05 100644 (file)
--- a/src/lib/server/cond_eval.c
+++ b/src/lib/server/cond_eval.c
@@ -514,7 +514,7 @@ static int cond_realize_attr(request_t *request, fr_value_box_t **realized, fr_v
        return 0;
 }
 
-static int cond_compare_attrs(request_t *request, fr_value_box_t *lhs, map_t const *map)
+static bool cond_compare_attrs(request_t *request, fr_value_box_t *lhs, map_t const *map)
 {
        int                     rcode;
        fr_pair_t               *vp;
@@ -547,10 +547,10 @@ static int cond_compare_attrs(request_t *request, fr_value_box_t *lhs, map_t con
        }
 
        tmpl_pair_cursor_clear(&cc);
-       return rcode;
+       return (rcode == 1);
 }
 
-static int cond_compare_virtual(request_t *request, map_t const *map)
+static bool cond_compare_virtual(request_t *request, map_t const *map)
 {
        int                     rcode;
        fr_pair_t               *virt, *vp;
@@ -589,7 +589,7 @@ static int cond_compare_virtual(request_t *request, map_t const *map)
        }
 
        tmpl_pair_cursor_clear(&cc);
-       return rcode;
+       return (rcode == 1);
 }
 
 /** Evaluate a map
@@ -599,11 +599,10 @@ static int cond_compare_virtual(request_t *request, map_t const *map)
  * @param[in] async_lhs the asynchronously evaluated value box, for XLAT and EXEC
  * @param[in] async_rhs the asynchronously evaluated value box, for XLAT and EXEC
  * @return
- *     - -1 on failure.
- *     - 0 for "no match".
- *     - 1 for "match".
+ *     - false for no match (including "not found")
+ *     - true for match
  */
-static int cond_eval_map(request_t *request, fr_cond_t const *c,
+static bool cond_eval_map(request_t *request, fr_cond_t const *c,
                         fr_value_box_t *async_lhs, fr_value_box_t *async_rhs)
 {
        int             rcode = 0;
@@ -637,7 +636,7 @@ static int cond_eval_map(request_t *request, fr_cond_t const *c,
         */
        if (cond_realize_tmpl(request, &lhs, &lhs_free, map->lhs, map->rhs, async_lhs) < 0) {
                fr_strerror_const("Failed evaluating left side of condition");
-               return -1;
+               return false;
        }
 
        /*
@@ -645,7 +644,7 @@ static int cond_eval_map(request_t *request, fr_cond_t const *c,
         */
        if (cond_realize_tmpl(request, &rhs, &rhs_free, map->rhs, map->lhs, async_rhs) < 0) {
                fr_strerror_const("Failed evaluating right side of condition");
-               return -1;
+               return false;
        }
 
        /*
@@ -666,7 +665,7 @@ static int cond_eval_map(request_t *request, fr_cond_t const *c,
                        if (slen <= 0) {
                                REMARKER(rhs->vb_strvalue, -slen, "%s", fr_strerror());
                                EVAL_DEBUG("FAIL %d", __LINE__);
-                               return -1;
+                               return false;
                        }
                        preg = preg_free;
                }
@@ -707,7 +706,7 @@ static int cond_eval_map(request_t *request, fr_cond_t const *c,
 
                if (map->op == T_OP_REG_EQ) {
                        fr_strerror_const("Virtual attributes cannot be used with regular expressions");
-                       return -1;
+                       return false;
                }
 
                /*
@@ -726,7 +725,7 @@ static int cond_eval_map(request_t *request, fr_cond_t const *c,
                 */
                if (!rhs) {
                        fr_strerror_const("Invalid comparison for virtual attribute");
-                       return -1;
+                       return false;
                }
 
                MEM(vp = fr_pair_afrom_da(request->request_ctx, tmpl_da(map->lhs)));
@@ -839,7 +838,7 @@ done:
         *      request data, in which case we don't free it.
         */
        if (preg) talloc_free(preg_free);
-       return rcode;
+       return (rcode == 1);
 }
 
 
@@ -849,12 +848,10 @@ done:
  * @param[in] modreturn the previous module return code
  * @param[in] c the condition to evaluate
  * @return
- *     - -1 on failure.
- *     - -2 on attribute not found.
- *     - 0 for "no match".
- *     - 1 for "match".
+ *     - false for "no match", including "not found"
+ *     - true for "match"
  */
-int cond_eval(request_t *request, rlm_rcode_t modreturn, fr_cond_t const *c)
+bool cond_eval(request_t *request, rlm_rcode_t modreturn, fr_cond_t const *c)
 {
        int rcode = -1;
 
@@ -892,13 +889,13 @@ int cond_eval(request_t *request, rlm_rcode_t modreturn, fr_cond_t const *c)
                        break;
                default:
                        EVAL_DEBUG("FAIL %d", __LINE__);
-                       return -1;
+                       return false;
                }
 
                /*
-                *      Errors cause failures.
+                *      Errors are "no match".
                 */
-               if (rcode < 0) return rcode;
+               if (rcode < 0) return false;
 
                if (c->negate) rcode = !rcode;
 
@@ -912,7 +909,7 @@ int cond_eval(request_t *request, rlm_rcode_t modreturn, fr_cond_t const *c)
                while (!c->next) {
 return_to_parent:
                        c = c->parent;
-                       if (!c) return rcode;
+                       if (!c) goto done;
                }
 
                /*
@@ -938,10 +935,11 @@ return_to_parent:
                }
        }
 
+done:
        if (rcode < 0) {
                EVAL_DEBUG("FAIL %d", __LINE__);
        }
-       return rcode;
+       return (rcode == 1);
 }
 
 /** Asynchronous evaluation of conditions.
@@ -1112,7 +1110,7 @@ return_to_parent:
 /** Evaluate a map as if it is a condition.
  *
  */
-int fr_cond_eval_map(request_t *request, map_t const *map)
+bool fr_cond_eval_map(request_t *request, map_t const *map)
 {
        fr_cond_t cond;
 

diff --git a/src/lib/server/cond_eval.h b/src/lib/server/cond_eval.h
index 1c4c7eecf146f5758a23426f4d3b658d20527d1f..f754249e13ce8870199966d28ba6f69f2f8de8a7 100644 (file)
--- a/src/lib/server/cond_eval.h
+++ b/src/lib/server/cond_eval.h
@@ -39,7 +39,7 @@ typedef struct fr_cond_s fr_cond_t;
 
 void   cond_debug(fr_cond_t const *cond);
 
-int    cond_eval(request_t *request, rlm_rcode_t modreturn, fr_cond_t const *c);
+bool   cond_eval(request_t *request, rlm_rcode_t modreturn, fr_cond_t const *c);
 
 typedef struct {
        TALLOC_CTX      *ctx;           //!< for intermediate value boxes
@@ -65,7 +65,7 @@ typedef struct {
 
 int cond_eval_async(request_t *request, fr_cond_async_t *a);
 
-int fr_cond_eval_map(request_t *request, map_t const *map);
+bool fr_cond_eval_map(request_t *request, map_t const *map);
 
 #ifdef __cplusplus
 }

diff --git a/src/lib/server/cond_tokenize.c b/src/lib/server/cond_tokenize.c
index 6eb506a220bcb55cbdc706bb77cca0d197e6d06e..4ece2a3763e8569e6a25cdd0fdbb81701da163ef 100644 (file)
--- a/src/lib/server/cond_tokenize.c
+++ b/src/lib/server/cond_tokenize.c
@@ -637,15 +637,14 @@ static int cond_normalise(TALLOC_CTX *ctx, fr_token_t lhs_type, fr_cond_t **c_ou
                 */
                if (tmpl_is_data(c->data.map->lhs) &&
                    tmpl_is_data(c->data.map->rhs)) {
-                       int rcode;
+                       bool rcode;
 
                        fr_assert(c->next == NULL);
 
                        rcode = cond_eval(NULL, RLM_MODULE_NOOP, c);
-                       fr_assert(rcode >= 0);
                        TALLOC_FREE(c->data.map);
 
-                       if (rcode > 0) {
+                       if (rcode) {
                                c->type = COND_TYPE_TRUE;
                        } else {
                                c->type = COND_TYPE_FALSE;

diff --git a/src/lib/unlang/condition.c b/src/lib/unlang/condition.c
index 014652d08e1c5d28a8a19e8717aba031f706c505..9298328232f52b85b5dd5c095cc8869861b0d5e7 100644 (file)
--- a/src/lib/unlang/condition.c
+++ b/src/lib/unlang/condition.c
@@ -29,32 +29,15 @@ RCSID("$Id$")
 
 static unlang_action_t unlang_if(rlm_rcode_t *p_result, request_t *request, unlang_stack_frame_t *frame)
 {
-       int                     condition;
        unlang_group_t          *g = unlang_generic_to_group(frame->instruction);
        unlang_cond_t           *gext = unlang_group_to_cond(g);
 
        fr_assert(gext->cond != NULL);
 
-       condition = cond_eval(request, *p_result, gext->cond);
-       if (condition < 0) {
-               switch (condition) {
-               case -2:
-                       REDEBUG("Condition evaluation failed because a referenced attribute "
-                               "was not found in the request");
-                       break;
-               default:
-               case -1:
-                       REDEBUG("Condition evaluation failed because the value of an operand "
-                               "could not be determined - %s", fr_strerror());
-                       break;
-               }
-               condition = 0;
-       }
-
        /*
         *      Didn't pass.  Remember that.
         */
-       if (!condition) {
+       if (!cond_eval(request, *p_result, gext->cond)) {
                RDEBUG2("...");
                return UNLANG_ACTION_EXECUTE_NEXT;
        }

diff --git a/src/modules/rlm_files/rlm_files.c b/src/modules/rlm_files/rlm_files.c
index 89c0487647c417a740513da940d73dee861f1ba5..f3522fa64969d585a95448d8a351d13e11cabe8a 100644 (file)
--- a/src/modules/rlm_files/rlm_files.c
+++ b/src/modules/rlm_files/rlm_files.c
@@ -477,7 +477,6 @@ redo:
                 *      Realize the map to a list of VPs
                 */
                while ((map = fr_dlist_next(&pl->check, map))) {
-                       int rcode;
                        fr_pair_list_t tmp_list;
 
                        /*
@@ -505,13 +504,7 @@ redo:
                                 *      Evaluate the map, including regexes.
                                 */
                        default:
-                               rcode = fr_cond_eval_map(request, map);
-                               if (rcode < 0) {
-                                       RPWARN("Failed evaluating check item, skipping entry");
-                                       break;
-                               }
-
-                               if (rcode == 0) match = false;
+                               if (!fr_cond_eval_map(request, map)) match = false;
                                break;
                        }
 

diff --git a/src/tests/keywords/notfound b/src/tests/keywords/notfound
new file mode 100644 (file)
index 0000000..e617fe2
--- /dev/null
+++ b/src/tests/keywords/notfound
@@ -0,0 +1,25 @@
+#
+#  PRE: if update
+#
+
+#
+#  This doesn't exist, so  the condition shouldn't match.
+#
+if (&Tmp-String-0 == "foo") {
+       test_fail
+}
+else {
+       ok
+}
+
+#
+# Since the middle part doesn't match, "NOT" the middle part SHOULD match.
+#
+if (!(&Tmp-String-0 == "foo")) {
+       ok
+}
+else {
+       test_fail
+}
+
+success