From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 25 May 2022 20:45:54 +0000 (-0400)
Subject: fix final issues
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b8537d59f6fa933013a1806aea66d4292bdc6906;p=thirdparty%2Ffreeradius-server.git

fix final issues
---

diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c
index 588957079f8..1820099fc1f 100644
--- a/src/lib/unlang/xlat_expr.c
+++ b/src/lib/unlang/xlat_expr.c
@@ -572,6 +572,51 @@ static xlat_action_t xlat_func_unary_minus(TALLOC_CTX *ctx, fr_dcursor_t *out,
 	return XLAT_ACTION_DONE;
 }
 
+/** Convert XLAT_BOX arguments to XLAT_TMPL
+ *
+ *  xlat_tokenize() just makes all unknown arguments into XLAT_BOX, of data type FR_TYPE_STRING.  Whereas
+ *  xlat_tokenize_expr() calls tmpl_afrom_substr(), which tries hard to create a particular data type.
+ *
+ *  This function fixes up calls of the form %(op_add: 3 4), which normally passes 2 arguments of "3" and "4",
+ *  so that the arguments are instead passed as integers 3 and 4.
+ *
+ *  This fixup isn't *strictly* necessary, but it's good to have no surprises in the code, if the user creates
+ *  an expression manually.
+ */
+static int xlat_function_args_to_tmpl(xlat_inst_ctx_t const *xctx)
+{
+	xlat_exp_foreach(xctx->ex->call.args, arg) {
+		ssize_t slen;
+		xlat_exp_t *node;
+		tmpl_t *vpt;
+
+		fr_assert(arg->type == XLAT_GROUP);
+
+		node = xlat_exp_head(arg->group);
+		if (!node) continue;
+		if (node->type != XLAT_BOX) continue;
+		if (node->data.type != FR_TYPE_STRING) continue;
+
+		/*
+		 *	Try to parse it.  If we can't, leave it for a run-time error.
+		 */
+		slen = tmpl_afrom_substr(node, &vpt, &FR_SBUFF_IN(node->data.vb_strvalue, node->data.vb_length),
+					 node->quote, NULL, NULL);
+		if (slen <= 0) continue;
+		if ((size_t) slen < node->data.vb_length) continue;
+
+		/*
+		 *	Leave it as XLAT_BOX, but with the (guessed) new data type.
+		 */
+		fr_value_box_clear(&node->data);
+		fr_value_box_copy(node, &node->data, tmpl_value(vpt));
+		talloc_free(vpt);
+	}
+
+	return 0;
+}
+
+
 #undef XLAT_REGISTER_BINARY_OP
 #define XLAT_REGISTER_BINARY_OP(_op, _name) \
 do { \
@@ -579,6 +624,7 @@ do { \
 	xlat_func_args(xlat, binary_op_xlat_args); \
 	xlat_internal(xlat); \
 	xlat_print_set(xlat, xlat_expr_print_binary); \
+	xlat_async_instantiate_set(xlat, xlat_function_args_to_tmpl, NULL, NULL, NULL); \
 	xlat->token = _op; \
 } while (0)
 
diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt
index 05f9713f005..ff321664609 100644
--- a/src/tests/unit/xlat/purify.txt
+++ b/src/tests/unit/xlat/purify.txt
@@ -106,25 +106,12 @@ xlat_purify (4 + 3) + 6
 match 13
 
 #
-#  @todo - this gets parsed as "4" + "3", because the parsing of
-#  XLAT_BOX in xlat_tokenize_string() just makes everything into
-#  FR_TYPE_STRING.  The parsing in tmpl_tokenize() is different,
-#  as it tries to figure out what data type things are.
-#
-#  The solution is likely to have the expression code double-check
-#  it's arguments on instantiate, and if they are FR_TYPE_STRING with
-#  quote T_BARE_WORD, then go re-evaluate them.  Tho that seems
-#  terrible, TBH.  A cleaner way would be to add a parse flag which tells
-#  the xlat tokenizer "please try to figure out WTF this value is".
-#
-#  which then gets "43", and then "43" to int, and 43+6 -> 49
-#
-#  The rules for parsing tmpls at run-time are apparently different
-#  than parsing them at compile time?
+#  xlat_tokenize() parses it's arguments as XLAT_BOX,
+#  of FR_TYPE_STRING.  The binary op instantiation function
+#  fixes those up via tmpl_afrom_substr().
 #
 xlat_purify %(op_add:4 3) + 6
-match 49
-#match ((4 + 3) + 6)
+match 13
 
 #
 #  useless casts are omitted.
@@ -160,10 +147,7 @@ match (%{Service-Type} == 3)
 #match ERROR offset 1: Failed resolving attribute in expansion: Message
 
 #
-#  @todo - find some way quote the RHS?  Maybe just make it
-#  T_SINGLE_QUOTED_STRING for "string" types, and leave it T_BARE_WORD
-#  for everything else.  But this presumes that the RHS is always only
-#  one value-box, and perhaps it isn't?
+#  Strings are single quoted
 #
 xlat_purify &Filter-Id == ("foo" + "bar")
 match (%{Filter-Id} == 'foobar')