From: Alan T. DeKok <aland@freeradius.org>
Date: Sat, 5 Feb 2022 21:45:22 +0000 (-0500)
Subject: instantiate logical || / &&
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3b2ed43c9fd2dd560db4ffd82f0b0a54c4f62b1c;p=thirdparty%2Ffreeradius-server.git

instantiate logical || / &&

and actually do short-circuit operations.

Untested, of course.
---

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index 826ff7a2b34..c3ff6f7676e 100644
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -181,6 +181,12 @@ static int xlat_purify_expr(xlat_exp_t *node)
 
 	if (func->token == T_INVALID) return 0;
 
+	/*
+	 *	@todo - logical and/or don't have args, but they still do things.  We probably need a "purify"
+	 *	callback, or else a "purify" function which takes a request_t.
+	 */
+	if (!func->args) return 0;
+
 	/*
 	 *	@todo - for &&, ||, check only the LHS operation.  If
 	 *	it satisfies the criteria, then reparent the next
@@ -371,51 +377,154 @@ XLAT_BINARY_FUNC(cmp_le,  T_OP_LE)
 XLAT_BINARY_FUNC(cmp_gt,  T_OP_GT)
 XLAT_BINARY_FUNC(cmp_ge,  T_OP_GE)
 
-static xlat_arg_parser_t const short_circuit_xlat_args[] = {
-	{ .required = true, .type = FR_TYPE_BOOL },
-	{ .required = true, .type = FR_TYPE_BOOL },
-	XLAT_ARG_PARSER_TERMINATOR
-};
-
-static xlat_action_t xlat_func_logical_and(TALLOC_CTX *ctx, fr_dcursor_t *out,
-					   UNUSED xlat_ctx_t const *xctx,
-					   UNUSED request_t *request, fr_value_box_list_t *in)
-{
-	fr_value_box_t	*dst, *a, *b;
+typedef struct {
+	xlat_exp_t	*args;
+	bool		sense;
+} xlat_logical_inst_t;
 
-	MEM(dst = fr_value_box_alloc(ctx, FR_TYPE_BOOL, NULL, false));
+typedef struct {
+	bool			last_success;
+	xlat_exp_t		*current;
+	fr_value_box_list_t	list;
+} xlat_logical_rctx_t;
 
-	a = fr_dlist_head(in);
-	b = fr_dlist_next(in, a);
+static ssize_t xlat_expr_print_logical(fr_sbuff_t *out, xlat_exp_t const *node, void *instance, fr_sbuff_escape_rules_t const *e_rules)
+{
+	size_t	at_in = fr_sbuff_used_total(out);
+	xlat_logical_inst_t *inst = instance;
+	xlat_exp_t *current = inst->args;
 
 	/*
-	 *	@todo - short-circuit stuff inside of xlat_eval, not here.
+	 *	We might get called before the node is instantiated.
 	 */
-	dst->vb_bool = a->vb_bool && b->vb_bool;
+	if (!inst->args) current = node->child;
 
-	fr_dcursor_append(out, dst);
-	return XLAT_ACTION_DONE;
+	fr_assert(current != NULL);
+
+	FR_SBUFF_IN_CHAR_RETURN(out, '(');
+
+	while (current) {
+		xlat_print_node(out, current, e_rules);
+
+		if (!current->next) break;
+
+		FR_SBUFF_IN_STRCPY_RETURN(out, fr_tokens[node->call.func->token]);
+		FR_SBUFF_IN_CHAR_RETURN(out, ' ');
+		current = current->next;
+	}
+
+	FR_SBUFF_IN_CHAR_RETURN(out, ')');
+
+	return fr_sbuff_used_total(out) - at_in;
 }
 
+static int xlat_logical_instantiate(xlat_inst_ctx_t const *xctx)
+{
+	xlat_logical_inst_t	*inst = talloc_get_type_abort(xctx->inst, xlat_logical_inst_t);
+
+	inst->args = xctx->ex->child;
+	inst->sense = (xctx->ex->call.func->token == T_LOR);
+
+	return 0;
+}
 
