more purify for logical operations

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index a31893af63a595075900f0b51b2eb10464b425a6..724c507ab7cb5766380726184d366ff748cdc633 100644 (file)
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -465,6 +465,12 @@ static int xlat_logical_instantiate(xlat_inst_ctx_t const *xctx)
        return 0;
 }
 
+/*
+ *     This returns "false" for "ignore this argument"
+ *
+ *     result is "false" for "delete this argument"
+ *     result is "true" for "return this argument".
+ */
 static bool xlat_node_matches_bool(xlat_exp_t *parent, xlat_exp_head_t *head, bool sense, bool *result)
 {
        fr_value_box_t *box;
@@ -513,12 +519,37 @@ check:
        return true;
 }
 
+/** Undo work which shouldn't have been done.  :(
+ *
+ */
+static void xlat_ungroup(xlat_exp_head_t *head)
+{
+       xlat_exp_t *group, *node;
+
+       group = xlat_exp_head(head);
+       if (!group || xlat_exp_next(head, group)) return;
+
+       if (group->type != XLAT_GROUP) return;
+
+       node = xlat_exp_head(group->group);
+       if (!node || xlat_exp_next(group->group, node)) return;
+
+       (void) fr_dlist_remove(&head->dlist, group);
+       (void) fr_dlist_remove(&group->group->dlist, node);
+       (void) talloc_steal(head, node);
+
+       talloc_free(group);
+
+       fr_dlist_insert_tail(&head->dlist, node);
+       head->flags = node->flags;
+}
+
 /** Any argument resolves to inst->stop, the entire thing is a bool of inst->stop
  *
  *  @todo - for now, this does very simple checks, and doesn't purify
  *  its arguments.  We also need a standard way to deregister xlat functions
  */
-static int xlat_expr_logical_purify(xlat_exp_t *node, void *instance)
+static int xlat_expr_logical_purify(xlat_exp_t *node, void *instance, request_t *request)
 {
        int i, j;
        int deleted = 0;
@@ -533,7 +564,29 @@ static int xlat_expr_logical_purify(xlat_exp_t *node, void *instance)
         *      argument.
         */
        for (i = 0; i < inst->argc; i++) {
-               if (!xlat_node_matches_bool(node, inst->argv[i], inst->stop, &result)) continue;
+               /*
+                *      The argument is pure, so we purify it before
+                *      doing any other checks.
+                */
+               if (inst->argv[i]->flags.can_purify) {
+                       if (xlat_purify_list(inst->argv[i], request) < 0) return -1;
+
+                       /*
+                        *      xlat_purify_list expects that its outputs will be arguments to functions, so
+                        *      they're grouped.  We con't need that, so we ungroup them here.
+                        */
+                       xlat_ungroup(inst->argv[i]);
+               }
+
+               /*
+                *      This returns "false" for "ignore".
+                *
+                *      result is "false" for "delete this argument"
+                *      result is "true" for "return this argument".
+                */
+               if (!xlat_node_matches_bool(node, inst->argv[i], inst->stop, &result)) {
+                       continue;
+               }
 
                /*
                 *      0 && EXPR --> 0.
@@ -544,9 +597,10 @@ static int xlat_expr_logical_purify(xlat_exp_t *node, void *instance)
                if (result) return 0;
 
                /*
-                *      We're at the last argument, we should just return that, if it's 
+                *      We're at the last argument.  If we've deleted everything else, then just leave the
+                *      last argument alone.  Otherwise some arguments remain, so we can delete the last one.
                 */
-               if ((i + 1) == inst->argc) break;
+               if (((i + 1) == inst->argc) && (deleted == i)) break;
 
                TALLOC_FREE(inst->argv[i]);
                deleted++;

diff --git a/src/tests/unit/xlat/cond_base.txt b/src/tests/unit/xlat/cond_base.txt
index 0a93b547bd0d7c6d9d7412d553a02c8f157902de..d4fabb2ce62a01267289fc308466ada60e53b387 100644 (file)
--- a/src/tests/unit/xlat/cond_base.txt
+++ b/src/tests/unit/xlat/cond_base.txt
@@ -271,7 +271,8 @@ match ERROR offset 12: Missing separator, expected ':'
 xlat_purify true
 match true
 
-# @todo - for conditions, this should evaluate to "true"
+# @todo - for conditions, this should evaluate to "true".  However, this evaluation
+#  only occurs in the condition code, and not in the xlat code!
 xlat_purify 1
 match 1
 
@@ -281,22 +282,6 @@ match false
 xlat_purify 0
 match 0
 
-#
-#  @todo - purify logical operators.  The instantiation function should update the "can_purify" flags.
-#
-xlat_purify true || (&User-Name == "bob")
-match true
-
-xlat_purify true && (&User-Name == "bob")
-match (&User-Name == "bob")
-
-xlat_purify false && (&User-Name == "bob")
-match false
-
-xlat_purify false || (&User-Name == "bob")
-match (&User-Name == "bob")
-
-
 #
 #  Both sides static data with a cast: evaluate at parse time.
 #
@@ -648,8 +633,23 @@ xlat_purify (true) && (false)
 match false
 
 #
-#  More short-circuit evaluations
+#  Purify logical operators:
+#
+#  * TRUE OP EXPR --> TRUE
+#  * FALSE OP EXPR --> EXPR
 #
+xlat_purify true || (&User-Name == "bob")
+match true
+
+xlat_purify true && (&User-Name == "bob")
+match (&User-Name == "bob")
+
+xlat_purify false && (&User-Name == "bob")
+match false
+
+xlat_purify false || (&User-Name == "bob")
+match (&User-Name == "bob")
+
 xlat_purify (&User-Name == "bob") && (false)
 match false
 
@@ -659,9 +659,8 @@ match false
 xlat_purify (&User-Name == "bob") && (&User-Password == "hello") && false
 match false
 
-# @todo - last argument isn't purified!
 xlat_purify (&User-Name == "bob") && (&User-Password == "hello") && (1 > 2)
-match ((&User-Name == "bob") && (&User-Password == "hello") && (1 > 2))
+match false
 
 xlat_purify (&User-Name == "bob") || (true)
 match true
@@ -672,12 +671,35 @@ match 1
 xlat_purify 1 || 2 || (&User-Name == "bob")
 match 1
 
+#
+#  @todo - this is arguably incorrect.  It could return
+#  "true", or "1".
+#
 xlat_purify (&User-Name == "bob") || 1 || 2
 match 1
 
 xlat_purify 1 && 2
 match 2
 
+#
+#  Cases which always match should be omitted
+#
+xlat_purify (&User-Name == "bob") && true
+match (&User-Name == "bob")
+
+xlat_purify (&User-Name == "bob") && (&User-Password == "hello") && true
+match ((&User-Name == "bob") && (&User-Password == "hello"))
+
+xlat_purify (&User-Name == "bob") || true
+match true
+
+xlat_purify (&User-Name == "bob") && (&User-Password == "hello") && ((&User-Name == "bob") || true)
+match ((&User-Name == "bob") && (&User-Password == "hello"))
+
+xlat_purify (&User-Name == "bob") && (&User-Password == "hello") && ((&User-Name == "bob") || (1 < 2))
+match ((&User-Name == "bob") && (&User-Password == "hello"))
+
+
 #
 #  A && (B || C) is not the same as (A && B) || C, for 0/1/1
 #
@@ -688,4 +710,4 @@ xlat_purify (&User-Name == "bob") && ((&User-Password == "bob") || &EAP-Message)
 match ((&User-Name == "bob") && ((&User-Password == "bob") || &EAP-Message))
 
 count
-match 280
+match 290