catch more corner cases of constant strings when tokenizing

update to commit b17b28a02

diff --git a/src/lib/unlang/xlat_tokenize.c b/src/lib/unlang/xlat_tokenize.c
index 982cb17e1e71da17d041090839b421078d539ccb..f6ad9077e8c8a5cb08e97c642d62a5207b191122 100644 (file)
--- a/src/lib/unlang/xlat_tokenize.c
+++ b/src/lib/unlang/xlat_tokenize.c
@@ -924,14 +924,12 @@ static CC_HINT(nonnull(1,2,4)) ssize_t xlat_tokenize_input(xlat_exp_head_t *head
                         *      string into it instead of inserting a fresh node.  This merge ensures that we
                         *      only have one constant value-box produced, instead of many.
                         */
-                       if (prev && (prev->type == XLAT_BOX)) {
+                       if (prev && (prev->type == XLAT_BOX) && (prev->data.type == FR_TYPE_STRING)) {
                                size_t  prev_len = prev->data.vb_length;
                                size_t  add_len = talloc_strlen(str);
                                size_t  total = prev_len + add_len;
                                char    *merged;
 
-                               fr_assert(prev->data.type == FR_TYPE_STRING);
-
                                MEM(fr_value_box_bstr_realloc(prev, &merged, &prev->data, total) == 0);
                                memcpy(merged + prev_len, str, add_len + 1);
 
@@ -1581,32 +1579,43 @@ fr_slen_t xlat_tokenize_word(TALLOC_CTX *ctx, xlat_exp_t **out, fr_sbuff_t *in,
                xlat_exp_set_name(node, fr_sbuff_current(&m), fr_sbuff_behind(&m));
 
                /*
-                *      There's no expansion in the string.  Hoist the value-box.
+                *      There's no expansion in the string.  Hoist the value-box where we can.
                 */
                if (node->flags.constant) {
-                       xlat_exp_t *child;
+                       size_t num = fr_dlist_num_elements(&node->group->dlist);
 
                        /*
-                        *      The list is either empty, or else it has one child, which is the constant
-                        *      node.
+                        *      Empty string: convert the wrapper to an empty XLAT_BOX.
+                        *
+                        *      Exactly one child of type XLAT_BOX: hoist the child up in place of the
+                        *      wrapper.
+                        *
+                        *      Anything else (multiple constant children, or a single non-box child like
+                        *      a hoisted %{(const)} expression result) is left wrapped in an XLAT_GROUP.
+                        *      The hoist flag on the GROUP makes the runtime concatenate the children's
+                        *      stringified values at eval time, which is the correct semantics for cases
+                        *      like "x%{(1)}y".  The integer 1 still needs to be stringified before it
+                        *      can be merged into the surrounding literal text.
                         */
-                       if (fr_dlist_num_elements(&node->group->dlist) == 0) {
+                       if (num == 0) {
                                xlat_exp_set_type(node, XLAT_BOX);
 
                                fr_value_box_init(&node->data, FR_TYPE_STRING, NULL, false);
                                fr_value_box_strdup(node, &node->data, NULL, "", false);
 
-                       } else {
-                               fr_assert(fr_dlist_num_elements(&node->group->dlist) == 1);
-
-                               child = talloc_steal(ctx, xlat_exp_head(node->group));
-                               talloc_free(node);
-                               node = child;
-                       }
+                               fr_assert(node->type == XLAT_BOX);
+                               node->quote = quote;
 
-                       fr_assert(node->type == XLAT_BOX);
+                       } else if (num == 1) {
+                               xlat_exp_t *child = xlat_exp_head(node->group);
 
-                       node->quote = quote; /* not the same node! */
+                               if (child->type == XLAT_BOX) {
+                                       (void) talloc_steal(ctx, child);
+                                       talloc_free(node);
+                                       node = child;
+                                       node->quote = quote; /* not the same node! */
+                               } /* else there are single non-box constant child, leave the wrapper alone */
+                       } /* else there are multiple constant children, leave the wrapper alone */
                }
                break;
 

diff --git a/src/tests/unit/xlat/base.txt b/src/tests/unit/xlat/base.txt
index 246bb7284e4ec3518a27a0b008120ae5940758bc..1713ed1bab4a467bd545bab9f8be2672c2be02e1 100644 (file)
--- a/src/tests/unit/xlat/base.txt
+++ b/src/tests/unit/xlat/base.txt
@@ -272,10 +272,10 @@ xlat_argv echo hello %{Filter-Id}:1234 world
 match [0]{ echo }, [1]{ hello }, [2]{ %{Filter-Id}:1234 }, [3]{ world }
 
 #
-#  Double-quoted constant arguments containing a literal '%'.  Without the
-#  in-tokenizer merge in xlat_tokenize_input(), the group node would contain
-#  multiple constant value-box children and trip the
-#  fr_dlist_num_elements(...) == 1 assertion in xlat_tokenize_word().
+#  Double-quoted constant arguments which contain a literal '%'.  Without the
+#  value-box merge in xlat_tokenize_input(), the group node would contain
+#  multiple constant value-box children, and violate the assumptions of
+#  the rest of the code.
 #
 xlat_argv "100%"
 match [0]{ "100\%" }
@@ -289,6 +289,34 @@ match [0]{ /bin/sh }, [1]{ "100\%" }, [2]{ "%{User-Name}" }
 xlat_argv foo "ratio is %" bar
 match [0]{ foo }, [1]{ "ratio is \%" }, [2]{ bar }
 
+#
+#  Double-quoted strings containing a `%{(<constant expression>)}` between
+#  literal text.  The `(...)` form takes the expression branch in
+#  xlat_tokenize_expansion(), and the inner expression is constant — so the
+#  resulting XLAT_GROUP has flags.constant=true.  xlat_tokenize_word's
+#  hoist-when-constant block used to assert dlist == 1, which failed here
+#  because head is [BOX text, GROUP %{(const)}, BOX text].  It now leaves
+#  the wrapper alone for the multi-constant case; hoist=true on the GROUP
+#  stringifies and concatenates at eval time.
+#
+xlat_argv "x%{(1)}y"
+match [0]{ "x%{1}y" }
+
+xlat_argv "x%{((1))}y"
+match [0]{ "x%{1}y" }
+
+xlat_argv "x%{(true)}y"
+match [0]{ "x%{true}y" }
+
+xlat_argv "a%{(1)}"
+match [0]{ "a%{1}" }
+
+xlat_argv "%{(1)}b"
+match [0]{ "%{1}b" }
+
+xlat_argv "x%{(1 + 2)}y"
+match [0]{ "x%{(1 + 2)}y" }
+
 xlat %debug(5)
 match %debug(5)
 
@@ -343,4 +371,4 @@ xlat %hash.md5('arg"')
 match %hash.md5(0x61726722)
 
 count
-match 179
+match 191