From: Alan T. DeKok Date: Wed, 1 Jun 2022 00:09:40 +0000 (-0400) Subject: more purify for logical operations X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bd715216dec730267dac61e33c42325522478758;p=thirdparty%2Ffreeradius-server.git more purify for logical operations --- diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index a31893af63a..724c507ab7c 100644 --- 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 0a93b547bd0..d4fabb2ce62 100644 --- 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