-static xlat_action_t xlat_func_logical_or(TALLOC_CTX *ctx, fr_dcursor_t *out,
-					  UNUSED xlat_ctx_t const *xctx,
-					  UNUSED request_t *request, fr_value_box_list_t *in)
+static xlat_action_t xlat_logical_resume(TALLOC_CTX *ctx, fr_dcursor_t *out,
+					 xlat_ctx_t const *xctx,
+					 request_t *request, UNUSED fr_value_box_list_t *in)
 {
-	fr_value_box_t	*dst, *a, *b;
+	xlat_logical_inst_t	*inst = talloc_get_type_abort(xctx->inst, xlat_logical_inst_t);
+	xlat_logical_rctx_t	*rctx = talloc_get_type_abort(xctx->rctx, xlat_logical_rctx_t);
+	fr_value_box_t		*dst, *vb;
 
-	MEM(dst = fr_value_box_alloc(ctx, FR_TYPE_BOOL, NULL, false));
+	/*
+	 *	If one of the expansions fails, then we fail the
+	 *	entire thing.
+	 */
+	if (!rctx->last_success) {
+	fail:
+		talloc_free(rctx);
+		return XLAT_ACTION_FAIL;
+	}
 
-	a = fr_dlist_head(in);
-	b = fr_dlist_next(in, a);
+	vb = fr_dlist_head(&rctx->list);
 
 	/*
-	 *	@todo - short-circuit stuff inside of xlat_eval, not here.
+	 *	If it's a list, smash it to a boolean
 	 */
-	dst->vb_bool = a->vb_bool || b->vb_bool;
+	if ((fr_dlist_num_elements(&rctx->list) > 1) &&
+	    (fr_value_box_list_concat_in_place(ctx, vb, &rctx->list, FR_TYPE_BOOL,
+					       FR_VALUE_BOX_LIST_FREE, true,
+					       SIZE_MAX) < 0)) {
+		RPEDEBUG("Failed concatenating arguments");
+		goto fail;
+	}
 
-	fr_dcursor_append(out, dst);
-	return XLAT_ACTION_DONE;
+	/*
+	 *	If it's not a boolean, then smash it to a boolean,
+	 *	allocated in the working list ctx.
+	 */
+	if ((vb->type != FR_TYPE_BOOL) &&
+	    (fr_value_box_cast_in_place(rctx, vb, FR_TYPE_BOOL, NULL) < 0)) {
+		RPEDEBUG("Failed casting argument result");
+		return XLAT_ACTION_FAIL;
+	}
+
+	/*
+	 *	false -> false (for &&)
+	 *	true  -> true  (for ||)
+	 *	done -> whatever the last result was
+	 *
+	 *	Allocate the output box in the output ctx.
+	 */
+	if ((vb->vb_bool == inst->sense) || !rctx->current->next) {
+		MEM(dst = fr_value_box_alloc(ctx, FR_TYPE_BOOL, NULL, false));
+		dst->vb_bool = vb->vb_bool;
+		fr_dcursor_append(out, dst);
+
+		talloc_free(rctx);
+		return XLAT_ACTION_DONE;
+	}
+
+	/*
+	 *	Free the box we don't need any more, and clear the list.
+	 */
+	talloc_free(vb);
+	fr_value_box_list_init(&rctx->list);
+
+	/*
+	 *	Go to the next one, and push the next xlat onto the stack.
+	 */
+	rctx->current = rctx->current->next;
+
+	if (unlang_xlat_yield(request, xlat_logical_resume, NULL, rctx) != XLAT_ACTION_YIELD) goto fail;
+
+	if (unlang_xlat_push(rctx, &rctx->last_success, &rctx->list,
+			     request, rctx->current, UNLANG_SUB_FRAME) < 0) goto fail;
+
+	return XLAT_ACTION_PUSH_UNLANG;
+}
+
+
+static xlat_action_t xlat_func_logical(TALLOC_CTX *ctx, fr_dcursor_t *out,
+				       xlat_ctx_t const *xctx,
+				       request_t *request, UNUSED fr_value_box_list_t *in)
+{
+	xlat_logical_inst_t	*inst = talloc_get_type_abort(xctx->inst, xlat_logical_inst_t);
+	xlat_logical_rctx_t	*rctx = talloc_get_type_abort(xctx->rctx, xlat_logical_rctx_t);
+
+	rctx->current = inst->args;
+	fr_value_box_list_init(&rctx->list);
+
+	if (unlang_xlat_yield(request, xlat_logical_resume, NULL, rctx) != XLAT_ACTION_YIELD) {
+	fail:
+		talloc_free(rctx);
+		return XLAT_ACTION_FAIL;
+	}
+
+	if (unlang_xlat_push(ctx, &rctx->last_success, out->dlist,
+			     request, rctx->current, UNLANG_SUB_FRAME) < 0) goto fail;
+
+	return XLAT_ACTION_PUSH_UNLANG;
 }
 
 
