From: Alan T. DeKok Date: Wed, 9 Apr 2025 20:40:32 +0000 (-0400) Subject: remove bounce through tmpl code for %{...} X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=434a5b550a3b48bf5a11ef74bf197902dbd494eb;p=thirdparty%2Ffreeradius-server.git remove bounce through tmpl code for %{...} Instead, we create an XLAT_GROUP to hold the child xlat. We then also create a "hoist" flag, which only exists for a group node, and isn't in the flags. update the debug printer to match. Update the evaluation code to look for the "hoist' flag, and then hoist the result instead of creating a value-box group. Note that the result may be empty. In which case nothing is added to the output dcursor. For xlat function arguments, this highlights the need to have each argument expanded into its own group. --- diff --git a/src/lib/unlang/xlat.h b/src/lib/unlang/xlat.h index 3086ae86210..baefee54094 100644 --- a/src/lib/unlang/xlat.h +++ b/src/lib/unlang/xlat.h @@ -110,13 +110,13 @@ typedef struct xlat_s xlat_t; * */ typedef struct { - bool needs_resolving;//!< Needs pass2 resolution. - bool pure; //!< has no external side effects, true for BOX, LITERAL, and some functions - bool impure_func; //!< xlat contains an impure function - bool can_purify; //!< if the xlat has a pure function with pure arguments. + uint8_t needs_resolving : 1; //!< Needs pass2 resolution. + uint8_t pure : 1; //!< has no external side effects, true for BOX, LITERAL, and some functions + uint8_t impure_func : 1; //!< xlat contains an impure function + uint8_t can_purify : 1; //!< if the xlat has a pure function with pure arguments. - bool constant; //!< xlat is just tmpl_attr_tail_data, or XLAT_BOX - bool xlat; //!< it's an xlat wrapper + uint8_t constant : 1; //!< xlat is just tmpl_attr_tail_data, or XLAT_BOX + uint8_t xlat : 1; //!< it's an xlat wrapper } xlat_flags_t; extern fr_table_num_sorted_t const xlat_action_table[]; diff --git a/src/lib/unlang/xlat_eval.c b/src/lib/unlang/xlat_eval.c index c9c156b5b2d..8dddb1bfc1c 100644 --- a/src/lib/unlang/xlat_eval.c +++ b/src/lib/unlang/xlat_eval.c @@ -1097,6 +1097,19 @@ xlat_action_t xlat_frame_eval_repeat(TALLOC_CTX *ctx, fr_dcursor_t *out, XLAT_DEBUG("** [%i] %s(child) - continuing %%{%s ...}", unlang_interpret_stack_depth(request), __FUNCTION__, node->fmt); + /* + * Hoist %{...} to its results. + * + * There may be zero or more results. + */ + if (node->hoist) { + while ((arg = fr_value_box_list_pop_head(result)) != NULL) { + talloc_steal(ctx, arg); + fr_dcursor_insert(out, arg); + } + break; + } + MEM(arg = fr_value_box_alloc(ctx, FR_TYPE_GROUP, NULL)); if (!fr_value_box_list_empty(result)) { @@ -1385,10 +1398,12 @@ static int xlat_sync_stringify(TALLOC_CTX *ctx, request_t *request, xlat_exp_hea char *escaped; /* - * Groups only come about because of quoted strings. + * Groups commonly are because of quoted strings. + * + * However, we sometimes have a group because of %{...}, in which case the result is just + * a leaf value. */ - if (node->type == XLAT_GROUP) { - fr_assert(vb->type == FR_TYPE_GROUP); + if ((node->type == XLAT_GROUP) && (vb->type == FR_TYPE_GROUP)) { fr_assert(node->quote != T_BARE_WORD); if (xlat_sync_stringify(vb, request, node->group, &vb->vb_group, escape, escape_ctx) < 0) return -1; diff --git a/src/lib/unlang/xlat_priv.h b/src/lib/unlang/xlat_priv.h index 75d7dd2ded5..8419ed20601 100644 --- a/src/lib/unlang/xlat_priv.h +++ b/src/lib/unlang/xlat_priv.h @@ -161,7 +161,10 @@ struct xlat_exp_s { #endif union { - xlat_exp_head_t *group; //!< children of a group + struct { + xlat_exp_head_t *group; //!< children of a group + uint8_t hoist : 1; //!< it's a group, but we need to hoist the results + }; /** An tmpl_t reference * diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index ead27df4bb5..0b7fdab4fc7 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -601,17 +601,14 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head, /* * It must be an expression. * - * We wrap the xlat in a tmpl, so that the result is just a value, and not wrapped in another - * XLAT_GROUP, which turns into a wrapper of FR_TYPE_GROUP in the value-box. + * We wrap the xlat in a group, and then mark the group to be hoisted. */ { - xlat_exp_head_t *child; tmpl_rules_t my_rules; fr_sbuff_set(in, &m_s); /* backtrack to the start of the expression */ - MEM(node = xlat_exp_alloc(head, XLAT_TMPL, NULL, 0)); - MEM(node->vpt = tmpl_alloc(node, TMPL_TYPE_XLAT, T_BARE_WORD, "", 1)); + MEM(node = xlat_exp_alloc(head, XLAT_GROUP, NULL, 0)); if (t_rules) { my_rules = *t_rules; @@ -620,7 +617,7 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head, t_rules = &my_rules; } - ret = xlat_tokenize_expression(node->vpt, &child, in, &attr_p_rules, t_rules); + ret = xlat_tokenize_expression(node, &node->group, in, &attr_p_rules, t_rules); if (ret <= 0) { talloc_free(node); return ret; @@ -632,14 +629,15 @@ static CC_HINT(nonnull(1,2)) int xlat_tokenize_expansion(xlat_exp_head_t *head, } xlat_exp_set_name(node, fr_sbuff_current(&m_s), fr_sbuff_behind(&m_s)); - tmpl_set_name_shallow(node->vpt, T_BARE_WORD, node->fmt, fr_sbuff_behind(&m_s)); + node->flags = node->group->flags; - tmpl_set_xlat(node->vpt, child); - xlat_exp_insert_tail(head, node); + /* + * Print it as %{...}. Then when we're evaluating a string, hoist the results. + */ + node->flags.xlat = true; + node->hoist = true; - child->flags.xlat = true; - node->flags = child->flags; - fr_assert(tmpl_xlat(node->vpt) != NULL); + xlat_exp_insert_tail(head, node); (void) fr_sbuff_next(in); /* skip '}' */ return ret; @@ -1038,6 +1036,8 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t if (!node) return 0; + if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '%', '{'); + switch (node->type) { case XLAT_GROUP: if (node->quote != T_BARE_WORD) FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->quote]); @@ -1046,7 +1046,12 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t if (xlat_exp_next(head, node)) { if (c) FR_SBUFF_IN_CHAR_RETURN(out, c); - FR_SBUFF_IN_CHAR_RETURN(out, ' '); /* Add ' ' between args */ + + /* + * This thing is %{...}, and we don't print a space between the last argument + * and the '}'. + */ + if (!node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, ' '); /* Add ' ' between args */ } goto done; @@ -1106,9 +1111,7 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t if (tmpl_contains_xlat(node->vpt)) { /* xlat and exec */ if (node->vpt->quote == T_BARE_WORD) { - if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '%', '{'); xlat_print(out, tmpl_xlat(node->vpt), NULL); - if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '}'); } else { FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->vpt->quote]); xlat_print(out, tmpl_xlat(node->vpt), fr_value_escape_by_quote[node->quote]); @@ -1215,6 +1218,8 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t FR_SBUFF_IN_CHAR_RETURN(out, close); done: + if (node->flags.xlat) FR_SBUFF_IN_CHAR_RETURN(out, '}'); + return fr_sbuff_used_total(out) - at_in; } @@ -1931,6 +1936,8 @@ int xlat_resolve(xlat_exp_head_t *head, xlat_res_rules_t const *xr_rules) */ if (tmpl_resolve(node->vpt, xr_rules->tr_rules) < 0) return -1; + if (xlat_tmpl_normalize(node) < 0) return -1; + node->flags.needs_resolving = false; node->flags.pure = tmpl_is_data(node->vpt); break; diff --git a/src/tests/keywords/xlat-alternation b/src/tests/keywords/xlat-alternation index 719476823e6..b6ea1005d4a 100644 --- a/src/tests/keywords/xlat-alternation +++ b/src/tests/keywords/xlat-alternation @@ -10,7 +10,7 @@ NAS-Identifier := "bar" # First choice # result_string := "%{Filter-Id || NAS-Identifier}" -if (!(result_string == 'foo')) { +if (result_string != 'foo') { test_fail } @@ -19,7 +19,7 @@ if (!(result_string == 'foo')) { # request -= Filter-Id[*] result_string := "%{Filter-Id || NAS-Identifier}" -if (!(result_string == 'bar')) { +if (result_string != 'bar') { test_fail } @@ -27,7 +27,7 @@ if (!(result_string == 'bar')) { # Multiple things in an alternation # result_string := %{%{Filter-Id} || "%{NAS-Identifier} foo"} -if (!(result_string == 'bar foo')) { +if (result_string != 'bar foo') { test_fail } @@ -35,7 +35,7 @@ if (!(result_string == 'bar foo')) { # Alternation is empty # result_string := "%{Filter-Id || ''}" -if (!(result_string == '')) { +if (result_string != '') { test_fail } @@ -48,7 +48,7 @@ request -= NAS-Identifier[*] # Both sides are failing, so the assignment returns a NULL string # result_string := "%{Filter-Id || NAS-Identifier}" -if (!(result_string == "")) { +if (result_string !='') { test_fail } diff --git a/src/tests/unit/xlat/purify.txt b/src/tests/unit/xlat/purify.txt index c9f53cdb0e6..7693a4e8bde 100644 --- a/src/tests/unit/xlat/purify.txt +++ b/src/tests/unit/xlat/purify.txt @@ -227,8 +227,11 @@ match "hello 3" xlat_purify "hello " + (string)%{1 + 2} + " bob" match "hello 3 bob" +# +# @todo - xlat_purify doesn't hoist results. +# xlat_purify "hello %{1 + 2} bob" -match "hello %{3} bob" +match "hello 3 bob" # # New syntax!