From: Alan T. DeKok Date: Mon, 16 Oct 2023 22:10:58 +0000 (-0400) Subject: remove "hoist vpt->xlat into xlat" in tokenize expression X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=152f4d89cf47d28d9ce0778d1ce183b088821361;p=thirdparty%2Ffreeradius-server.git remove "hoist vpt->xlat into xlat" in tokenize expression the hoisting would put the expansion into an XLAT_GROUP, which meant that any output value-boxes were wrapped in a value-box group. Which was distinctly unexpected. --- diff --git a/src/lib/unlang/xlat_expr.c b/src/lib/unlang/xlat_expr.c index abf43e6b238..548c3959cfe 100644 --- a/src/lib/unlang/xlat_expr.c +++ b/src/lib/unlang/xlat_expr.c @@ -2462,30 +2462,6 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf * Do various tmpl fixups. */ - /* - * Don't keep an intermediate tmpl for xlats. Just hoist - * the xlat to be a child of this node. Exec and regexes - * are left alone, as they are handled by different code. - */ - if (tmpl_is_xlat(vpt) || tmpl_is_xlat_unresolved(vpt)) { - xlat_exp_head_t *xlat = tmpl_xlat(vpt); - xlat_exp_t *arg = NULL; - - XLAT_HEAD_VERIFY(xlat); - - MEM(arg = xlat_exp_alloc(head, XLAT_GROUP, vpt->name, strlen(vpt->name))); - - talloc_steal(arg->group, xlat); - arg->group = xlat; - arg->quote = quote; - arg->flags = xlat->flags; - - talloc_free(node); /* also frees tmpl, leaving just the xlat */ - node = arg; - - goto done; - } - /* * Try and add any unknown attributes to the dictionary immediately. This means any future * references will all point to the same da. @@ -2520,7 +2496,16 @@ static fr_slen_t tokenize_field(xlat_exp_head_t *head, xlat_exp_t **out, fr_sbuf node->quote = quote; xlat_exp_set_name_buffer_shallow(node, vpt->name); - node->flags.pure = tmpl_is_data(node->vpt); + if (tmpl_is_data(node->vpt)) { + node->flags.pure = true; + + } else if (tmpl_contains_xlat(node->vpt)) { + node->flags = tmpl_xlat(vpt)->flags; + + } else { + node->flags.pure = false; + } + node->flags.constant = node->flags.pure; node->flags.needs_resolving = tmpl_needs_resolving(node->vpt); diff --git a/src/lib/unlang/xlat_purify.c b/src/lib/unlang/xlat_purify.c index fd297fe6fb0..0f7e20b47a4 100644 --- a/src/lib/unlang/xlat_purify.c +++ b/src/lib/unlang/xlat_purify.c @@ -77,6 +77,18 @@ int xlat_purify_list(xlat_exp_head_t *head, request_t *request) if (!node->flags.can_purify) continue; switch (node->type) { + case XLAT_TMPL: + if (tmpl_is_xlat(node->vpt)) { + xlat_exp_head_t *child = tmpl_xlat(node->vpt); + + rcode = xlat_purify_list(child, request); + if (rcode < 0) return rcode; + + node->flags = child->flags; + break; + } + FALL_THROUGH; + default: fr_strerror_printf("Internal error - cannot purify xlat"); return -1; diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c index 256dc4ae9e8..e1ceb6214e6 100644 --- a/src/lib/unlang/xlat_tokenize.c +++ b/src/lib/unlang/xlat_tokenize.c @@ -1486,15 +1486,14 @@ ssize_t xlat_print_node(fr_sbuff_t *out, xlat_exp_head_t const *head, xlat_exp_t goto done; } - if (tmpl_is_xlat(node->vpt)) { - xlat_print(out, tmpl_xlat(node->vpt), fr_value_escape_by_quote[node->quote]); - goto done; - } - - if (tmpl_contains_xlat(node->vpt)) { /* exec */ - 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]); - FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->vpt->quote]); + if (tmpl_contains_xlat(node->vpt)) { /* xlat and exec */ + if (node->vpt->quote == T_BARE_WORD) { + xlat_print(out, tmpl_xlat(node->vpt), NULL); + } 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]); + FR_SBUFF_IN_CHAR_RETURN(out, fr_token_quote[node->vpt->quote]); + } goto done; } diff --git a/src/tests/xlat/expr.txt b/src/tests/xlat/expr.txt index d5a51373476..7666387e6b9 100644 --- a/src/tests/xlat/expr.txt +++ b/src/tests/xlat/expr.txt @@ -35,7 +35,7 @@ xlat_expr (uint32) 1 << 31 match 2147483648 xlat_expr %n -match {0} +match 0 xlat_expr &Net.Src.IP =~ /^127.0/ match true @@ -122,7 +122,7 @@ match foo baz bar # The User-Name has to exist # xlat_expr %{User-Name} -match {bob} +match bob # # We're not hashing the string value of the attribute reference diff --git a/src/tests/xlat/file.txt b/src/tests/xlat/file.txt index 9bbd91945c0..4a7e33802aa 100644 --- a/src/tests/xlat/file.txt +++ b/src/tests/xlat/file.txt @@ -1,29 +1,35 @@ xlat_expr %file.head('src/tests/xlat/file/one') -match {foo} +match foo # # No LF at the end of the line # xlat_expr %file.head('src/tests/xlat/file/one-no-lf') -match {foo} +match foo xlat_expr %file.head('src/tests/xlat/file/two') -match {foo bar} +match foo bar xlat_expr %file.tail('src/tests/xlat/file/two') -match {baz is good} +match baz is good xlat_expr %file.tail('src/tests/xlat/file/two-no-lf') -match {baz is good} +match baz is good xlat_expr %file.tail('src/tests/xlat/file/three') -match {but it was good} +match but it was good xlat_expr %file.tail('src/tests/xlat/file/three', 1) -match {but it was good} +match but it was good xlat_expr %file.tail('src/tests/xlat/file/three', 2) -match {who was very baaad\012but it was good} +match who was very baaad\012but it was good xlat_expr %file.size('src/tests/xlat/file/three') -match {64} +match 64 + +xlat_expr %file.exists('src/tests/xlat/file/three') +match yes + +xlat_expr %file.exists('src/tests/xlat/file/no-such-file') +match no