@@ -508,15 +617,15 @@ int xlat_register_expressions(void)
 	/*
 	 *	&&, ||
 	 */
-	if (!(xlat = xlat_register(NULL, "logical_and", xlat_func_logical_and, XLAT_FLAG_PURE))) return -1;
-	xlat_func_args(xlat, short_circuit_xlat_args);
+	if (!(xlat = xlat_register(NULL, "logical_and", xlat_func_logical, XLAT_FLAG_PURE))) return -1;
+	xlat_async_instantiate_set(xlat, xlat_logical_instantiate, xlat_logical_inst_t, NULL, NULL);
 	xlat_internal(xlat);
-	xlat_print_set(xlat, xlat_expr_print_binary); /* @todo - fix this when we fix the instantiation */
+	xlat_print_set(xlat, xlat_expr_print_logical);
 	xlat->token = T_LAND;
 
-	if (!(xlat = xlat_register(NULL, "logical_or", xlat_func_logical_or, XLAT_FLAG_PURE))) return -1;
-	xlat_func_args(xlat, short_circuit_xlat_args);
-	xlat_print_set(xlat, xlat_expr_print_binary); /* @todo - fix this when we fix the instantiation */
+	if (!(xlat = xlat_register(NULL, "logical_or", xlat_func_logical, XLAT_FLAG_PURE))) return -1;
+	xlat_async_instantiate_set(xlat, xlat_logical_instantiate, xlat_logical_inst_t, NULL, NULL);
+	xlat_print_set(xlat, xlat_expr_print_logical);
 	xlat_internal(xlat);
 	xlat->token = T_LOR;
 
diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c
index 8c0a295bf8a..1473dc995e4 100644
--- a/src/lib/unlang/xlat_tokenize.c
+++ b/src/lib/unlang/xlat_tokenize.c
@@ -1170,8 +1170,6 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_t const *head, fr_sbuff_escape
 		goto done;
 
 	case XLAT_FUNC:
-		if (node->call.func->input_type != XLAT_INPUT_ARGS) break;
-
 		/*
 		 *	We have a callback for printing this node, go
 		 *	call it.
diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt
index 141e7e154c7..863711e0c72 100644
--- a/src/tests/unit/xlat/purify.txt
+++ b/src/tests/unit/xlat/purify.txt
@@ -85,10 +85,10 @@ match (192.168.0.0/24 > %{Framed-IP-Address})
 #  Logical && and ||
 #
 xlat_purify 1 < 2 || 4 > 3
-match yes
+match (yes || yes)
 
 xlat_purify 2 || (1 > 4)
-match yes
+match (2 || no)
 
 xlat_purify &Filter-Id
 match %{Filter-Id}